diff options
| author | Tim Foley <tfoleyNV@users.noreply.github.com> | 2020-06-12 13:30:32 -0700 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2020-06-12 13:30:32 -0700 |
| commit | 36a06f1289c9a68a261920ef5d34f075f2a43219 (patch) | |
| tree | dd11eba962d87da0d437a752b818ddc68f5b6603 /source/slang/slang-check-decl.cpp | |
| parent | 2359921bb7aba569b36ce3c1904b2dccbde5ffec (diff) | |
Diagnose circularly-defined constants (#1384)
* Diagnose circularly-defined constants
Work on #1374
This change diagnoses cases like the following:
```hlsl
static const int kCircular = kCircular;
static const int kInfinite = kInfinite + 1;
static const int kHere = kThere;
static const int kThere = kHere;
```
By diagnosing these as errors in the front-end we protect against infinite recursion leading to stack overflow crashes.
The basic approach is to have front-end constant folding track variables that are in use when folding a sub-expression, and then diagnosing an error if the same variable is encountered again while it is in use. In order to make sure the error occurs whether or not the constant is referenced, we invoke constant folding on all `static const` integer variables.
Limitations:
* This only works for integers, since that is all front-end constant folding applies to. A future change can/should catch circularity in constants at the IR level (and handle more types).
* This only works for constants. Circular references in the definition of a global variable are harder to diagnose, but at least shouldn't result in compiler crashes.
* This doesn't work across modules, or through generic specialization: anything that requires global knowledge won't be checked
* fixup: missing files
* fixup: review feedback
Diffstat (limited to 'source/slang/slang-check-decl.cpp')
| -rw-r--r-- | source/slang/slang-check-decl.cpp | 54 |
1 files changed, 43 insertions, 11 deletions
diff --git a/source/slang/slang-check-decl.cpp b/source/slang/slang-check-decl.cpp index b69c8cf0d..3aeca23cd 100644 --- a/source/slang/slang-check-decl.cpp +++ b/source/slang/slang-check-decl.cpp @@ -775,6 +775,24 @@ namespace Slang return true; } + void SemanticsVisitor::_validateCircularVarDefinition(VarDeclBase* varDecl) + { + // The easiest way to test if the declaration is circular is to + // validate it as a constant. + // + // TODO: The logic here will only apply for `static const` declarations + // of integer type, given that our constant folding currently only + // applies to such types. A more robust fix would involve a truly + // recursive walk of the AST declarations, and an even *more* robust + // fix would wait until after IR linking to detect and diagnose circularity + // in case it crosses module boundaries. + // + // + if(!isScalarIntegerType(varDecl->type)) + return; + tryConstantFoldDeclRef(DeclRef<VarDeclBase>(varDecl, nullptr), nullptr); + } + void SemanticsDeclHeaderVisitor::checkVarDeclCommon(VarDeclBase* varDecl) { // A variable that didn't have an explicit type written must @@ -804,6 +822,8 @@ namespace Slang varDecl->initExpr = initExpr; varDecl->type.type = initExpr->type; + + _validateCircularVarDefinition(varDecl); } // If we've gone down this path, then the variable @@ -857,11 +877,16 @@ namespace Slang { // If the variable has an explicit initial-value expression, // then we simply need to check that expression and coerce - // it to the tyep of the variable. + // it to the type of the variable. // initExpr = CheckTerm(initExpr); initExpr = coerce(varDecl->type.Ptr(), initExpr); varDecl->initExpr = initExpr; + + // We need to ensure that any variable doesn't introduce + // a constant with a circular definition. + // + _validateCircularVarDefinition(varDecl); } else { @@ -1970,18 +1995,25 @@ namespace Slang return (BaseTypeInfo::getInfo(baseType).flags & BaseTypeInfo::Flag::Integer) != 0; } - void SemanticsVisitor::validateEnumTagType(Type* type, SourceLoc const& loc) + bool SemanticsVisitor::isScalarIntegerType(Type* type) { - if(auto basicType = as<BasicExpressionType>(type)) - { - // Allow the built-in integer types. - if(isIntegerBaseType(basicType->baseType)) - return; + auto basicType = as<BasicExpressionType>(type); + if(!basicType) + return false; - // By default, don't allow other types to be used - // as an `enum` tag type. - } + return isIntegerBaseType(basicType->baseType); + } + void SemanticsVisitor::validateEnumTagType(Type* type, SourceLoc const& loc) + { + // Allow the built-in integer types. + // + if(isScalarIntegerType(type)) + return; + + // By default, don't allow other types to be used + // as an `enum` tag type. + // getSink()->diagnose(loc, Diagnostics::invalidEnumTagType, type); } @@ -2177,7 +2209,7 @@ namespace Slang // the tag value for a successor case that doesn't // provide an explicit tag. - IntVal* explicitTagVal = TryConstantFoldExpr(explicitTagValExpr); + IntVal* explicitTagVal = tryConstantFoldExpr(explicitTagValExpr, nullptr); if(explicitTagVal) { if(auto constIntVal = as<ConstantIntVal>(explicitTagVal)) |
