Require override keyword for overriding default interface methods.

docs/user-guide/06-interfaces-generics.md

@@ -58,6 +58,17 @@ interface IFoo
 struct MyType : IFoo {}
 ```
 
+A concrete type that provides its overriding implementation to an interface method requirement that has a default implementation must be explicitly marked as 'override'. For example:
+
+```slang
+struct MyType2 : IFoo
+{
+    // Explicitly mark `getVal` as `override` is needed
+    // because `IFoo.getVal` has a body.
+    override int getVal() { return 1; }
+}
+```
+
 Generics
 ---------------------
 

source/slang/slang-ast-modifier.h

@@ -66,6 +66,21 @@ class InternalModifier : public VisibilityModifier
     FIDDLE(...)
 };
 
+FIDDLE()
+class OverrideModifier : public Modifier
+{
+    FIDDLE(...)
+};
+
+// Marks that a decl is verified to be overriding another decl defined in a base type.
+FIDDLE()
+class IsOverridingModifier : public Modifier
+{
+    FIDDLE(...)
+
+    FIDDLE() Decl* overridedDecl = nullptr;
+};
+
 FIDDLE()
 class RequireModifier : public Modifier
 {

source/slang/slang-check-decl.cpp

@@ -5055,6 +5055,40 @@ HasInterfaceDefaultImplModifier* hasDefaultImpl(DeclRef<Decl> declRef)
     return nullptr;
 }
 
+void SemanticsVisitor::markOverridingDecl(
+    ConformanceCheckingContext* context,
+    Decl* memberDecl,
+    DeclRef<Decl> requiredMemberDeclRef)
+{
+    if (!memberDecl->isChildOf(context->parentDecl))
+    {
+        // If the member being checked isn't the child of the container decl containing
+        // the inheritance decl that triggers the conformance check, don't modify/diagnose
+        // anything since it should have already been diagnosed when checking its parent.
+        // This can happen when we check for things like:
+        //    extension float : IComparable{}
+        // where the method used to satisfy IComparable comes from outside the extension decl.
+        // we don't want to diagnose or check anything about the found memberDecl that doesn't
+        // belong to this extension decl (context->parentDecl).
+        //
+        return;
+    }
+
+    if (hasDefaultImpl(requiredMemberDeclRef))
+    {
+        memberDecl = maybeGetInner(memberDecl);
+        // If the required member has a default implementation,
+        // we need to make sure the member we found is marked as 'override'.
+        if (!memberDecl->hasModifier<OverrideModifier>())
+        {
+            getSink()->diagnose(memberDecl, Diagnostics::missingOverride);
+        }
+    }
+    auto overridingModifier = m_astBuilder->create<IsOverridingModifier>();
+    overridingModifier->overridedDecl = requiredMemberDeclRef.getDecl();
+    addModifier(memberDecl, overridingModifier);
+}
+
 bool SemanticsVisitor::trySynthesizeMethodRequirementWitness(
     ConformanceCheckingContext* context,
     LookupResult const& lookupResult,
@@ -5345,6 +5379,8 @@ bool SemanticsVisitor::trySynthesizeMethodRequirementWitness(
                         return false;
                     }
                 }
+
+                markOverridingDecl(context, callee.getDecl(), requiredMemberDeclRef);
             }
         }
     }
@@ -7232,6 +7268,7 @@ bool SemanticsVisitor::findWitnessForInterfaceRequirement(
                     QualifiedDeclPath(requiredMemberDeclRef));
                 return false;
             }
+            markOverridingDecl(context, member.declRef.getDecl(), requiredMemberDeclRef);
             return true;
         }
     }
@@ -7738,6 +7775,33 @@ void SemanticsVisitor::checkAggTypeConformance(AggTypeDecl* decl)
         // only the types that are affected by these interface decls.
         //
         astBuilder->incrementEpoch();
+
+        // For all members marked as `override`, we need to ensure they are actually
+        // overriding something.
+        for (auto member : decl->getMembers())
+        {
+            auto innerMember = maybeGetInner(member);
+            bool hasOverride = false;
+            bool isOverriding = false;
+            for (auto modifier : innerMember->modifiers)
+            {
+                if (as<OverrideModifier>(modifier))
+                {
+                    hasOverride = true;
+                }
+                else if (as<IsOverridingModifier>(modifier))
+                {
+                    isOverriding = true;
+                }
+            }
+            if (hasOverride && !isOverriding)
+            {
+                getSink()->diagnose(
+                    innerMember,
+                    Diagnostics::overrideModifierNotOverridingBaseDecl,
+                    innerMember);
+            }
+        }
     }
 }
 

source/slang/slang-check-impl.h

@@ -1892,6 +1892,11 @@ struct SemanticsVisitor : public SemanticsContext
         DeclRef<CallableDecl> requirement,
         DeclRef<CallableDecl> method);
 
+    void markOverridingDecl(
+        ConformanceCheckingContext* context,
+        Decl* memberDecl,
+        DeclRef<Decl> requiredMemberDeclRef);
+
     /// Attempt to synthesize a method that can satisfy `requiredMemberDeclRef` using
     /// `lookupResult`.
     ///

source/slang/slang-check-modifier.cpp

@@ -1529,6 +1529,8 @@ bool isModifierAllowedOnDecl(bool isGLSLInput, ASTNodeType modifierType, Decl* d
         return isGlobalDecl(decl) || isEffectivelyStatic(decl);
     case ASTNodeType::DynModifier:
         return as<InterfaceDecl>(decl) || as<VarDecl>(decl) || as<ParamDecl>(decl);
+    case ASTNodeType::OverrideModifier:
+        return as<FunctionDeclBase>(decl) && as<AggTypeDecl>(getParentDecl(decl));
     default:
         return true;
     }

source/slang/slang-diagnostic-defs.h

@@ -1731,6 +1731,17 @@ DIAGNOSTIC(
     invalidExtensionOnInterface,
     "cannot extend interface type '$0'. consider using a generic extension: `extension<T:$0> T "
     "{...}`.")
+DIAGNOSTIC(
+    30853,
+    Error,
+    missingOverride,
+    "missing 'override' keyword for methods that overrides the default implementation in the "
+    "interface.")
+DIAGNOSTIC(
+    30854,
+    Error,
+    overrideModifierNotOverridingBaseDecl,
+    "'$0' marked as 'override' is not overriding any base declarations.")
 
 // 309xx: subscripts
 DIAGNOSTIC(

source/slang/slang-parser.cpp

@@ -9467,6 +9467,7 @@ static const SyntaxParseInfo g_parseSyntaxEntries[] = {
     _makeParseModifier("writeonly", parseWriteonlyModifier),
     _makeParseModifier("export", getSyntaxClass<HLSLExportModifier>()),
     _makeParseModifier("dynamic_uniform", getSyntaxClass<DynamicUniformModifier>()),
+    _makeParseModifier("override", getSyntaxClass<OverrideModifier>()),
 
     // Modifiers for geometry shader input
     _makeParseModifier("point", getSyntaxClass<HLSLPointModifier>()),

tests/diagnostics/missing-override.slang

@@ -0,0 +1,20 @@
+//DIAGNOSTIC_TEST:SIMPLE(filecheck=CHECK):
+
+interface IFoo
+{
+    int getVal() { return 0; }
+}
+
+struct Impl : IFoo
+{
+    // Missing override for getVal, which should trigger a diagnostic.
+    // CHECK: ([[# @LINE+1]]): error 30853
+    int getVal() { return 1; }
+}
+
+struct Impl2 : IFoo
+{
+    // Overriding getVal with a different signature should also trigger a diagnostic.
+    // CHECK: ([[# @LINE+1]]): error 30854
+    override int getVal(int x) { return x; }
+}

tests/language-feature/interfaces/default-method-generic-interface.slang

@@ -30,7 +30,7 @@ struct Impl2 : IFoo<2>
     }
 
     // overriding default implementation.
-    int getGreaterVal<int x>()
+    override int getGreaterVal<int x>()
     {
         return 100 + x;
     }

tests/language-feature/interfaces/override-default-method.slang

@@ -0,0 +1,40 @@
+//TEST(compute):COMPARE_COMPUTE(filecheck-buffer=CHECK): -shaderobj -output-using-type
+
+// Test that a generic interface method can have a body providing default implementation.
+
+interface IFoo
+{
+    int getVal();
+    int getGreaterVal<int x>()
+    {
+        return getVal() + x;
+    }
+}
+
+struct Impl : IFoo
+{
+    int getVal()
+    {
+        return 42;
+    }
+
+    // override default implementation for getGreaterVal.
+    override int getGreaterVal<int x>()
+    {
+        return getVal() + x + 1; // Adding 1 to differentiate from the default implementation.
+    }
+}
+
+int test<T:IFoo>(T v) { return v.getGreaterVal<1>(); }
+
+//TEST_INPUT: set resultBuffer = out ubuffer(data=[0 0 0 0], stride=4)
+RWStructuredBuffer<int> resultBuffer;
+
+[numthreads(1,1,1)]
+void computeMain()
+{
+    Impl impl = {};
+    int result = test(impl);
+    resultBuffer[0] = result;
+    // CHECK: 44
+}