From f9d99fde581c7dfdeb46e87f32da1fed8ac5441c Mon Sep 17 00:00:00 2001 From: Tim Foley Date: Thu, 20 Feb 2020 13:44:03 -0800 Subject: Initial support for user-defined initializer/constructor declarations (#1233) The basic idea is that the user can write: ```hlsl struct MyThing { int a; float b; __init(int x, float y) { a = x; b = y; } } ``` and after that point, they can create an intstance of their `MyThing` type as simply as `MyThing(123, 4.56f)`. There was already a large amount of infrastructure laying around that is shared between ininitializers and ordinary functions, so enabling this feature mostly amounted to tying up some loose ends: * In the parser, make sure to properly push/pop the scope for an `__init` (or `__subscript`) declaration, so parameters would be visible to the body * In semantic checking, make sure that declaration "header" checking properly bottlenecks all the function-like cases into a base routine * In semantic checking, make sure that the logic for checking function bodies applies to every `FunctionDeclBase` with a body, and not just `FuncDecl`s * Update semeantic checking for statements to allow for any `FunctionDeclBase` as the parent declaration, not just a `FuncDecl` * In lookup, treat the `this` parameter of an `__init` (well, not actually a *parameter* in this case) as being mutable, just like for a `[mutating]` method * In IR codegen, don't just assume that all `__init`s are intrinsics, and narrow the scope of that hack to just `__init`s without bodies * In IR codegen, detect when we are emitting an IR function for an `__init`, and in that case create a local variable to represent the `this` value, and implicitly return that value at the end of the body. From that point on the rest of the compiler Just Works and IR codegen doesn't have to think of an `__init` as being any different than if the user had declared a `static MyThing make(...)` function. Caveats: * C++ users might like to use that naming convention (so `MyThing` as the name instead of `__init`). We can consider that later. * Everybody else might prefer a keyword other than `__init` (e.g., just `init` as in Swift), but I'm keeping this as a "preview" feature for now, rather than something officially supported * Early `return`s from the body of an `__init` aren't going to work right now. * There is currently no provision for automatically synthesizing initializers for `struct` types based on their fields. This seems like a reasonable direction to take in the future. * There is no provision for routing `{}`-based initializer lists over to initializer calls. The two syntaxes probably need to be unified at some point so that doing `MyType x = { a, b, c }` and `let x = MyType(a, b, c)` are semantically equivalent. It is possible that as a byproduct of this change user-defined `__subscript`s might Just Work, but I am guessing there will still be loose ends on that front as well, so I will refrain from looking into that feature until we have a use case that calls for it. --- source/slang/slang-check-decl.cpp | 40 +++++++------ source/slang/slang-check-impl.h | 13 ++--- source/slang/slang-check-stmt.cpp | 2 +- source/slang/slang-check.cpp | 2 +- source/slang/slang-lookup.cpp | 6 +- source/slang/slang-lower-to-ir.cpp | 68 ++++++++++++++++++---- source/slang/slang-parser.cpp | 4 ++ tests/compute/user-defined-initializer.slang | 40 +++++++++++++ .../user-defined-initializer.slang.expected.txt | 4 ++ 9 files changed, 141 insertions(+), 38 deletions(-) create mode 100644 tests/compute/user-defined-initializer.slang create mode 100644 tests/compute/user-defined-initializer.slang.expected.txt diff --git a/source/slang/slang-check-decl.cpp b/source/slang/slang-check-decl.cpp index a37d80ae8..99400c17c 100644 --- a/source/slang/slang-check-decl.cpp +++ b/source/slang/slang-check-decl.cpp @@ -70,6 +70,8 @@ namespace Slang void visitAssocTypeDecl(AssocTypeDecl* decl); + void checkCallableDeclCommon(CallableDecl* decl); + void visitFuncDecl(FuncDecl* funcDecl); void visitParamDecl(ParamDecl* paramDecl); @@ -149,7 +151,7 @@ namespace Slang void visitEnumDecl(EnumDecl* decl); - void visitFuncDecl(FuncDecl* funcDecl); + void visitFunctionDeclBase(FunctionDeclBase* funcDecl); void visitParamDecl(ParamDecl* paramDecl); }; @@ -1990,11 +1992,11 @@ namespace Slang getSink()->diagnose(decl, Slang::Diagnostics::assocTypeInInterfaceOnly); } - void SemanticsDeclBodyVisitor::visitFuncDecl(FuncDecl* funcDecl) + void SemanticsDeclBodyVisitor::visitFunctionDeclBase(FunctionDeclBase* decl) { - if (auto body = funcDecl->Body) + if (auto body = decl->Body) { - checkBodyStmt(body, funcDecl); + checkBodyStmt(body, decl); } } @@ -2546,6 +2548,14 @@ namespace Slang } } + void SemanticsDeclHeaderVisitor::checkCallableDeclCommon(CallableDecl* decl) + { + for(auto& paramDecl : decl->GetParameters()) + { + ensureDecl(paramDecl, DeclCheckState::ReadyForReference); + } + } + void SemanticsDeclHeaderVisitor::visitFuncDecl(FuncDecl* funcDecl) { auto resultType = funcDecl->ReturnType; @@ -2559,10 +2569,7 @@ namespace Slang } funcDecl->ReturnType = resultType; - for (auto& para : funcDecl->GetParameters()) - { - ensureDecl(para, DeclCheckState::ReadyForReference); - } + checkCallableDeclCommon(funcDecl); } IntegerLiteralValue SemanticsVisitor::GetMinBound(RefPtr val) @@ -2700,6 +2707,7 @@ namespace Slang // significant, and we need to make a choice // sooner or later. // + ensureDecl(extDeclRef, DeclCheckState::CanUseExtensionTargetType); auto targetType = GetTargetType(extDeclRef); return calcThisType(targetType); } @@ -2749,23 +2757,15 @@ namespace Slang void SemanticsDeclHeaderVisitor::visitConstructorDecl(ConstructorDecl* decl) { - for (auto& paramDecl : decl->GetParameters()) - { - ensureDecl(paramDecl, DeclCheckState::CanUseTypeOfValueDecl); - } - // We need to compute the result tyep for this declaration, // since it wasn't filled in for us. decl->ReturnType.type = findResultTypeForConstructorDecl(decl); + + checkCallableDeclCommon(decl); } void SemanticsDeclHeaderVisitor::visitSubscriptDecl(SubscriptDecl* decl) { - for (auto& paramDecl : decl->GetParameters()) - { - ensureDecl(paramDecl, DeclCheckState::CanUseTypeOfValueDecl); - } - decl->ReturnType = CheckUsableType(decl->ReturnType); // If we have a subscript declaration with no accessor declarations, @@ -2789,6 +2789,8 @@ namespace Slang getterDecl->ParentDecl = decl; decl->Members.add(getterDecl); } + + checkCallableDeclCommon(decl); } void SemanticsDeclHeaderVisitor::visitAccessorDecl(AccessorDecl* decl) @@ -2808,6 +2810,8 @@ namespace Slang { getSink()->diagnose(decl, Diagnostics::accessorMustBeInsideSubscriptOrProperty); } + + checkCallableDeclCommon(decl); } GenericDecl* SemanticsVisitor::GetOuterGeneric(Decl* decl) diff --git a/source/slang/slang-check-impl.h b/source/slang/slang-check-impl.h index 863b5f38a..764f0526a 100644 --- a/source/slang/slang-check-impl.h +++ b/source/slang/slang-check-impl.h @@ -408,7 +408,7 @@ namespace Slang // so that we can add some quality-of-life features for users // in cases where the compiler crashes // - void dispatchStmt(Stmt* stmt, FuncDecl* parentFunc, OuterStmtInfo* outerStmts); + void dispatchStmt(Stmt* stmt, FunctionDeclBase* parentFunc, OuterStmtInfo* outerStmts); void dispatchExpr(Expr* expr); /// Ensure that a declaration has been checked up to some state @@ -781,7 +781,7 @@ namespace Slang // as the tag type for an `enum` void validateEnumTagType(Type* type, SourceLoc const& loc); - void checkStmt(Stmt* stmt, FuncDecl* outerFunction, OuterStmtInfo* outerStmts); + void checkStmt(Stmt* stmt, FunctionDeclBase* outerFunction, OuterStmtInfo* outerStmts); void getGenericParams( GenericDecl* decl, @@ -1369,20 +1369,19 @@ namespace Slang : public SemanticsVisitor , StmtVisitor { - SemanticsStmtVisitor(SharedSemanticsContext* shared, FuncDecl* parentFunc, OuterStmtInfo* outerStmts) + SemanticsStmtVisitor(SharedSemanticsContext* shared, FunctionDeclBase* parentFunc, OuterStmtInfo* outerStmts) : SemanticsVisitor(shared) , m_parentFunc(parentFunc) , m_outerStmts(outerStmts) {} /// The parent function (if any) that surrounds the statement being checked. - // TODO: This should probably be a more general case like `CallableDecl` - FuncDecl* m_parentFunc = nullptr; + FunctionDeclBase* m_parentFunc = nullptr; /// The linked list of lexically surrounding statements. OuterStmtInfo* m_outerStmts = nullptr; - FuncDecl* getParentFunc() { return m_parentFunc; } + FunctionDeclBase* getParentFunc() { return m_parentFunc; } void checkStmt(Stmt* stmt); @@ -1433,7 +1432,7 @@ namespace Slang : SemanticsVisitor(shared) {} - void checkBodyStmt(Stmt* stmt, FuncDecl* parentDecl) + void checkBodyStmt(Stmt* stmt, FunctionDeclBase* parentDecl) { checkStmt(stmt, parentDecl, nullptr); } diff --git a/source/slang/slang-check-stmt.cpp b/source/slang/slang-check-stmt.cpp index 7a157a1fd..58d301549 100644 --- a/source/slang/slang-check-stmt.cpp +++ b/source/slang/slang-check-stmt.cpp @@ -35,7 +35,7 @@ namespace Slang }; } - void SemanticsVisitor::checkStmt(Stmt* stmt, FuncDecl* parentDecl, OuterStmtInfo* outerStmts) + void SemanticsVisitor::checkStmt(Stmt* stmt, FunctionDeclBase* parentDecl, OuterStmtInfo* outerStmts) { if (!stmt) return; dispatchStmt(stmt, parentDecl, outerStmts); diff --git a/source/slang/slang-check.cpp b/source/slang/slang-check.cpp index 990f9a0f5..dd28a9d71 100644 --- a/source/slang/slang-check.cpp +++ b/source/slang/slang-check.cpp @@ -223,7 +223,7 @@ namespace Slang translationUnit->getModule()->_collectShaderParams(); } - void SemanticsVisitor::dispatchStmt(Stmt* stmt, FuncDecl* parentFunc, OuterStmtInfo* outerStmts) + void SemanticsVisitor::dispatchStmt(Stmt* stmt, FunctionDeclBase* parentFunc, OuterStmtInfo* outerStmts) { SemanticsStmtVisitor visitor(getShared(), parentFunc, outerStmts); try diff --git a/source/slang/slang-lookup.cpp b/source/slang/slang-lookup.cpp index e860d91d1..49f0bbf0c 100644 --- a/source/slang/slang-lookup.cpp +++ b/source/slang/slang-lookup.cpp @@ -518,7 +518,11 @@ void DoLookupImpl( session, name, containerDeclRef, request, result, breadcrumbs); - if( auto funcDeclRef = containerDeclRef.as() ) + if( containerDeclRef.is() ) + { + thisParameterMode = LookupResultItem::Breadcrumb::ThisParameterMode::Mutating; + } + else if( auto funcDeclRef = containerDeclRef.as() ) { if( funcDeclRef.getDecl()->HasModifier() ) { diff --git a/source/slang/slang-lower-to-ir.cpp b/source/slang/slang-lower-to-ir.cpp index fdbb2cb27..360376b73 100644 --- a/source/slang/slang-lower-to-ir.cpp +++ b/source/slang/slang-lower-to-ir.cpp @@ -804,17 +804,34 @@ LoweredValInfo emitCallToDeclRef( if( auto ctorDeclRef = funcDeclRef.as() ) { - // HACK: we know all constructors are builtins for now, - // so we need to emit them as a call to the corresponding - // builtin operation. - // - // TODO: these should all either be intrinsic operations, - // or calls to library functions. - - return LoweredValInfo::simple(builder->emitConstructorInst(type, argCount, args)); + if(!ctorDeclRef.getDecl()->Body) + { + // HACK: For legacy reasons, all of the built-in initializers + // in the standard library are declared without proper + // intrinsic-op modifiers, so we will assume that an + // initializer without a body should map to `kIROp_Construct`. + // + // TODO: We should make all the initializers in the + // standard library have either a body or a proper + // intrinsic-op modifier. + // + // TODO: We should eliminate `kIROp_Construct` from the + // IR completely, in favor of more detailed/specific ops + // that cover the cases we actually care about. + // + return LoweredValInfo::simple(builder->emitConstructorInst(type, argCount, args)); + } } // Fallback case is to emit an actual call. + // + // TODO: We are constructing a type that we expect the function + // being called to have here, but that type doesn't account + // for `in` vs. `out`/`inout` parameters, so it could easily + // be wrong. We should sort out why this path in the code + // even needs to be computing a type (rather than taking + // it directly from the declaration). + // if(!funcType) { List argTypes; @@ -6175,15 +6192,46 @@ struct DeclLoweringVisitor : DeclVisitor } } - // Lower body + // We will now set about emitting the code for the body of + // the function/callable. + // + // In the case of an initializer ("constructor") declaration, + // the `this` value is not a parameter, but rather a placeholder + // for the value that will be returned. We thus need to set up + // a local variable to represent this value. + // + auto constructorDecl = as(decl); + if(constructorDecl) + { + auto thisVar = subContext->irBuilder->emitVar(irResultType); + subContext->thisVal = LoweredValInfo::ptr(thisVar); + } + + // We lower whatever statement was stored on the declaration + // as the body of the new IR function. + // lowerStmt(subContext, decl->Body); // We need to carefully add a terminator instruction to the end // of the body, in case the user didn't do so. + // if (!subContext->irBuilder->getBlock()->getTerminator()) { - if(as(irResultType)) + if(constructorDecl) + { + // A constructor declaration should return the + // value of the `this` variable that was set + // up at the start. + // + // TODO: This should also apply if any code + // path in an initializer/constructor attempts + // to do an early `return;`. + // + subContext->irBuilder->emitReturn( + getSimpleVal(subContext, subContext->thisVal)); + } + else if(as(irResultType)) { // `void`-returning function can get an implicit // return on exit of the body statement. diff --git a/source/slang/slang-parser.cpp b/source/slang/slang-parser.cpp index c5420a936..e5a1ad576 100644 --- a/source/slang/slang-parser.cpp +++ b/source/slang/slang-parser.cpp @@ -2447,6 +2447,7 @@ namespace Slang { RefPtr decl = new ConstructorDecl(); parser->FillPosition(decl.Ptr()); + parser->PushScope(decl); // TODO: we need to make sure that all initializers have // the same name, but that this name doesn't conflict @@ -2462,6 +2463,7 @@ namespace Slang decl->Body = parseOptBody(parser); + parser->PopScope(); return decl; } @@ -2506,6 +2508,7 @@ namespace Slang { RefPtr decl = new SubscriptDecl(); parser->FillPosition(decl.Ptr()); + parser->PushScope(decl); // TODO: the use of this name here is a bit magical... decl->nameAndLoc.name = getName(parser, "operator[]"); @@ -2533,6 +2536,7 @@ namespace Slang // empty body should be treated like `{ get; }` } + parser->PopScope(); return decl; } diff --git a/tests/compute/user-defined-initializer.slang b/tests/compute/user-defined-initializer.slang new file mode 100644 index 000000000..8f7492881 --- /dev/null +++ b/tests/compute/user-defined-initializer.slang @@ -0,0 +1,40 @@ +// user-defined-initializer.slang + +// Confirm that user-defined initializer/constructor +// methods in a type work as expected. + +//TEST(compute):COMPARE_COMPUTE: +//TEST(compute):COMPARE_COMPUTE:-cpu + +struct Pair +{ + int head; + int tail; + + __init(int h, int t) + { + head = h; + tail = t; + } + + int getHead() { return head; } + int getTail() { return tail; } +} + +int test(int value) +{ + Pair p = Pair(value, value+1); + return p.getHead()*16 + p.getTail(); +} + +//TEST_INPUT:ubuffer(data=[0 1 2 3], stride=4):out,name=outputBuffer +RWStructuredBuffer outputBuffer : register(u0); + +[numthreads(4, 1, 1)] +void computeMain(uint3 dispatchThreadID : SV_DispatchThreadID) +{ + uint tid = dispatchThreadID.x; + int inVal = outputBuffer[tid]; + int outVal = test(inVal); + outputBuffer[tid] = outVal; +} \ No newline at end of file diff --git a/tests/compute/user-defined-initializer.slang.expected.txt b/tests/compute/user-defined-initializer.slang.expected.txt new file mode 100644 index 000000000..98be8c82f --- /dev/null +++ b/tests/compute/user-defined-initializer.slang.expected.txt @@ -0,0 +1,4 @@ +1 +12 +23 +34 -- cgit v1.2.3