diff options
| -rw-r--r-- | docs/user-guide/06-interfaces-generics.md | 11 | ||||
| -rw-r--r-- | source/slang/slang-ast-modifier.h | 15 | ||||
| -rw-r--r-- | source/slang/slang-check-decl.cpp | 64 | ||||
| -rw-r--r-- | source/slang/slang-check-impl.h | 5 | ||||
| -rw-r--r-- | source/slang/slang-check-modifier.cpp | 2 | ||||
| -rw-r--r-- | source/slang/slang-diagnostic-defs.h | 11 | ||||
| -rw-r--r-- | source/slang/slang-parser.cpp | 1 | ||||
| -rw-r--r-- | tests/diagnostics/missing-override.slang | 20 | ||||
| -rw-r--r-- | tests/language-feature/interfaces/default-method-generic-interface.slang | 2 | ||||
| -rw-r--r-- | tests/language-feature/interfaces/override-default-method.slang | 40 |
10 files changed, 170 insertions, 1 deletions
diff --git a/docs/user-guide/06-interfaces-generics.md b/docs/user-guide/06-interfaces-generics.md index 1912fcb02..bb19fd776 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 4f6a08e4f..ca023a19b 100644 --- a/source/slang/slang-ast-modifier.h +++ b/source/slang/slang-ast-modifier.h @@ -67,6 +67,21 @@ class InternalModifier : public VisibilityModifier }; 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 { FIDDLE(...) diff --git a/source/slang/slang-check-decl.cpp b/source/slang/slang-check-decl.cpp index 8471acb0a..23458c94a 100644 --- a/source/slang/slang-check-decl.cpp +++ b/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); + } + } } } diff --git a/source/slang/slang-check-impl.h b/source/slang/slang-check-impl.h index be6b1c2fc..365f21b1c 100644 --- a/source/slang/slang-check-impl.h +++ b/source/slang/slang-check-impl.h @@ -1892,6 +1892,11 @@ public: 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`. /// diff --git a/source/slang/slang-check-modifier.cpp b/source/slang/slang-check-modifier.cpp index 1d38fa802..2a4707db5 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<InterfaceDecl>(decl) || as<VarDecl>(decl) || as<ParamDecl>(decl); + case ASTNodeType::OverrideModifier: + return as<FunctionDeclBase>(decl) && as<AggTypeDecl>(getParentDecl(decl)); default: return true; } diff --git a/source/slang/slang-diagnostic-defs.h b/source/slang/slang-diagnostic-defs.h index 4f0edf53b..64b47ece8 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:$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( diff --git a/source/slang/slang-parser.cpp b/source/slang/slang-parser.cpp index 6d6341e4e..9727140f3 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<HLSLExportModifier>()), _makeParseModifier("dynamic_uniform", getSyntaxClass<DynamicUniformModifier>()), + _makeParseModifier("override", getSyntaxClass<OverrideModifier>()), // Modifiers for geometry shader input _makeParseModifier("point", getSyntaxClass<HLSLPointModifier>()), diff --git a/tests/diagnostics/missing-override.slang b/tests/diagnostics/missing-override.slang new file mode 100644 index 000000000..cc2cbcad3 --- /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 fb32b2001..f33950a82 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<int x>() + override int getGreaterVal<int x>() { 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 000000000..22ae48311 --- /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<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 +}
\ No newline at end of file |