RFR: 8336919: Cleanup and rename tags in placeholders code [v3]

[David Holmes]

On Thu, 25 Jul 2024 18:10:04 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> This change does some renaming of tags, fields and function name (resolve_super_or_fail => resolve_with_circularity_detection) in the Placeholders and SystemDictionary (class loading) code.  resolve_super_or_fail implied that it's always the super class, which it is not, especially with the upcoming changes to valhalla.  Renaming the function to resolve/load and check for circularity error is more accurate. These are changes made by @fparain brought over from the valhalla branch.
>> Tested with tier1-4.
>
> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Whitespace and wording change.

I like that you added `resolve_with_circularity_detection` as more general API that is called by `resolve_super_or_fail`, but there are still a number of comments that are very super-centric and code that says it deal with supers without consulting `is_super_class`.

Line 387: `// Must be called for any superclass or superinterface resolution`

Line 427:  `// If klass is already loaded, just return the superclass or superinterface.`

Also `klass` should be `class_name` (pre-existing)

Line 471: `// Resolve the superclass or superinterface, check results on return`

Line 458:      `// Be careful not to exit resolve_super without removing this placeholder.`

Method name needs updating

Thanks.

src/hotspot/share/classfile/systemDictionary.cpp line 509:

> 507:   // The only thing that takes different action for is_superclass is dumping the static archive, which doesn't
> 508:   // reach this path.
> 509:   assert (!CDSConfig::is_dumping_static_archive(), "should not be parallel loading static archive");

Suggestion: "should not be dumping static archive"

src/hotspot/share/classfile/systemDictionary.hpp line 106:

> 104: 
> 105:   static InstanceKlass* resolve_with_circularity_detection(Symbol* class_name,
> 106:                                                            Symbol* super_name,

s/super_name/next_name/

-------------

PR Review: https://git.openjdk.org/jdk/pull/20279#pullrequestreview-2203696400
PR Review Comment: https://git.openjdk.org/jdk/pull/20279#discussion_r1694418160
PR Review Comment: https://git.openjdk.org/jdk/pull/20279#discussion_r1694418563