Fix assumption warning should only be triggered for usages that reach a @fallback.

Author: chumer

truffle/CHANGELOG.md

@@ -49,6 +49,8 @@ Guards that only bind idempotent methods and no dynamic values can always be ass
 The generated code leverages this information and asserts instead of executes the guard on the fast-path.
 The DSL now emits warnings with for all guards where specifying the annotations may be beneficial.
 Note that all guards that do not bind dynamic values are assumed idempotent by default for compatibility reasons.
+* GR-43903 Usages of `@Specialization(assumptions=...)` that reach a `@Fallback` specialization now produce a suppressable warning. In most situations, such specializations should be migrated to use a regular guard instead. For example, instead of using `@Specialization(assumptions = "assumption")` you might need to be using `@Specialization(guards = "assumption.isValid()")`.
+* GR-43903 Added `@Idempotent` and `@NonIdempotent` DSL annotations useful for DSL guard optimizations. Guards that only bind idempotent methods and no dynamic values can always be assumed `true` after they were `true` once on the slow-path. The generated code leverages this information and asserts instead of executes the guard on the fast-path. The DSL now emits warnings with for all guards where specifying the annotations may be beneficial. Note that all guards that do not bind dynamic values are assumed idempotent by default for compatibility reasons.
 
 
 * GR-43903 DSL method guards that do not bind dynamic values are no longer just asserted on the fast-path. 

truffle/docs/DSLWarnings.md

@@ -26,7 +26,7 @@ The following warning keys are supported:
 * `truffle-static-method` warnings when the DSL recommends to use the `static` modifier.
 * `truffle-unused` warnings if a DSL attribute or annotation has no effect and is recommended to be removed. 
 * `truffle-abstract-export` warnings if an abstract message of a Truffle library is not exported.
-* `truffle-assumption` if the assumptions feature is used with a specialization that may never have multiple instances.
+* `truffle-assumption` if the assumptions feature is used with a specialization that reaches a `@Fallback` specialization.
 * `truffle-guard` if a guard uses methods where a `@Idempotent` or `@NonIdempotent` method may be beneficial for the generated code.  
 
 Specific warnings can also be suppressed globally using the `-Atruffle.dsl.SuppressWarnings=truffle-inlining,truffle-neverdefault` Java compiler processor option. 

truffle/src/com.oracle.truffle.api.dsl.test/src/com/oracle/truffle/api/dsl/test/AssumptionsTest.java

@@ -784,6 +784,7 @@ abstract static class AssumptionFallbackNode extends SlowPathListenerNode {
 
         abstract boolean execute(Object arg);
 
+        @ExpectError("It is discouraged to use assumptions with a specialization that reaches a @Fallback specialization. %")
         @Specialization(guards = {"arg > 1", "arg == cachedArg"}, assumptions = "assumption", limit = "1")
         public boolean s0(int arg,
                         @Cached("arg") int cachedArg,
@@ -816,7 +817,6 @@ abstract static class AssumptionNoGuardNode extends SlowPathListenerNode {
 
         abstract boolean execute(Object arg);
 
-        @ExpectError("It is discouraged to use the assumptions property with a specialization that cannot have multiple specialization instances%")
         @Specialization(assumptions = "assumption")
         public boolean s0(Object arg,
                         @Cached("NEVER_VALID") Assumption assumption) {

truffle/src/com.oracle.truffle.api.dsl.test/src/com/oracle/truffle/api/dsl/test/CachedReachableFallbackTest.java

@@ -304,6 +304,7 @@ abstract static class GuardKindsNode extends Node {
 
         abstract Object execute(Object other);
 
+        @SuppressWarnings("truffle-assumption")
         @Specialization(guards = {"guardNode1.execute(obj)", "notTwo(obj)"}, rewriteOn = RuntimeException.class, assumptions = "createAssumption()", limit = "1")
         protected Object s1(int obj,
                         @Cached(value = "create(1)") NotGuardNode guardNode1) {

truffle/src/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/parser/NodeParser.java

@@ -1532,6 +1532,22 @@ private static void initializeFallbackReachability(NodeData node) {
             }
 
         }
+
+        for (SpecializationData specialization : specializations) {
+            if (!specialization.getAssumptionExpressions().isEmpty() && specialization.isReachesFallback()) {
+                specialization.addSuppressableWarning(TruffleSuppressedWarnings.ASSUMPTION,
+                                """
+                                                It is discouraged to use assumptions with a specialization that reaches a @%s specialization.\s\
+                                                Specialization instances get removed if assumptions are no longer valid, which may lead to unexpected @%s invocations.\s\
+                                                This may be fixed by translating the assumption usage to a regular method guard instead.\s\
+                                                Instead of assumptions="a" you may use guards="a.isValid()".\s\
+                                                This problem may also be fixed by adding a new more generic specialization that replaces this specialization.\s\
+                                                """,
+                                getSimpleName(types.Fallback),
+                                getSimpleName(types.Fallback));
+            }
+        }
+
     }
 
     private static void initializeExecutableTypeHierarchy(NodeData node) {
@@ -3025,21 +3041,6 @@ private void initializeAssumptions(SpecializationData specialization, DSLExpress
             assumptionId++;
         }
         specialization.setAssumptionExpressions(assumptionExpressions);
-
-        if (!assumptionExpressions.isEmpty() && !specialization.isGuardBindsCache()) {
-            specialization.addSuppressableWarning(TruffleSuppressedWarnings.ASSUMPTION, """
-                            It is discouraged to use the assumptions property with a specialization that cannot have multiple specialization instances.\s\
-                            Note that assumption specializations get removed if the assumption is no longer true.\s\
-                            This rarely makes sense for specializations without multiple instances.\s\
-                            Most often this can be fixed by translating the assumption usage to a regular method guard instead.\s\
-                            So instead of assumptions="a" you may use guards="a.isValid()".\s\
-                            If your specialization should have multiple instances, make sure you bind a cached value in the guard.\s\
-                            """,
-                            getSimpleName(types.Fallback),
-                            getSimpleName(types.Specialization),
-                            getSimpleName(types.Fallback));
-        }
-
     }
 
     private void initializeLimit(SpecializationData specialization, DSLExpressionResolver resolver, boolean uncached) {