diff options
| -rw-r--r-- | source/slang/check.cpp | 127 | ||||
| -rw-r--r-- | source/slang/core.meta.slang | 3 | ||||
| -rw-r--r-- | source/slang/core.meta.slang.h | 3 | ||||
| -rw-r--r-- | source/slang/diagnostic-defs.h | 2 | ||||
| -rw-r--r-- | source/slang/lookup.cpp | 19 | ||||
| -rw-r--r-- | source/slang/lower-to-ir.cpp | 30 | ||||
| -rw-r--r-- | source/slang/modifier-defs.h | 6 | ||||
| -rw-r--r-- | source/slang/syntax.h | 19 | ||||
| -rw-r--r-- | tests/compute/mutating-methods.slang | 51 | ||||
| -rw-r--r-- | tests/compute/mutating-methods.slang.expected.txt | 4 | ||||
| -rw-r--r-- | tests/diagnostics/setter-method.slang.expected | 4 |
11 files changed, 202 insertions, 66 deletions
diff --git a/source/slang/check.cpp b/source/slang/check.cpp index f0e7be32b..985c5c0b7 100644 --- a/source/slang/check.cpp +++ b/source/slang/check.cpp @@ -475,11 +475,13 @@ namespace Slang } RefPtr<Expr> createImplicitThisMemberExpr( - Type* type, - SourceLoc loc) + Type* type, + SourceLoc loc, + LookupResultItem::Breadcrumb::ThisParameterMode thisParameterMode) { RefPtr<ThisExpr> expr = new ThisExpr(); expr->type = type; + expr->type.IsLeftValue = thisParameterMode == LookupResultItem::Breadcrumb::ThisParameterMode::Mutating; expr->loc = loc; return expr; } @@ -528,14 +530,16 @@ namespace Slang { bb = createImplicitThisMemberExpr( GetTargetType(extensionDeclRef), - loc); + loc, + breadcrumb->thisParameterMode); } else { auto type = DeclRefType::Create(getSession(), breadcrumb->declRef); bb = createImplicitThisMemberExpr( type, - loc); + loc, + breadcrumb->thisParameterMode); } } break; @@ -779,6 +783,10 @@ namespace Slang decl->SetCheckState(DeclCheckState::CheckingHeader); } + // Check the modifiers on the declaration first, in case + // semantics of the body itself will depend on them. + checkModifiers(decl); + // Use visitor pattern to dispatch to correct case dispatchDecl(decl); @@ -2197,12 +2205,6 @@ namespace Slang EnusreAllDeclsRec(d); } - // Do any semantic checking required on modifiers? - for (auto d : programNode->Members) - { - checkModifiers(d.Ptr()); - } - if (pass == 0) { // now we can check all interface conformances @@ -2236,6 +2238,14 @@ namespace Slang DeclRef<CallableDecl> requiredMemberDeclRef, RefPtr<WitnessTable> witnessTable) { + if(satisfyingMemberDeclRef.getDecl()->HasModifier<MutatingAttribute>() + && !requiredMemberDeclRef.getDecl()->HasModifier<MutatingAttribute>()) + { + // A `[mutating]` method can't satisfy a non-`[mutating]` requirement, + // but vice-versa is okay. + return false; + } + // TODO: actually implement matching here. For now we'll // just pretend that things are satisfied in order to make progress.. witnessTable->requirementDictionary.Add( @@ -4599,6 +4609,44 @@ namespace Slang // + /// Given an immutable `expr` used as an l-value emit a special diagnostic if it was derived from `this`. + void maybeDiagnoseThisNotLValue(Expr* expr) + { + // We will try to handle expressions of the form: + // + // e ::= "this" + // | e . name + // | e [ expr ] + // + // We will unwrap the `e.name` and `e[expr]` cases in a loop. + RefPtr<Expr> e = expr; + for(;;) + { + if(auto memberExpr = e.As<MemberExpr>()) + { + e = memberExpr->BaseExpression; + } + else if(auto subscriptExpr = e.As<IndexExpr>()) + { + e = subscriptExpr->BaseExpression; + } + else + { + break; + } + } + // + // Now we check to see if we have a `this` expression, + // and if it is immutable. + if(auto thisExpr = e.As<ThisExpr>()) + { + if(!thisExpr->type.IsLeftValue) + { + getSink()->diagnose(thisExpr, Diagnostics::thisIsImmutableByDefault); + } + } + } + RefPtr<Expr> visitAssignExpr(AssignExpr* expr) { expr->left = CheckExpr(expr->left); @@ -4622,40 +4670,7 @@ namespace Slang // is immutable. We can give the user a bit more context into // what is going on. // - // We will try to handle expressions of the form: - // - // e ::= "this" - // | e . name - // | e [ expr ] - // - // We will unwrap the `e.name` and `e[expr]` cases in a loop. - RefPtr<Expr> e = expr->left; - for(;;) - { - if(auto memberExpr = e.As<MemberExpr>()) - { - e = memberExpr->BaseExpression; - } - else if(auto subscriptExpr = e.As<IndexExpr>()) - { - e = subscriptExpr->BaseExpression; - } - else - { - break; - } - } - // - // Now we check to see if we have a `this` expression, - // and if it is immutable. - if(auto thisExpr = e.As<ThisExpr>()) - { - if(!thisExpr->type.IsLeftValue) - { - getSink()->diagnose(thisExpr, Diagnostics::thisIsImmutableByDefault); - } - } - + maybeDiagnoseThisNotLValue(expr->left); } } expr->type = type; @@ -6298,7 +6313,10 @@ namespace Slang LookupResultItem ctorItem; ctorItem.declRef = ctorDeclRef; - ctorItem.breadcrumbs = new LookupResultItem::Breadcrumb(LookupResultItem::Breadcrumb::Kind::Member, typeItem.declRef, typeItem.breadcrumbs); + ctorItem.breadcrumbs = new LookupResultItem::Breadcrumb( + LookupResultItem::Breadcrumb::Kind::Member, + typeItem.declRef, + typeItem.breadcrumbs); OverloadCandidate candidate; candidate.flavor = OverloadCandidate::Flavor::Func; @@ -7585,6 +7603,8 @@ namespace Slang implicitCastExpr->Arguments[0]->type, implicitCastExpr->type); } + + maybeDiagnoseThisNotLValue(argExpr); } } else @@ -8132,6 +8152,9 @@ namespace Slang // expression. RefPtr<Expr> visitThisExpr(ThisExpr* expr) { + // A `this` expression will default to immutable. + expr->type.IsLeftValue = false; + // We will do an upwards search starting in the current // scope, looking for a surrounding type (or `extension`) // declaration that could be the referrant of the expression. @@ -8139,14 +8162,22 @@ namespace Slang while (scope) { auto containerDecl = scope->containerDecl; - if (auto aggTypeDecl = containerDecl->As<AggTypeDecl>()) + + if( auto funcDeclBase = containerDecl->As<FunctionDeclBase>() ) + { + if( funcDeclBase->HasModifier<MutatingAttribute>() ) + { + expr->type.IsLeftValue = true; + } + } + else if (auto aggTypeDecl = containerDecl->As<AggTypeDecl>()) { checkDecl(aggTypeDecl); // Okay, we are using `this` in the context of an // aggregate type, so the expression should be // of the corresponding type. - expr->type = DeclRefType::Create( + expr->type.type = DeclRefType::Create( getSession(), makeDeclRef(aggTypeDecl)); return expr; @@ -8165,7 +8196,7 @@ namespace Slang // if there are multiple extensions in scope that add // members with the same name... // - expr->type = QualType(extensionDecl->targetType.type); + expr->type.type = extensionDecl->targetType.type; return expr; } diff --git a/source/slang/core.meta.slang b/source/slang/core.meta.slang index 8626a832a..b1e6e0fd6 100644 --- a/source/slang/core.meta.slang +++ b/source/slang/core.meta.slang @@ -1235,3 +1235,6 @@ attribute_syntax [__vulkanRayPayload] : VulkanRayPayloadAttribute; __attributeTarget(VarDeclBase) attribute_syntax [__vulkanHitAttributes] : VulkanHitAttributesAttribute; + +__attributeTarget(FunctionDeclBase) +attribute_syntax [mutating] : MutatingAttribute; diff --git a/source/slang/core.meta.slang.h b/source/slang/core.meta.slang.h index 854d114bf..f0d61fa12 100644 --- a/source/slang/core.meta.slang.h +++ b/source/slang/core.meta.slang.h @@ -1253,3 +1253,6 @@ SLANG_RAW("attribute_syntax [__vulkanRayPayload] : VulkanRayPayloadAttribute;\n" SLANG_RAW("\n") SLANG_RAW("__attributeTarget(VarDeclBase)\n") SLANG_RAW("attribute_syntax [__vulkanHitAttributes] : VulkanHitAttributesAttribute;\n") +SLANG_RAW("\n") +SLANG_RAW("__attributeTarget(FunctionDeclBase)\n") +SLANG_RAW("attribute_syntax [mutating] : MutatingAttribute;\n") diff --git a/source/slang/diagnostic-defs.h b/source/slang/diagnostic-defs.h index 8a9773ba9..1c27dde0e 100644 --- a/source/slang/diagnostic-defs.h +++ b/source/slang/diagnostic-defs.h @@ -204,7 +204,7 @@ DIAGNOSTIC(30035, Error, componentOverloadTypeMismatch, "'$0': type of overloade DIAGNOSTIC(30041, Error, bitOperationNonIntegral, "bit operation: operand must be integral type.") DIAGNOSTIC(30047, Error, argumentExpectedLValue, "argument passed to parameter '$0' must be l-value.") DIAGNOSTIC(30048, Note, implicitCastUsedAsLValue, "argument was implicitly cast from '$0' to '$1', and Slang does not support using an implicit cast as an l-value") -DIAGNOSTIC(30049, Note, thisIsImmutableByDefault, "a 'this' parameter is currently immutable in Slang") +DIAGNOSTIC(30049, Note, thisIsImmutableByDefault, "a 'this' parameter is an immutable parameter by default in Slang; apply the `[mutating]` attribute to the function declaration to opt in to a mutable `this`") DIAGNOSTIC(30051, Error, invalidValueForArgument, "invalid value for argument '$0'") DIAGNOSTIC(30052, Error, invalidSwizzleExpr, "invalid swizzle pattern '$0' on type '$1'") diff --git a/source/slang/lookup.cpp b/source/slang/lookup.cpp index 2735bc6ba..e4ae3c8bb 100644 --- a/source/slang/lookup.cpp +++ b/source/slang/lookup.cpp @@ -20,6 +20,7 @@ DeclRef<ExtensionDecl> ApplyExtensionToType( struct BreadcrumbInfo { LookupResultItem::Breadcrumb::Kind kind; + LookupResultItem::Breadcrumb::ThisParameterMode thisParameterMode = LookupResultItem::Breadcrumb::ThisParameterMode::Default; DeclRef<Decl> declRef; BreadcrumbInfo* prev = nullptr; }; @@ -161,7 +162,8 @@ LookupResultItem CreateLookupResultItem( breadcrumbs = new LookupResultItem::Breadcrumb( bb->kind, bb->declRef, - breadcrumbs); + breadcrumbs, + bb->thisParameterMode); } item.breadcrumbs = breadcrumbs; return item; @@ -433,6 +435,8 @@ void DoLookupImpl( LookupRequest const& request, LookupResult& result) { + auto thisParameterMode = LookupResultItem::Breadcrumb::ThisParameterMode::Default; + auto scope = request.scope; auto endScope = request.endScope; for (;scope != endScope; scope = scope->parent) @@ -464,6 +468,7 @@ void DoLookupImpl( if (auto aggTypeDeclRef = containerDeclRef.As<AggTypeDeclBase>()) { breadcrumb.kind = LookupResultItem::Breadcrumb::Kind::This; + breadcrumb.thisParameterMode = thisParameterMode; breadcrumb.declRef = aggTypeDeclRef; breadcrumb.prev = nullptr; @@ -488,6 +493,18 @@ void DoLookupImpl( DoLocalLookupImpl( session, name, containerDeclRef, request, result, breadcrumbs); + + if( auto funcDeclRef = containerDeclRef.As<FunctionDeclBase>() ) + { + if( funcDeclRef.getDecl()->HasModifier<MutatingAttribute>() ) + { + thisParameterMode = LookupResultItem::Breadcrumb::ThisParameterMode::Mutating; + } + else + { + thisParameterMode = LookupResultItem::Breadcrumb::ThisParameterMode::Default; + } + } } if (result.isValid()) diff --git a/source/slang/lower-to-ir.cpp b/source/slang/lower-to-ir.cpp index 7e22c4694..86cf63b25 100644 --- a/source/slang/lower-to-ir.cpp +++ b/source/slang/lower-to-ir.cpp @@ -4494,19 +4494,10 @@ struct DeclLoweringVisitor : DeclVisitor<DeclLoweringVisitor, LoweredValInfo> // parameter for the enclosing type: // void addThisParameter( + ParameterDirection direction, Type* type, ParameterLists* ioParameterLists) { - // For now we make any `this` parameter default to `in`. Eventually - // we should add a way for the user to opt in to mutability of `this` - // in cases where they really want it. - // - // Note: an alternative here might be to have the built-in types like - // `Texture2D` actually use `class` declarations and say that the - // `this` parameter for a class type is always `in`, while `struct` - // types can default to `in out`. - ParameterDirection direction = kParameterDirection_In; - ParameterInfo info; info.type = type; info.decl = nullptr; @@ -4516,6 +4507,7 @@ struct DeclLoweringVisitor : DeclVisitor<DeclLoweringVisitor, LoweredValInfo> ioParameterLists->params.Add(info); } void addThisParameter( + ParameterDirection direction, AggTypeDecl* typeDecl, ParameterLists* ioParameterLists) { @@ -4526,6 +4518,7 @@ struct DeclLoweringVisitor : DeclVisitor<DeclLoweringVisitor, LoweredValInfo> auto declRef = createDefaultSpecializedDeclRef(typeDecl); RefPtr<Type> type = DeclRefType::Create(context->getSession(), declRef); addThisParameter( + direction, type, ioParameterLists); } @@ -4558,13 +4551,26 @@ struct DeclLoweringVisitor : DeclVisitor<DeclLoweringVisitor, LoweredValInfo> // parameter corresponding to the outer declaration. if( innerMode != kParameterListCollectMode_Static ) { + // For now we make any `this` parameter default to `in`. + // + ParameterDirection direction = kParameterDirection_In; + // + // Applications can opt in to a mutable `this` parameter, + // by applying the `[mutating]` attribute to their + // declaration. + // + if( decl->HasModifier<MutatingAttribute>() ) + { + direction = kParameterDirection_InOut; + } + if( auto aggTypeDecl = dynamic_cast<AggTypeDecl*>(parentDecl) ) { - addThisParameter(aggTypeDecl, ioParameterLists); + addThisParameter(direction, aggTypeDecl, ioParameterLists); } else if( auto extensionDecl = dynamic_cast<ExtensionDecl*>(parentDecl) ) { - addThisParameter(extensionDecl->targetType, ioParameterLists); + addThisParameter(direction, extensionDecl->targetType, ioParameterLists); } } } diff --git a/source/slang/modifier-defs.h b/source/slang/modifier-defs.h index 35bd38e2e..dc4e99cdd 100644 --- a/source/slang/modifier-defs.h +++ b/source/slang/modifier-defs.h @@ -399,6 +399,12 @@ SIMPLE_SYNTAX_CLASS(VulkanRayPayloadAttribute, Attribute) // intersection shader to pass hit attribute information. SIMPLE_SYNTAX_CLASS(VulkanHitAttributesAttribute, Attribute) +// A `[mutating]` attribute, which indicates that a member +// function is allowed to modify things through its `this` +// argument. +// +SIMPLE_SYNTAX_CLASS(MutatingAttribute, Attribute) + // HLSL modifiers for geometry shader input topology SIMPLE_SYNTAX_CLASS(HLSLGeometryShaderInputPrimitiveTypeModifier, Modifier) SIMPLE_SYNTAX_CLASS(HLSLPointModifier , HLSLGeometryShaderInputPrimitiveTypeModifier) diff --git a/source/slang/syntax.h b/source/slang/syntax.h index 24660b7fd..019740a5e 100644 --- a/source/slang/syntax.h +++ b/source/slang/syntax.h @@ -864,7 +864,7 @@ namespace Slang class Breadcrumb : public RefObject { public: - enum class Kind + enum class Kind : uint8_t { // The lookup process looked "through" an in-scope // declaration to the fields inside of it, so that @@ -895,6 +895,16 @@ namespace Slang // The kind of lookup step that was performed Kind kind; + // For the `Kind::This` case, is the `this` parameter + // mutable or not? + enum class ThisParameterMode : uint8_t + { + Default, + Mutating, + }; + ThisParameterMode thisParameterMode = ThisParameterMode::Default; + + // As needed, a reference to the declaration that faciliated // the lookup step. // @@ -910,8 +920,13 @@ namespace Slang // arrive at a final value. RefPtr<Breadcrumb> next; - Breadcrumb(Kind kind, DeclRef<Decl> declRef, RefPtr<Breadcrumb> next) + Breadcrumb( + Kind kind, + DeclRef<Decl> declRef, + RefPtr<Breadcrumb> next, + ThisParameterMode thisParameterMode = ThisParameterMode::Default) : kind(kind) + , thisParameterMode(thisParameterMode) , declRef(declRef) , next(next) {} diff --git a/tests/compute/mutating-methods.slang b/tests/compute/mutating-methods.slang new file mode 100644 index 000000000..736d4f7e0 --- /dev/null +++ b/tests/compute/mutating-methods.slang @@ -0,0 +1,51 @@ +// mutating-methods.slang +//TEST(compute):COMPARE_COMPUTE_EX:-slang -compute +//TEST(compute):COMPARE_COMPUTE_EX:-slang -compute -dx12 +//TEST(compute, vulkan):COMPARE_COMPUTE_EX:-vk -compute + +interface IAccumulator +{ + [mutating] void accumulate(int v); +} + +struct Accumulator : IAccumulator +{ + int state; + + [mutating] void accumulate(int v) + { + state += v; + } + + [mutating] void accumulateMore(int v) + { + this.state += v; + } +} + +void doStuff<A : IAccumulator>(inout A a) +{ + a.accumulate(16); +} + +int test(int x) +{ + Accumulator a; + a.state = 0; + + a.accumulate(x); + a.accumulateMore(x); + doStuff(a); + + return a.state; +} + +//TEST_INPUT:ubuffer(data=[0 0 0 0], stride=4):dxbinding(0),glbinding(0),out +RWStructuredBuffer<int> outputBuffer; + +[numthreads(4, 1, 1)] +void computeMain(uint3 dispatchThreadID : SV_DispatchThreadID) +{ + int tid = dispatchThreadID.x; + outputBuffer[tid] = test(tid); +} diff --git a/tests/compute/mutating-methods.slang.expected.txt b/tests/compute/mutating-methods.slang.expected.txt new file mode 100644 index 000000000..128492da7 --- /dev/null +++ b/tests/compute/mutating-methods.slang.expected.txt @@ -0,0 +1,4 @@ +10 +12 +14 +16 diff --git a/tests/diagnostics/setter-method.slang.expected b/tests/diagnostics/setter-method.slang.expected index 237c499ee..0c2b5a2c9 100644 --- a/tests/diagnostics/setter-method.slang.expected +++ b/tests/diagnostics/setter-method.slang.expected @@ -1,9 +1,9 @@ result code = -1 standard error = { tests/diagnostics/setter-method.slang(16): error 30011: left of '=' is not an l-value. -tests/diagnostics/setter-method.slang(16): note 30049: a 'this' parameter is currently immutable in Slang +tests/diagnostics/setter-method.slang(16): note 30049: a 'this' parameter is an immutable parameter by default in Slang; apply the `[mutating]` attribute to the function declaration to opt in to a mutable `this` tests/diagnostics/setter-method.slang(21): error 30011: left of '=' is not an l-value. -tests/diagnostics/setter-method.slang(21): note 30049: a 'this' parameter is currently immutable in Slang +tests/diagnostics/setter-method.slang(21): note 30049: a 'this' parameter is an immutable parameter by default in Slang; apply the `[mutating]` attribute to the function declaration to opt in to a mutable `this` } standard output = { } |