From 3b0de8b6ea484091146f61e663c63beeac5b4798 Mon Sep 17 00:00:00 2001 From: Yong He Date: Wed, 15 May 2024 18:07:36 -0700 Subject: Add diagnostic to prevent defining unsized variables. (#4168) * Add diagnostic to prevent defining unsized static variables. * Fix tests. * Add more tests. * Fix to allow defining variables of link-time size. * update diagnostic message. * Fix tests. * Simplify code. --- source/slang/slang-ast-decl.h | 3 +- source/slang/slang-check-conformance.cpp | 3 +- source/slang/slang-check-decl.cpp | 23 ++++-- source/slang/slang-check-overload.cpp | 19 ++++- source/slang/slang-diagnostic-defs.h | 5 +- source/slang/slang-lower-to-ir.cpp | 133 +++++++++++++++++-------------- source/slang/slang-syntax.cpp | 2 + 7 files changed, 120 insertions(+), 68 deletions(-) (limited to 'source') diff --git a/source/slang/slang-ast-decl.h b/source/slang/slang-ast-decl.h index f7f537ed6..8f12a0f61 100644 --- a/source/slang/slang-ast-decl.h +++ b/source/slang/slang-ast-decl.h @@ -137,7 +137,8 @@ enum class TypeTag { None = 0, Unsized = 1, - Incomplete = 2 + Incomplete = 2, + LinkTimeSized = 4, }; // Declaration of a type that represents some sort of aggregate diff --git a/source/slang/slang-check-conformance.cpp b/source/slang/slang-check-conformance.cpp index a0e51b180..394d0693f 100644 --- a/source/slang/slang-check-conformance.cpp +++ b/source/slang/slang-check-conformance.cpp @@ -274,7 +274,8 @@ namespace Slang } else if (auto intVal = arrayType->getElementCount()) { - sized = !intVal->isLinkTimeVal(); + sized = true; + typeTag = (TypeTag)((int)typeTag | (int)TypeTag::LinkTimeSized); } if (!sized) typeTag = (TypeTag)((int)typeTag | (int)TypeTag::Unsized); diff --git a/source/slang/slang-check-decl.cpp b/source/slang/slang-check-decl.cpp index 875fddf2f..9a4ee9d71 100644 --- a/source/slang/slang-check-decl.cpp +++ b/source/slang/slang-check-decl.cpp @@ -2040,11 +2040,14 @@ namespace Slang } } - if (auto parentDecl = as(getParentDecl(varDecl))) + TypeTag varTypeTags = getTypeTags(varDecl->getType()); + auto parentDecl = as(getParentDecl(varDecl)); + if (parentDecl) { - auto typeTags = getTypeTags(varDecl->getType()); - parentDecl->addTag(typeTags); - if ((int)typeTags & (int)TypeTag::Unsized) + parentDecl->addTag(varTypeTags); + auto unsizedMask = (int)TypeTag::Unsized; + bool isUnknownSize = (((int)varTypeTags & unsizedMask) != 0); + if (isUnknownSize) { // Unsized decl must appear as the last member of the struct. for (auto memberIdx = parentDecl->members.getCount() - 1; memberIdx >= 0; memberIdx--) @@ -2064,7 +2067,17 @@ namespace Slang } } } - + bool isGlobalOrLocalVar = !isGlobalShaderParameter(varDecl) && !as(varDecl) && + (!parentDecl || isEffectivelyStatic(varDecl)); + if (isGlobalOrLocalVar) + { + bool isUnsized = (((int)varTypeTags & (int)TypeTag::Unsized) != 0); + if (isUnsized) + { + getSink()->diagnose(varDecl, Diagnostics::varCannotBeUnsized); + } + } + if (auto elementType = getConstantBufferElementType(varDecl->getType())) { if (doesTypeHaveTag(elementType, TypeTag::Incomplete)) diff --git a/source/slang/slang-check-overload.cpp b/source/slang/slang-check-overload.cpp index 6b51493e1..367c200cc 100644 --- a/source/slang/slang-check-overload.cpp +++ b/source/slang/slang-check-overload.cpp @@ -263,6 +263,14 @@ namespace Slang // bool success = true; + auto maybeReportGeneralError = [&]() + { + if (context.mode != OverloadResolveContext::Mode::JustTrying) + { + getSink()->diagnose(context.loc, Diagnostics::cannotSpecializeGeneric, candidate.item.declRef); + } + }; + Index aa = 0; for (auto memberRef : getMembers(m_astBuilder, genericDeclRef)) { @@ -287,7 +295,10 @@ namespace Slang // or this reference is ill-formed. auto substType = typeParamRef.substitute(m_astBuilder, typeParamRef.getDecl()->initType.type); if (!substType) + { + maybeReportGeneralError(); return false; + } checkedArgs.add(substType); continue; } @@ -355,7 +366,10 @@ namespace Slang ConstantFoldingCircularityInfo newCircularityInfo(valParamRef.getDecl(), nullptr); auto defaultVal = tryConstantFoldExpr(valParamRef.substitute(m_astBuilder, valParamRef.getDecl()->initExpr), ConstantFoldingKind::CompileTime, &newCircularityInfo); if (!defaultVal) + { + maybeReportGeneralError(); return false; + } checkedArgs.add(defaultVal); continue; } @@ -475,8 +489,9 @@ namespace Slang break; case OverloadCandidate::Flavor::Generic: - return TryCheckGenericOverloadCandidateTypes(context, candidate); - + { + return TryCheckGenericOverloadCandidateTypes(context, candidate); + } default: SLANG_UNEXPECTED("unknown flavor of overload candidate"); break; diff --git a/source/slang/slang-diagnostic-defs.h b/source/slang/slang-diagnostic-defs.h index c2c4953e0..85989b26d 100644 --- a/source/slang/slang-diagnostic-defs.h +++ b/source/slang/slang-diagnostic-defs.h @@ -325,7 +325,10 @@ DIAGNOSTIC(30066, Error, classCanOnlyBeInitializedWithNew, "a class can only be DIAGNOSTIC(30067, Error, mutatingMethodOnFunctionInputParameterError, "mutating method '$0' called on `in` parameter '$1'; changes will not be visible to caller. copy the parameter into a local variable if this behavior is intended") DIAGNOSTIC(30068, Warning, mutatingMethodOnFunctionInputParameterWarning, "mutating method '$0' called on `in` parameter '$1'; changes will not be visible to caller. copy the parameter into a local variable if this behavior is intended") -DIAGNOSTIC(30070, Error, unsizedMemberMustAppearLast, "unsized member can only appear as the last member in a composite type.") +DIAGNOSTIC(30070, Error, unsizedMemberMustAppearLast, "member with unknown size at compile time can only appear as the last member in a composite type.") +DIAGNOSTIC(30071, Error, varCannotBeUnsized, "cannot instantiate a variable of unsized type.") + +DIAGNOSTIC(30075, Error, cannotSpecializeGeneric, "cannot specialize generic '$0' with the provided arguments.") DIAGNOSTIC(30100, Error, staticRefToNonStaticMember, "type '$0' cannot be used to refer to non-static member '$1'") DIAGNOSTIC(30101, Error, cannotDereferenceType, "cannot dereference type '$0', do you mean to use '.'?") diff --git a/source/slang/slang-lower-to-ir.cpp b/source/slang/slang-lower-to-ir.cpp index debe5078d..f2a31a8da 100644 --- a/source/slang/slang-lower-to-ir.cpp +++ b/source/slang/slang-lower-to-ir.cpp @@ -7782,6 +7782,33 @@ struct DeclLoweringVisitor : DeclVisitor return false; } + struct NestedContext + { + IRGenEnv subEnvStorage; + IRBuilder subBuilderStorage; + IRGenContext subContextStorage; + + NestedContext(DeclLoweringVisitor* outer) + : subBuilderStorage(*outer->getBuilder()) + , subContextStorage(*outer->context) + { + auto outerContext = outer->context; + + subEnvStorage.outer = outerContext->env; + + subContextStorage.irBuilder = &subBuilderStorage; + subContextStorage.env = &subEnvStorage; + + subContextStorage.thisType = outerContext->thisType; + subContextStorage.thisTypeWitness = outerContext->thisTypeWitness; + + subContextStorage.returnDestination = LoweredValInfo(); + } + + IRBuilder* getBuilder() { return &subBuilderStorage; } + IRGenContext* getContext() { return &subContextStorage; } + }; + LoweredValInfo lowerGlobalShaderParam(VarDecl* decl) { IRType* paramType = lowerType(context, decl->getType()); @@ -7938,47 +7965,51 @@ struct DeclLoweringVisitor : DeclVisitor return lowerGlobalConstantDecl(decl); } - IRType* varType = lowerType(context, decl->getType()); + NestedContext nested(this); + auto subBuilder = nested.getBuilder(); + auto subContext = nested.getContext(); - auto builder = getBuilder(); + IRGeneric* outerGeneric = nullptr; + + // If we are static, then we need to insert the declaration before the parent. + // This tries to match the behavior of previous `lowerFunctionStaticConstVarDecl` functionality + if (isFunctionStaticVarDecl(decl)) + { + // We need to insert the constant at a level above + // the function being emitted. This will usually + // be the global scope, but it might be an outer + // generic if we are lowering a generic function. + subBuilder->setInsertBefore(subBuilder->getFunc()); + } + else if (!isFunctionVarDecl(decl)) + { + outerGeneric = emitOuterGenerics(subContext, decl, decl); + } + + IRType* varType = lowerType(subContext, decl->getType()); // TODO(JS): Do we create something derived from IRGlobalVar? Or do we use // a decoration to identify an *actual* global? - IRGlobalValueWithCode* irGlobal = builder->createGlobalVar(varType); - LoweredValInfo globalVal = LoweredValInfo::ptr(irGlobal); + IRGlobalValueWithCode* irGlobal = subBuilder->createGlobalVar(varType); - addLinkageDecoration(context, irGlobal, decl); - addNameHint(context, irGlobal, decl); + addLinkageDecoration(subContext, irGlobal, decl); + addNameHint(subContext, irGlobal, decl); - maybeSetRate(context, irGlobal, decl); + maybeSetRate(subContext, irGlobal, decl); - addVarDecorations(context, irGlobal, decl); - maybeAddDebugLocationDecoration(context, irGlobal); + addVarDecorations(subContext, irGlobal, decl); + maybeAddDebugLocationDecoration(subContext, irGlobal); if (decl) { - builder->addHighLevelDeclDecoration(irGlobal, decl); + subBuilder->addHighLevelDeclDecoration(irGlobal, decl); } - // A global variable's SSA value is a *pointer* to - // the underlying storage. - context->setGlobalValue(decl, globalVal); - if( auto initExpr = decl->initExpr ) { - IRBuilder subBuilderStorage = *getBuilder(); - IRBuilder* subBuilder = &subBuilderStorage; - subBuilder->setInsertInto(irGlobal); - IRGenContext subContextStorage = *context; - IRGenContext* subContext = &subContextStorage; - - subContext->irBuilder = subBuilder; - - // TODO: set up a parent IR decl to put the instructions into - IRBlock* entryBlock = subBuilder->emitBlock(); subBuilder->setInsertInto(entryBlock); @@ -7986,38 +8017,14 @@ struct DeclLoweringVisitor : DeclVisitor subContext->irBuilder->emitReturn(getSimpleVal(subContext, initVal)); } - irGlobal->moveToEnd(); + // A global variable's SSA value is a *pointer* to + // the underlying storage. + auto loweredValue = LoweredValInfo::ptr(finishOuterGenerics(subBuilder, irGlobal, outerGeneric)); + context->setGlobalValue(decl, loweredValue); - return globalVal; + return loweredValue; } - struct NestedContext - { - IRGenEnv subEnvStorage; - IRBuilder subBuilderStorage; - IRGenContext subContextStorage; - - NestedContext(DeclLoweringVisitor* outer) - : subBuilderStorage(*outer->getBuilder()) - , subContextStorage(*outer->context) - { - auto outerContext = outer->context; - - subEnvStorage.outer = outerContext->env; - - subContextStorage.irBuilder = &subBuilderStorage; - subContextStorage.env = &subEnvStorage; - - subContextStorage.thisType = outerContext->thisType; - subContextStorage.thisTypeWitness = outerContext->thisTypeWitness; - - subContextStorage.returnDestination = LoweredValInfo(); - } - - IRBuilder* getBuilder() { return &subBuilderStorage; } - IRGenContext* getContext() { return &subContextStorage; } - }; - LoweredValInfo lowerFunctionStaticConstVarDecl( VarDeclBase* decl) { @@ -10235,10 +10242,10 @@ bool canDeclLowerToAGeneric(Decl* decl) // a generic that returns a type (a simple type-level function). if(as(decl)) return true; - // If we have a variable declaration that is *static* and *const* we can lower to a generic + // A static member variable declaration can be lowered into a generic. if (auto varDecl = as(decl)) { - if (varDecl->hasModifier() && varDecl->hasModifier()) + if (varDecl->hasModifier()) { return !isFunctionVarDecl(varDecl); } @@ -10360,7 +10367,9 @@ LoweredValInfo emitDeclRef( // We can only really specialize things that map to single values. // It would be an error if we got a non-`None` value that // wasn't somehow a single value. - auto irGenericVal = getSimpleVal(context, genericVal); + genericVal = materialize(context, genericVal); + auto irGenericVal = genericVal.val; + SLANG_ASSERT(irGenericVal); // We have the IR value for the generic we'd like to specialize, // and now we need to get the value for the arguments. @@ -10392,8 +10401,16 @@ LoweredValInfo emitDeclRef( irGenericVal, irArgs.getCount(), irArgs.getBuffer()); - - return LoweredValInfo::simple(irSpecializedVal); + switch (genericVal.flavor) + { + case LoweredValInfo::Flavor::Simple: + return LoweredValInfo::simple(irSpecializedVal); + case LoweredValInfo::Flavor::Ptr: + return LoweredValInfo::ptr(irSpecializedVal); + default: + SLANG_UNEXPECTED("unhandled lowered value flavor"); + UNREACHABLE_RETURN(LoweredValInfo()); + } } else if(auto thisTypeSubst = as(subst)) { diff --git a/source/slang/slang-syntax.cpp b/source/slang/slang-syntax.cpp index b6fd8936e..0b92d07af 100644 --- a/source/slang/slang-syntax.cpp +++ b/source/slang/slang-syntax.cpp @@ -25,6 +25,8 @@ void printDiagnosticArg(StringBuilder& sb, Decl* decl) void printDiagnosticArg(StringBuilder& sb, DeclRefBase* declRefBase) { + if (!declRefBase) + return; printDiagnosticArg(sb, declRefBase->getDecl()); } -- cgit v1.2.3