From 92f21de580e16a37600f082c0968913111f5ef91 Mon Sep 17 00:00:00 2001 From: Yong He Date: Tue, 12 Dec 2023 14:07:35 -0800 Subject: Add check for invalid use of modifiers. (#3402) * Add check for invalid use of modifiers. * Fixes. * Add test. --------- Co-authored-by: Yong He --- source/slang/slang-check-modifier.cpp | 201 +++++++++++++++++++++++++++++++++- 1 file changed, 200 insertions(+), 1 deletion(-) (limited to 'source/slang/slang-check-modifier.cpp') diff --git a/source/slang/slang-check-modifier.cpp b/source/slang/slang-check-modifier.cpp index 20a393ff5..53d121b4b 100644 --- a/source/slang/slang-check-modifier.cpp +++ b/source/slang/slang-check-modifier.cpp @@ -919,6 +919,179 @@ namespace Slang return attr; } + ASTNodeType getModifierConflictGroupKind(ASTNodeType modifierType) + { + switch (modifierType) + { + // Allowed only on parameters and global variables. + case ASTNodeType::InModifier: + return modifierType; + case ASTNodeType::OutModifier: + case ASTNodeType::RefModifier: + case ASTNodeType::ConstRefModifier: + case ASTNodeType::InOutModifier: + return ASTNodeType::OutModifier; + + // Modifiers that are their own exclusive group. + case ASTNodeType::GLSLLayoutModifier: + case ASTNodeType::GLSLParsedLayoutModifier: + case ASTNodeType::GLSLConstantIDLayoutModifier: + case ASTNodeType::GLSLLocationLayoutModifier: + case ASTNodeType::GLSLUnparsedLayoutModifier: + case ASTNodeType::GLSLLayoutModifierGroupMarker: + case ASTNodeType::GLSLLayoutModifierGroupBegin: + case ASTNodeType::GLSLLayoutModifierGroupEnd: + case ASTNodeType::GLSLBufferModifier: + case ASTNodeType::GLSLWriteOnlyModifier: + case ASTNodeType::GLSLReadOnlyModifier: + case ASTNodeType::GLSLPatchModifier: + case ASTNodeType::RayPayloadAccessSemantic: + case ASTNodeType::RayPayloadReadSemantic: + case ASTNodeType::RayPayloadWriteSemantic: + case ASTNodeType::GloballyCoherentModifier: + case ASTNodeType::PreciseModifier: + case ASTNodeType::IntrinsicOpModifier: + case ASTNodeType::InlineModifier: + case ASTNodeType::ExternModifier: + case ASTNodeType::HLSLExportModifier: + case ASTNodeType::ExternCppModifier: + case ASTNodeType::ExportedModifier: + case ASTNodeType::ConstModifier: + case ASTNodeType::ConstExprModifier: + case ASTNodeType::MatrixLayoutModifier: + case ASTNodeType::RowMajorLayoutModifier: + case ASTNodeType::HLSLRowMajorLayoutModifier: + case ASTNodeType::GLSLColumnMajorLayoutModifier: + case ASTNodeType::ColumnMajorLayoutModifier: + case ASTNodeType::HLSLColumnMajorLayoutModifier: + case ASTNodeType::GLSLRowMajorLayoutModifier: + case ASTNodeType::HLSLEffectSharedModifier: + case ASTNodeType::HLSLGroupSharedModifier: + case ASTNodeType::HLSLVolatileModifier: + case ASTNodeType::GLSLPrecisionModifier: + return modifierType; + + case ASTNodeType::HLSLStaticModifier: + case ASTNodeType::ActualGlobalModifier: + case ASTNodeType::HLSLUniformModifier: + return ASTNodeType::HLSLStaticModifier; + + case ASTNodeType::HLSLNoInterpolationModifier: + case ASTNodeType::HLSLNoPerspectiveModifier: + case ASTNodeType::HLSLLinearModifier: + case ASTNodeType::HLSLSampleModifier: + case ASTNodeType::HLSLCentroidModifier: + case ASTNodeType::PerVertexModifier: + return ASTNodeType::InterpolationModeModifier; + + case ASTNodeType::PrefixModifier: + case ASTNodeType::PostfixModifier: + return ASTNodeType::PrefixModifier; + + case ASTNodeType::BuiltinModifier: + case ASTNodeType::PublicModifier: + case ASTNodeType::PrivateModifier: + case ASTNodeType::InternalModifier: + return ASTNodeType::VisibilityModifier; + + default: + return ASTNodeType::NodeBase; + } + } + + bool isModifierAllowedOnDecl(ASTNodeType modifierType, Decl* decl) + { + switch (modifierType) + { + // Allowed only on parameters and global variables. + case ASTNodeType::InModifier: + case ASTNodeType::OutModifier: + case ASTNodeType::InOutModifier: + case ASTNodeType::RefModifier: + case ASTNodeType::ConstRefModifier: + case ASTNodeType::GLSLLayoutModifier: + case ASTNodeType::GLSLParsedLayoutModifier: + case ASTNodeType::GLSLConstantIDLayoutModifier: + case ASTNodeType::GLSLLocationLayoutModifier: + case ASTNodeType::GLSLUnparsedLayoutModifier: + case ASTNodeType::GLSLLayoutModifierGroupMarker: + case ASTNodeType::GLSLLayoutModifierGroupBegin: + case ASTNodeType::GLSLLayoutModifierGroupEnd: + case ASTNodeType::GLSLBufferModifier: + case ASTNodeType::GLSLWriteOnlyModifier: + case ASTNodeType::GLSLReadOnlyModifier: + case ASTNodeType::GLSLPatchModifier: + case ASTNodeType::RayPayloadAccessSemantic: + case ASTNodeType::RayPayloadReadSemantic: + case ASTNodeType::RayPayloadWriteSemantic: + case ASTNodeType::GloballyCoherentModifier: + return (as(decl) && isGlobalDecl(decl)) || as(decl) || as(decl); + + // Allowed only on parameters, struct fields and global variables. + case ASTNodeType::InterpolationModeModifier: + case ASTNodeType::HLSLNoInterpolationModifier: + case ASTNodeType::HLSLNoPerspectiveModifier: + case ASTNodeType::HLSLLinearModifier: + case ASTNodeType::HLSLSampleModifier: + case ASTNodeType::HLSLCentroidModifier: + case ASTNodeType::PerVertexModifier: + case ASTNodeType::HLSLUniformModifier: + return (as(decl) && (isGlobalDecl(decl) || as(getParentDecl(decl)))) || as(decl); + + case ASTNodeType::HLSLSemantic: + case ASTNodeType::HLSLLayoutSemantic: + case ASTNodeType::HLSLRegisterSemantic: + case ASTNodeType::HLSLPackOffsetSemantic: + case ASTNodeType::HLSLSimpleSemantic: + return (as(decl) && (isGlobalDecl(decl) || as(getParentDecl(decl)))) || as(decl) || as(decl); + + // Allowed only on functions + case ASTNodeType::IntrinsicOpModifier: + case ASTNodeType::SpecializedForTargetModifier: + case ASTNodeType::InlineModifier: + case ASTNodeType::PrefixModifier: + case ASTNodeType::PostfixModifier: + return as(decl); + + case ASTNodeType::BuiltinModifier: + case ASTNodeType::PublicModifier: + case ASTNodeType::PrivateModifier: + case ASTNodeType::InternalModifier: + case ASTNodeType::ExternModifier: + case ASTNodeType::HLSLExportModifier: + case ASTNodeType::ExternCppModifier: + return as(decl) || as(decl) || as(decl) || as(decl) + || as(decl) || as(decl) || as(decl) || as(decl); + + case ASTNodeType::ExportedModifier: + return as(decl); + + case ASTNodeType::ConstModifier: + case ASTNodeType::HLSLStaticModifier: + case ASTNodeType::ConstExprModifier: + case ASTNodeType::PreciseModifier: + return as(decl) || as(decl); + + case ASTNodeType::ActualGlobalModifier: + case ASTNodeType::MatrixLayoutModifier: + case ASTNodeType::RowMajorLayoutModifier: + case ASTNodeType::HLSLRowMajorLayoutModifier: + case ASTNodeType::GLSLColumnMajorLayoutModifier: + case ASTNodeType::ColumnMajorLayoutModifier: + case ASTNodeType::HLSLColumnMajorLayoutModifier: + case ASTNodeType::GLSLRowMajorLayoutModifier: + case ASTNodeType::HLSLEffectSharedModifier: + case ASTNodeType::HLSLGroupSharedModifier: + case ASTNodeType::HLSLVolatileModifier: + return as(decl) || as(decl); + + case ASTNodeType::GLSLPrecisionModifier: + return as(decl) || as(decl) || as(decl); + default: + return true; + } + } + Modifier* SemanticsVisitor::checkModifier( Modifier* m, ModifiableSyntaxNode* syntaxNode) @@ -935,6 +1108,15 @@ namespace Slang return checkAttribute(hlslUncheckedAttribute, syntaxNode); } + if (auto decl = as(syntaxNode)) + { + if (!isModifierAllowedOnDecl(m->astNodeType, decl)) + { + getSink()->diagnose(m, Diagnostics::modifierNotAllowed, m); + return m; + } + } + if (auto hlslSemantic = as(m)) { if (hlslSemantic->name.getName() == getSession()->getCompletionRequestTokenName()) @@ -1164,9 +1346,25 @@ namespace Slang Modifier* resultModifiers = nullptr; Modifier** resultModifierLink = &resultModifiers; + // We will keep track of the modifiers for each conflict group. + Dictionary mapExclusiveGroupToModifier; + Modifier* modifier = syntaxNode->modifiers.first; - while(modifier) + while (modifier) { + // Check if a modifier belonging to the same conflict group is already + // defined. + Modifier* existingModifier = nullptr; + auto conflictGroup = getModifierConflictGroupKind(modifier->astNodeType); + if (conflictGroup != ASTNodeType::NodeBase) + { + if (mapExclusiveGroupToModifier.tryGetValue(conflictGroup, existingModifier)) + { + getSink()->diagnose(modifier->loc, Diagnostics::duplicateModifier, modifier, existingModifier); + } + mapExclusiveGroupToModifier[conflictGroup] = modifier; + } + // Because we are rewriting the list in place, we need to extract // the next modifier here (not at the end of the loop). auto next = modifier->next; @@ -1177,6 +1375,7 @@ namespace Slang modifier->next = nullptr; auto checkedModifier = checkModifier(modifier, syntaxNode); + if(checkedModifier) { // If checking gave us a modifier to add, then we -- cgit v1.2.3