diff options
| author | Tim Foley <tfoleyNV@users.noreply.github.com> | 2021-03-10 15:18:06 -0800 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2021-03-10 15:18:06 -0800 |
| commit | 6cbd9d68a03f0a22305d4e224a3da7633b23de38 (patch) | |
| tree | de436717081a9b2b7ddd3644f2e7ada130951141 /source/slang/slang-check-expr.cpp | |
| parent | 6ef4054f8a8aea4ec61481057fa7e16aaecde6d7 (diff) | |
A bunch of overlapping semantic-checking fixes (#1743)
This change originally started with the simple goal of allowing generic functions with default argument values on their parameters to work:
```
void someFunction<T>(T value, int optional = 0);
```
The core problem there was that the compiler code was (correctly) anticipate the case where the default argument value for a parameter depends on a generic parameter, such as:
```
interface IDefaultable { static This getDefault(); }
void anotherFunction<T : IDefaultable>(T first, T second = T.getDefault());
```
Supporting this latter case requires some kind of ability to apply subsitutions to an `Expr`, but our compiler logic simply errored out in that case. The first major fix that went into this change was to add a new `SubstExpr<T>` type that behaves a lot like `DeclRef<T>` in that it stores a `T*` plus a set of substititions that need to be applied to it.
In addition, it was found that even if `anotherFunction<ConcreteType>(...)` might work, when generic argument inference was used for just `anotherFunction(...)` would fail because it includes a strict match on the number of arguments/parameters in the call expression.
The next problem that arose was that the test I'd created used an interace with an `__init` requirement, and it appeared that our code generation didn't work for that case:
```
interface IStuff { __init(int val); }
void f<T : IStuff>(T x = T(0));
```
In this case, the `T(0)` initialization would get compiled to `(ConcreteType) 0` in the output rather than calling the function generated for the `__init` inside `ConcreteType`. The basic problem there was a bit of crufty old logic we have in place to work around the large number of `__init` declarations in the stdlib that don't have proper `__intrinsic_op` modifiers on them. We really need to fix the underlying problem there, but I worked around it by having the IR lowering pass only do its workaround magic on stdlib declarations.
The next problem down this line was that my test had two different `__init` declarations in the concrete type and the logic for checking interface conformance was picking the wrong one to satisfying an interface requirement despite it being obviously wrong (not even the right number of parameter).
This last problem led me down the rabbit-hole of trying to actually get our semantic checking for interface requirements right. There were a few pieces to that work:
* Actually checking that the parameter and result types for two callables match is the simple part. If that was all that would be required we would have implement this logic a long time ago.
* Next we have to deal with functions that make use of the `This` type, associated types, etc. We have to know that when the interface uses `This`, we want to treat that as equivalent to `ConcreteType`, and similarly for associated types. Getting that working is mostly a matter of setting up a this-type subsitution for the interface member being checked.
* Finally, when comparing generic declarations like `IBase::doThing<T>` and `Derived::doThing<U>` we need to deal with the way that `T` and `U` represent the "same" logical type parameter, but are distinct `Decl`s. This is handled by specializing the base declaration to the parameters of the derived one (e.g., forming `IBase::doThing<U>` using the `U` from `Derived::doThing`).
The result seems to be passing our tests, but there are still a few gotchas lurking, I'm sure.
Diffstat (limited to 'source/slang/slang-check-expr.cpp')
| -rw-r--r-- | source/slang/slang-check-expr.cpp | 46 |
1 files changed, 25 insertions, 21 deletions
diff --git a/source/slang/slang-check-expr.cpp b/source/slang/slang-check-expr.cpp index 5c00cb64b..fbbfcd473 100644 --- a/source/slang/slang-check-expr.cpp +++ b/source/slang/slang-check-expr.cpp @@ -694,7 +694,7 @@ namespace Slang } IntVal* SemanticsVisitor::tryConstantFoldExpr( - InvokeExpr* invokeExpr, + SubstExpr<InvokeExpr> invokeExpr, ConstantFoldingCircularityInfo* circularityInfo) { // We need all the operands to the expression @@ -703,10 +703,10 @@ namespace Slang // // For right now we will look for calls to intrinsic functions, and then inspect // their names (this is bad and slow). - auto funcDeclRefExpr = as<DeclRefExpr>(invokeExpr->functionExpr); + auto funcDeclRefExpr = getBaseExpr(invokeExpr).as<DeclRefExpr>(); if (!funcDeclRefExpr) return nullptr; - auto funcDeclRef = funcDeclRefExpr->declRef; + auto funcDeclRef = getDeclRef(m_astBuilder, funcDeclRefExpr); auto intrinsicMod = funcDeclRef.getDecl()->findModifier<IntrinsicOpModifier>(); if (!intrinsicMod) { @@ -722,31 +722,31 @@ namespace Slang // Let's not constant-fold operations with more than a certain number of arguments, for simplicity static const int kMaxArgs = 8; - if (invokeExpr->arguments.getCount() > kMaxArgs) + auto argCount = getArgCount(invokeExpr); + if (argCount > kMaxArgs) return nullptr; // Before checking the operation name, let's look at the arguments IntVal* argVals[kMaxArgs]; IntegerLiteralValue constArgVals[kMaxArgs]; - int argCount = 0; bool allConst = true; - for (auto argExpr : invokeExpr->arguments) + for(Index a = 0; a < argCount; ++a) { + auto argExpr = getArg(invokeExpr, a); auto argVal = tryFoldIntegerConstantExpression(argExpr, circularityInfo); if (!argVal) return nullptr; - argVals[argCount] = argVal; + argVals[a] = argVal; if (auto constArgVal = as<ConstantIntVal>(argVal)) { - constArgVals[argCount] = constArgVal->value; + constArgVals[a] = constArgVal->value; } else { allConst = false; } - argCount++; } if (!allConst) @@ -866,25 +866,25 @@ namespace Slang } IntVal* SemanticsVisitor::tryConstantFoldExpr( - Expr* expr, + SubstExpr<Expr> expr, ConstantFoldingCircularityInfo* circularityInfo) { // Unwrap any "identity" expressions - while (auto parenExpr = as<ParenExpr>(expr)) + while (auto parenExpr = expr.as<ParenExpr>()) { - expr = parenExpr->base; + expr = getBaseExpr(parenExpr); } // TODO(tfoley): more serious constant folding here - if (auto intLitExpr = as<IntegerLiteralExpr>(expr)) + if (auto intLitExpr = expr.as<IntegerLiteralExpr>()) { return getIntVal(intLitExpr); } // it is possible that we are referring to a generic value param - if (auto declRefExpr = as<DeclRefExpr>(expr)) + if (auto declRefExpr = expr.as<DeclRefExpr>()) { - auto declRef = declRefExpr->declRef; + auto declRef = getDeclRef(m_astBuilder, declRefExpr); if (auto genericValParamRef = declRef.as<GenericValueParamDecl>()) { @@ -913,13 +913,13 @@ namespace Slang } } - if(auto castExpr = as<TypeCastExpr>(expr)) + if(auto castExpr = expr.as<TypeCastExpr>()) { - auto val = tryConstantFoldExpr(castExpr->arguments[0], circularityInfo); + auto val = tryConstantFoldExpr(getArg(castExpr, 0), circularityInfo); if(val) return val; } - else if (auto invokeExpr = as<InvokeExpr>(expr)) + else if (auto invokeExpr = expr.as<InvokeExpr>()) { auto val = tryConstantFoldExpr(invokeExpr, circularityInfo); if (val) @@ -930,12 +930,12 @@ namespace Slang } IntVal* SemanticsVisitor::tryFoldIntegerConstantExpression( - Expr* expr, + SubstExpr<Expr> expr, ConstantFoldingCircularityInfo* circularityInfo) { // Check if type is acceptable for an integer constant expression // - if(!isScalarIntegerType(expr->type)) + if(!isScalarIntegerType(getType(m_astBuilder, expr))) return nullptr; // Consider operations that we might be able to constant-fold... @@ -2037,7 +2037,11 @@ namespace Slang { auto containerDecl = scope->containerDecl; - if( auto setterDecl = as<SetterDecl>(containerDecl) ) + if( auto ctorDecl = as<ConstructorDecl>(containerDecl) ) + { + expr->type.isLeftValue = true; + } + else if( auto setterDecl = as<SetterDecl>(containerDecl) ) { expr->type.isLeftValue = true; } |
