diff --git a/docs/user-guide/06-interfaces-generics.md b/docs/user-guide/06-interfaces-generics.md index 1912fcb024a..bb19fd776e3 100644 --- a/docs/user-guide/06-interfaces-generics.md +++ b/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 --------------------- diff --git a/source/slang/slang-ast-modifier.h b/source/slang/slang-ast-modifier.h index 4f6a08e4f35..ca023a19ba6 100644 --- a/source/slang/slang-ast-modifier.h +++ b/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 { diff --git a/source/slang/slang-check-decl.cpp b/source/slang/slang-check-decl.cpp index 8471acb0ac4..23458c94afb 100644 --- a/source/slang/slang-check-decl.cpp +++ b/source/slang/slang-check-decl.cpp @@ -5055,6 +5055,40 @@ HasInterfaceDefaultImplModifier* hasDefaultImpl(DeclRef declRef) return nullptr; } +void SemanticsVisitor::markOverridingDecl( + ConformanceCheckingContext* context, + Decl* memberDecl, + DeclRef 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()) + { + getSink()->diagnose(memberDecl, Diagnostics::missingOverride); + } + } + auto overridingModifier = m_astBuilder->create(); + 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(modifier)) + { + hasOverride = true; + } + else if (as(modifier)) + { + isOverriding = true; + } + } + if (hasOverride && !isOverriding) + { + getSink()->diagnose( + innerMember, + Diagnostics::overrideModifierNotOverridingBaseDecl, + innerMember); + } + } } } diff --git a/source/slang/slang-check-impl.h b/source/slang/slang-check-impl.h index be6b1c2fc97..365f21b1c8b 100644 --- a/source/slang/slang-check-impl.h +++ b/source/slang/slang-check-impl.h @@ -1892,6 +1892,11 @@ struct SemanticsVisitor : public SemanticsContext DeclRef requirement, DeclRef method); + void markOverridingDecl( + ConformanceCheckingContext* context, + Decl* memberDecl, + DeclRef requiredMemberDeclRef); + /// Attempt to synthesize a method that can satisfy `requiredMemberDeclRef` using /// `lookupResult`. /// diff --git a/source/slang/slang-check-modifier.cpp b/source/slang/slang-check-modifier.cpp index 1d38fa802c8..2a4707db566 100644 --- a/source/slang/slang-check-modifier.cpp +++ b/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(decl) || as(decl) || as(decl); + case ASTNodeType::OverrideModifier: + return as(decl) && as(getParentDecl(decl)); default: return true; } diff --git a/source/slang/slang-diagnostic-defs.h b/source/slang/slang-diagnostic-defs.h index 4f0edf53b34..64b47ece8f2 100644 --- a/source/slang/slang-diagnostic-defs.h +++ b/source/slang/slang-diagnostic-defs.h @@ -1731,6 +1731,17 @@ DIAGNOSTIC( invalidExtensionOnInterface, "cannot extend interface type '$0'. consider using a generic extension: `extension 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( diff --git a/source/slang/slang-parser.cpp b/source/slang/slang-parser.cpp index 6d6341e4e08..9727140f36d 100644 --- a/source/slang/slang-parser.cpp +++ b/source/slang/slang-parser.cpp @@ -9467,6 +9467,7 @@ static const SyntaxParseInfo g_parseSyntaxEntries[] = { _makeParseModifier("writeonly", parseWriteonlyModifier), _makeParseModifier("export", getSyntaxClass()), _makeParseModifier("dynamic_uniform", getSyntaxClass()), + _makeParseModifier("override", getSyntaxClass()), // Modifiers for geometry shader input _makeParseModifier("point", getSyntaxClass()), diff --git a/tests/diagnostics/missing-override.slang b/tests/diagnostics/missing-override.slang new file mode 100644 index 00000000000..cc2cbcad3ee --- /dev/null +++ b/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; } +} \ No newline at end of file diff --git a/tests/language-feature/interfaces/default-method-generic-interface.slang b/tests/language-feature/interfaces/default-method-generic-interface.slang index fb32b20013f..f33950a82a1 100644 --- a/tests/language-feature/interfaces/default-method-generic-interface.slang +++ b/tests/language-feature/interfaces/default-method-generic-interface.slang @@ -30,7 +30,7 @@ struct Impl2 : IFoo<2> } // overriding default implementation. - int getGreaterVal() + override int getGreaterVal() { return 100 + x; } diff --git a/tests/language-feature/interfaces/override-default-method.slang b/tests/language-feature/interfaces/override-default-method.slang new file mode 100644 index 00000000000..22ae483118e --- /dev/null +++ b/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() + { + return getVal() + x; + } +} + +struct Impl : IFoo +{ + int getVal() + { + return 42; + } + + // override default implementation for getGreaterVal. + override int getGreaterVal() + { + return getVal() + x + 1; // Adding 1 to differentiate from the default implementation. + } +} + +int test(T v) { return v.getGreaterVal<1>(); } + +//TEST_INPUT: set resultBuffer = out ubuffer(data=[0 0 0 0], stride=4) +RWStructuredBuffer resultBuffer; + +[numthreads(1,1,1)] +void computeMain() +{ + Impl impl = {}; + int result = test(impl); + resultBuffer[0] = result; + // CHECK: 44 +} \ No newline at end of file