RFR: 8254129: IR Test Framework to support regex-based matching on the IR in JTreg compiler tests [v2]

[Christian Hagedorn]

On Fri, 16 Apr 2021 01:10:07 GMT, Igor Ignatyev <iignatyev at openjdk.org> wrote:

>> test/lib/jdk/test/lib/hotspot/ir_framework/TestFramework.java line 367:
>> 
>>> 365: 
>>> 366:         for (Class<?> helperClass : helperClasses) {
>>> 367:             TestRun.check(!this.helperClasses.contains(helperClass), "Cannot add the same class twice: " + helperClass);
>> 
>> won't it be easy to use `Set` to store `helperClasses`?
>
> you might also want to update javadoc for this method to state that there can be duplicates.

Have I understood this correctly that you suggest to just remove this check completely and just specifying in the Javadocs that duplicates are ignore (due to using a `Set`)?

>> test/lib/jdk/test/lib/hotspot/ir_framework/TestFramework.java line 639:
>> 
>>> 637:             TestFormat.check(!scenarioIndices.contains(scenarioIndex),
>>> 638:                              "Cannot define two scenarios with the same index " + scenarioIndex);
>>> 639:             scenarioIndices.add(scenarioIndex);
>> 
>> you can use `Set::add` to verify that the element isn't in a set:
>> Suggestion:
>> 
>>             TestFormat.check(scenarioIndices.add(scenarioIndex),
>>                              "Cannot define two scenarios with the same index " + scenarioIndex);
>
> also, shouldn't this check be done as part of `addScenarios`?

Right, I moved it to `addScenarios`.

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

PR: https://git.openjdk.java.net/jdk/pull/3508