RFR: 8344942: Template-Based Testing Framework [v71]

[Emanuel Peter]

On Mon, 2 Jun 2025 13:13:24 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> Emanuel Peter has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 91 commits:
>> 
>>  - Merge branch 'master' into JDK-8344942-TemplateFramework-v3
>>  - validation tests
>>  - dollar and hashtag parsing validatiaon
>>  - wip refactor parsing dollar and hashtag
>>  - more fixes from Christian
>>  - more improvements
>>  - more suggestions applied
>>  - good practice
>>  - rename template arguments
>>  - more from Christian
>>  - ... and 81 more: https://git.openjdk.org/jdk/compare/90d6ad01...cb7037e7
>
> test/hotspot/jtreg/compiler/lib/template_framework/DataName.java line 181:
> 
>> 179:          */
>> 180:         public DataName sample() {
>> 181:             DataName n = (DataName)Renderer.getCurrent().sampleName(predicate());
> 
> Do you really need this cast? Can't you just return a `Name`. From the uses it seems that you only call interface methods from `Name` at the use-sites.

That would require `Name` to become public. I wanted to avoid that.

I want the user to think that `DataName` and `StructuralName` are separate but parallel. But internally, they have a unified implementation with `Name`.
An alternative would have been to use `NameSet` generically, once with `DataName` and `StructuralName`. But that would mean we could have a `DataName` with the same `name` as a `StructuralName`, because they would be in separate `NameSet`s.

What do you think?

> test/hotspot/jtreg/compiler/lib/template_framework/StructuralName.java line 47:
> 
>> 45:      */
>> 46:     public StructuralName {
>> 47:     }
> 
> Is this required? Is it not automatically added? Same for `DataName`.

If I remove it, then `javadoc` complains that we are now using the default constructor, and that it does not have a comment for it... kinda strange but ok 😅

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24217#discussion_r2122789086
PR Review Comment: https://git.openjdk.org/jdk/pull/24217#discussion_r2122796063