Ensure generic constraints are checked before inner extension. (#7685)

* Ensure generic constraints are checked before inner extension.

* Add warning for non-standard generic extension.

* Fix tests.

* Fix test.

* Ban interface types from equality constraints.

* Fix.

10 files changed, 203 insertions, 21 deletions

diff --git a/source/slang/slang-check-decl.cpp b/source/slang/slang-check-decl.cpp
index 7cd1d7df8..e67962ca3 100644
--- a/source/slang/slang-check-decl.cpp
+++ b/source/slang/slang-check-decl.cpp
@@ -1414,6 +1414,20 @@ static void _dispatchDeclCheckingVisitor(
     DeclCheckState state,
     SemanticsContext& shared);
 
+void SemanticsVisitor::ensureOuterGenericConstraints(Decl* decl, DeclCheckState state)
+{
+    if (auto genericDecl = GetOuterGeneric(decl))
+    {
+        auto nextGeneric = findNextOuterGeneric(genericDecl);
+        if (nextGeneric)
+        {
+            if (nextGeneric->checkState.getState() < state)
+                ensureOuterGenericConstraints(nextGeneric, state);
+        }
+        ensureDecl(genericDecl, state);
+    }
+}
+
 // Make sure a declaration has been checked, so we can refer to it.
 // Note that this may lead to us recursively invoking checking,
 // so this may not be the best way to handle things.
@@ -3159,6 +3173,18 @@ bool SemanticsVisitor::trySynthesizeDifferentialAssociatedTypeRequirementWitness
     return true;
 }
 
+bool isProperConstraineeType(Type* type)
+{
+    auto declRef = isDeclRefTypeOf<Decl>(type);
+    if (!declRef)
+        return false;
+    if (as<InterfaceDecl>(declRef.getDecl()))
+        return false;
+    // TODO: `some` type and `dyn` types are also inproper constrainee types.
+
+    return true;
+}
+
 void SemanticsDeclHeaderVisitor::validateGenericConstraintSubType(
     GenericTypeConstraintDecl* decl,
     TypeExp type)
@@ -3221,6 +3247,13 @@ void SemanticsDeclHeaderVisitor::validateGenericConstraintSubType(
             validateGenericConstraintSubType(decl, type);
         }
     }
+    if (!isProperConstraineeType(type.type))
+    {
+        // It is meaningless for certain types to be used in type constraints.
+        // For example, `IFoo<T>` should not appear as the left-hand-side of a generic constraint.
+        getSink()->diagnose(type.exp, Diagnostics::invalidConstraintSubType, type);
+        return;
+    }
 }
 
 // General utility function to collect all referenced declarations from a value
@@ -3333,12 +3366,6 @@ void SemanticsDeclHeaderVisitor::visitTypeCoercionConstraintDecl(TypeCoercionCon
 
 void SemanticsDeclHeaderVisitor::visitGenericTypeConstraintDecl(GenericTypeConstraintDecl* decl)
 {
-    // TODO: are there any other validations we can do at this point?
-    //
-    // There probably needs to be a kind of "occurs check" to make
-    // sure that the constraint actually applies to at least one
-    // of the parameters of the generic.
-    //
     CheckConstraintSubType(decl->sub);
 
     if (!decl->sub.type)
@@ -3346,18 +3373,32 @@ void SemanticsDeclHeaderVisitor::visitGenericTypeConstraintDecl(GenericTypeConst
     if (!decl->sup.type)
         decl->sup = TranslateTypeNodeForced(decl->sup);
 
-    // Check for forward references in generic constraints after type translation
-    checkForwardReferencesInGenericConstraint(decl);
-
     if (getLinkage()->m_optionSet.shouldRunNonEssentialValidation())
     {
-        validateGenericConstraintSubType(decl, decl->sub);
-    }
+        // Check for forward references in generic constraints after type translation
+        checkForwardReferencesInGenericConstraint(decl);
 
-    if (!decl->isEqualityConstraint && !isValidGenericConstraintType(decl->sup) &&
-        !as<ErrorType>(decl->sub.type))
-    {
-        getSink()->diagnose(decl->sup.exp, Diagnostics::invalidTypeForConstraint, decl->sup);
+        validateGenericConstraintSubType(decl, decl->sub);
+        if (decl->isEqualityConstraint)
+        {
+            if (!isProperConstraineeType(decl->sup) && !as<ErrorType>(decl->sup.type))
+            {
+                getSink()->diagnose(
+                    decl->sup.exp,
+                    Diagnostics::invalidEqualityConstraintSupType,
+                    decl->sup);
+            }
+        }
+        else
+        {
+            if (!isValidGenericConstraintType(decl->sup) && !as<ErrorType>(decl->sup.type))
+            {
+                getSink()->diagnose(
+                    decl->sup.exp,
+                    Diagnostics::invalidTypeForConstraint,
+                    decl->sup);
+            }
+        }
     }
 }
 
@@ -10708,14 +10749,18 @@ void SemanticsDeclBasesVisitor::_validateExtensionDeclGenericParams(ExtensionDec
         ensureDecl(genericDecl, DeclCheckState::ReadyForReference);
 
         // Collect all declarations referenced by the target type
-        HashSet<Decl*> referencedDecls;
-        collectReferencedDecls(decl->targetType.type, referencedDecls);
+        HashSet<Decl*> genericParamsReferencedByTargetType;
+        collectReferencedDecls(decl->targetType.type, genericParamsReferencedByTargetType);
+
+        HashSet<Decl*> genericParamsReferencedByConstraints;
 
         // Also collect declarations referenced by generic constraints
         for (auto constraint :
              getMembersOfType<GenericTypeConstraintDecl>(getASTBuilder(), genericDecl))
         {
-            collectReferencedDecls(constraint.getDecl()->sup.type, referencedDecls);
+            collectReferencedDecls(
+                constraint.getDecl()->sup.type,
+                genericParamsReferencedByConstraints);
         }
 
         // Note: We intentionally do NOT check inheritance declarations in the extension.
@@ -10727,7 +10772,9 @@ void SemanticsDeclBasesVisitor::_validateExtensionDeclGenericParams(ExtensionDec
         {
             if (as<GenericTypeParamDeclBase>(member) || as<GenericValueParamDecl>(member))
             {
-                if (!referencedDecls.contains(member))
+                bool referencedByTargetType = genericParamsReferencedByTargetType.contains(member);
+                bool referencedByConstraint = genericParamsReferencedByConstraints.contains(member);
+                if (!referencedByTargetType && !referencedByConstraint)
                 {
                     getSink()->diagnose(
                         member,
@@ -10735,6 +10782,14 @@ void SemanticsDeclBasesVisitor::_validateExtensionDeclGenericParams(ExtensionDec
                         member->getName(),
                         decl->targetType);
                 }
+                else if (!referencedByTargetType && !isFromCoreModule(decl))
+                {
+                    getSink()->diagnose(
+                        member,
+                        Diagnostics::genericParamInExtensionNotReferencedByTargetType,
+                        member->getName(),
+                        decl->targetType);
+                }
             }
         }
     }
@@ -10742,6 +10797,11 @@ void SemanticsDeclBasesVisitor::_validateExtensionDeclGenericParams(ExtensionDec
 
 void SemanticsDeclBasesVisitor::visitExtensionDecl(ExtensionDecl* decl)
 {
+    // If the extension is a generic, we need to make sure to check
+    // the outer generic constraints first.
+    //
+    ensureOuterGenericConstraints(decl, DeclCheckState::ReadyForReference);
+
     // We check the target type expression and members, and then validate
     // that the type it names is one that it makes sense
     // to extend.
diff --git a/source/slang/slang-check-impl.h b/source/slang/slang-check-impl.h
index 75d0bfc90..55211dde5 100644
--- a/source/slang/slang-check-impl.h
+++ b/source/slang/slang-check-impl.h
@@ -1474,6 +1474,8 @@ public:
     ///
     void ensureDeclBase(DeclBase* decl, DeclCheckState state, SemanticsContext* baseContext);
 
+    void ensureOuterGenericConstraints(Decl* decl, DeclCheckState state);
+
     // Check if `lambdaStruct` can be coerced to `funcType`, if so returns the coerced
     // expression in `outExpr`. The coercion is only valid if the lambda struct
     // does not contain any captures.
diff --git a/source/slang/slang-diagnostic-defs.h b/source/slang/slang-diagnostic-defs.h
index dd7885281..8dbdaaaa6 100644
--- a/source/slang/slang-diagnostic-defs.h
+++ b/source/slang/slang-diagnostic-defs.h
@@ -1647,6 +1647,11 @@ DIAGNOSTIC(
     requiredConstraintIsNotChecked,
     "the constraint providing '$0' is optional and must be checked with an 'is' statement before "
     "usage.")
+DIAGNOSTIC(
+    30404,
+    Error,
+    invalidEqualityConstraintSupType,
+    "type '$0' is not a proper type to use in a generic equality constraint.")
 
 // 305xx: initializer lists
 DIAGNOSTIC(30500, Error, tooManyInitializers, "too many initializers (expected $0, got $1)")
@@ -1795,7 +1800,12 @@ DIAGNOSTIC(
     Error,
     unreferencedGenericParamInExtension,
     "generic parameter '$0' is not referenced by extension target type '$1'.")
-
+DIAGNOSTIC(
+    30856,
+    Warning,
+    genericParamInExtensionNotReferencedByTargetType,
+    "the extension is non-standard and may not work as intended because the generic parameter '$0' "
+    "is not referenced by extension target type '$1'.")
 // 309xx: subscripts
 DIAGNOSTIC(
     30900,
diff --git a/source/slang/slang-parser.cpp b/source/slang/slang-parser.cpp
index a2fd944eb..6401b3d06 100644
--- a/source/slang/slang-parser.cpp
+++ b/source/slang/slang-parser.cpp
@@ -3649,7 +3649,7 @@ static void parseOptionalGenericConstraints(Parser* parser, ContainerDecl* decl)
 
             // substitution needs to be filled during check
             Type* paramType = nullptr;
-            if (as<GenericTypeParamDeclBase>(decl))
+            if (as<GenericTypeParamDeclBase>(decl) || as<GlobalGenericParamDecl>(decl))
             {
                 paramType = DeclRefType::create(parser->astBuilder, DeclRef<Decl>(decl));
 
diff --git a/tests/bugs/gh-5140.slang b/tests/bugs/gh-5140.slang
index 23d9b3a23..bf18b04a6 100644
--- a/tests/bugs/gh-5140.slang
+++ b/tests/bugs/gh-5140.slang
@@ -3,6 +3,8 @@
 
 //TEST_INPUT:ubuffer(data=[0 0 0 0], stride=4):out,name=outputBuffer
 
+#pragma warning(disable:30856)
+
 public interface A<T: IFloat>
 {
 }
diff --git a/tests/diagnostics/generic-constraint-equality-right-hand-side.slang b/tests/diagnostics/generic-constraint-equality-right-hand-side.slang
new file mode 100644
index 000000000..c49fe2035
--- /dev/null
+++ b/tests/diagnostics/generic-constraint-equality-right-hand-side.slang
@@ -0,0 +1,36 @@
+//TEST:SIMPLE(filecheck=CHECK):
+interface IGen<A>
+{
+    associatedtype TB;
+    int getVal();
+}
+
+interface Wrapper<A>
+{}
+
+struct Foo1<A> : IGen<A>
+{
+    typealias TB = Wrapper<A>; // `Wrapper<int>` also fails.
+    int val = 0;
+    int getVal()
+    {
+       return val;
+    }
+}
+
+struct Logic<A1, C1 : IGen<A1>>
+{
+    int val = 0;
+}
+
+extension<A, C1> Logic<A, C1>
+    where C1 : IGen<A>
+    //CHECK: ([[# @LINE+1]]): error 30404: 
+    where C1.TB == Wrapper<int>
+{
+    [mutating]
+    void setVal(int dataIn)
+    {
+        val = dataIn;
+    }
+}
diff --git a/tests/diagnostics/non-standard-extension.slang b/tests/diagnostics/non-standard-extension.slang
new file mode 100644
index 000000000..d6a4350a4
--- /dev/null
+++ b/tests/diagnostics/non-standard-extension.slang
@@ -0,0 +1,17 @@
+//TEST:SIMPLE(filecheck=CHECK): -warnings-as-errors 30856
+
+interface ICompatibleWith<T> {}
+interface IBase {}
+struct INT : IBase {}
+struct FLOAT : IBase {}
+
+extension<EX : IBase> EX : ICompatibleWith<INT> {}
+extension<EY : IBase> EY : ICompatibleWith<FLOAT> {}
+
+struct VECTOR<T> {}
+
+// CHECK: ([[# @LINE+1]]): error 30856: the extension is non-standard
+extension<T, U : ICompatibleWith<T>> VECTOR<U> : ICompatibleWith<VECTOR<T>> {}
+
+void main()
+{}
\ No newline at end of file
diff --git a/tests/language-feature/extensions/generic-extension-2.slang b/tests/language-feature/extensions/generic-extension-2.slang
index 2728a73d6..d56455e3d 100644
--- a/tests/language-feature/extensions/generic-extension-2.slang
+++ b/tests/language-feature/extensions/generic-extension-2.slang
@@ -1,4 +1,6 @@
 //TEST(compute):COMPARE_COMPUTE(filecheck-buffer=CHECK): -shaderobj
+
+#pragma warning(disable:30856)
 interface IFoo<T>
 {
     T getFirst();    
diff --git a/tests/language-feature/extensions/generic-extension-5.slang b/tests/language-feature/extensions/generic-extension-5.slang
new file mode 100644
index 000000000..e6466d614
--- /dev/null
+++ b/tests/language-feature/extensions/generic-extension-5.slang
@@ -0,0 +1,52 @@
+//TEST:INTERPRET(filecheck=CHECK):
+interface IGen<A>
+{
+    associatedtype TB;
+    TB getVal();
+}
+
+struct Foo1<A> : IGen<A>
+{
+    typealias TB = int;
+    int val = 0;
+    TB getVal()
+    {
+       return val; 
+    }
+}
+
+struct Foo2<A> : IGen<A>
+{
+    typealias TB = int;
+    int val = 0;
+    TB getVal()
+    {
+       return val; 
+    }
+}
+
+struct Logic<A1, C1 : IGen<A1>, C2 : IGen<A1>>
+{
+    int val = 0;
+}
+
+extension<A, C1, C2> Logic<A, C1, C2>
+    where C1 : IGen<A>
+    where C2 : IGen<A>
+    where C1.TB == C2.TB
+{
+    [mutating]
+    void setVal(int dataIn)
+    {
+        val = dataIn;
+    }
+}
+
+void main()
+{
+    Logic<int, Foo1<int>, Foo2<int>> logic;
+    logic.setVal(42);
+    int result = logic.val;
+    printf("Result: %d\n", result);
+    // CHECK: Result: 42
+}
\ No newline at end of file
diff --git a/tests/language-feature/overloaded-subscript.slang b/tests/language-feature/overloaded-subscript.slang
index 68ad1111a..e928e97ec 100644
--- a/tests/language-feature/overloaded-subscript.slang
+++ b/tests/language-feature/overloaded-subscript.slang
@@ -13,6 +13,7 @@ interface IRWBuf<T> : IBuf<T>
     void write(int x, T v);
 }
 
+#pragma warning(disable:30856)
 extension<T, U : IBuf<T>> U
 {
     __subscript(int x) -> T { get { return read(x); } }