From 2bbd043c143cba42b9302fe5dd4e1d7622be0fab Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Wed, 3 Sep 2025 17:11:03 +0200 Subject: [PATCH 1/6] Register assignments with mismatching ': register()' syntax now doesn't segfault the compiler anymore. Instead, it assumes that when you write : MYTEST() that you're intending to make it into a register, will parse the register the normal way to skip invalid syntax and will show an error that you shouldn't do that. --- .../clang/Basic/DiagnosticParseKinds.td | 2 + tools/clang/include/clang/Parse/Parser.h | 1 + tools/clang/lib/Parse/ParseDecl.cpp | 272 ++++++++++-------- 3 files changed, 156 insertions(+), 119 deletions(-) diff --git a/tools/clang/include/clang/Basic/DiagnosticParseKinds.td b/tools/clang/include/clang/Basic/DiagnosticParseKinds.td index 3de70da1e7..a722882071 100644 --- a/tools/clang/include/clang/Basic/DiagnosticParseKinds.td +++ b/tools/clang/include/clang/Basic/DiagnosticParseKinds.td @@ -1025,6 +1025,8 @@ def warn_hlsl_effect_technique : Warning < def warn_hlsl_semantic_identifier_collision : Warning < "'%0' interpreted as semantic; previous definition(s) ignored">, InGroup< HLSLSemanticIdentifierCollision >; +def err_hlsl_register_is_misspelled : Error < + "Syntax similar to : register() was used but unexpected keyword '%0' was used instead.">; def err_hlsl_enum : Error< "enum is unsupported in HLSL before 2017">; def warn_hlsl_new_feature : Warning < diff --git a/tools/clang/include/clang/Parse/Parser.h b/tools/clang/include/clang/Parse/Parser.h index 1c8eca36ce..e3039b8aee 100644 --- a/tools/clang/include/clang/Parse/Parser.h +++ b/tools/clang/include/clang/Parse/Parser.h @@ -2098,6 +2098,7 @@ class Parser : public CodeCompletionHandler { // HLSL Change Starts: Parse HLSL Attributes and append them to Declarator Object bool MaybeParseHLSLAttributes(std::vector &target); + bool ConsumeRegisterAssignment(hlsl::RegisterAssignment &asg); //Make sure it starts with register( first inline bool MaybeParseHLSLAttributes(Declarator &D) { return MaybeParseHLSLAttributes(D.UnusualAnnotations); } diff --git a/tools/clang/lib/Parse/ParseDecl.cpp b/tools/clang/lib/Parse/ParseDecl.cpp index 59be41a484..5ea3919a0e 100644 --- a/tools/clang/lib/Parse/ParseDecl.cpp +++ b/tools/clang/lib/Parse/ParseDecl.cpp @@ -341,6 +341,143 @@ static void ParseSpaceForHLSL(const StringRef name, } } +bool Parser::ConsumeRegisterAssignment(hlsl::RegisterAssignment &r) { + + ASTContext &context = getActions().getASTContext(); + + if (!Tok.is(tok::identifier)) { + Diag(Tok.getLocation(), diag::err_expected) << tok::identifier; + SkipUntil(tok::r_paren, StopAtSemi); // skip through ) + return true; + } + + StringRef identifierText = Tok.getIdentifierInfo()->getName(); + if (IsShaderProfileLike(identifierText) || + IsShaderProfileShort(identifierText)) { + r.ShaderProfile = Tok.getIdentifierInfo()->getName(); + ConsumeToken(); // consume shader model + if (ExpectAndConsume(tok::comma, diag::err_expected)) { + SkipUntil(tok::r_paren, StopAtSemi); // skip through ) + return true; + } + } + + if (!Tok.is(tok::identifier)) { + Diag(Tok.getLocation(), diag::err_expected) << tok::identifier; + SkipUntil(tok::r_paren, StopAtSemi); // skip through ) + return true; + } + + DXASSERT(Tok.is(tok::identifier), + "otherwise previous code should have failed"); + unsigned diagId; + + bool hasOnlySpace = false; + identifierText = Tok.getIdentifierInfo()->getName(); + if (identifierText.substr(0, sizeof("space") - 1).equals("space")) { + hasOnlySpace = true; + } else { + ParseRegisterNumberForHLSL(Tok.getIdentifierInfo()->getName(), + &r.RegisterType, &r.RegisterNumber, &diagId); + if (diagId == 0) { + r.setIsValid(true); + } else { + r.setIsValid(false); + Diag(Tok.getLocation(), diagId); + } + + ConsumeToken(); // consume register (type'#') + + ExprResult subcomponentResult; + if (Tok.is(tok::l_square)) { + BalancedDelimiterTracker brackets(*this, tok::l_square); + brackets.consumeOpen(); + + ExprResult result; + if (Tok.isNot(tok::r_square)) { + subcomponentResult = ParseConstantExpression(); + r.IsValid = r.IsValid && !subcomponentResult.isInvalid(); + Expr::EvalResult evalResult; + if (!subcomponentResult.get()->EvaluateAsRValue(evalResult, context) || + evalResult.hasSideEffects() || + (!evalResult.Val.isInt() && !evalResult.Val.isFloat())) { + Diag(Tok.getLocation(), + diag::err_hlsl_unsupported_register_noninteger); + r.setIsValid(false); + } else { + llvm::APSInt intResult; + if (evalResult.Val.isFloat()) { + bool isExact; + // TODO: consider what to do when convertToInteger fails + evalResult.Val.getFloat().convertToInteger( + intResult, llvm::APFloat::roundingMode::rmTowardZero, &isExact); + } else { + DXASSERT( + evalResult.Val.isInt(), + "otherwise prior test in this function should have failed"); + intResult = evalResult.Val.getInt(); + } + + if (intResult.isNegative()) { + Diag(Tok.getLocation(), + diag::err_hlsl_unsupported_register_noninteger); + r.setIsValid(false); + } else { + r.RegisterOffset = intResult.getLimitedValue(); + } + } + } else { + Diag(Tok.getLocation(), diag::err_expected_expression); + r.setIsValid(false); + } + + if (brackets.consumeClose()) { + SkipUntil(tok::r_paren, StopAtSemi); // skip through ) + return true; + } + } + } + if (hasOnlySpace) { + uint32_t RegisterSpaceValue = 0; + ParseSpaceForHLSL(Tok.getIdentifierInfo()->getName(), &RegisterSpaceValue, + &diagId); + if (diagId != 0) { + Diag(Tok.getLocation(), diagId); + r.setIsValid(false); + } else { + r.RegisterSpace = RegisterSpaceValue; + r.setIsValid(true); + } + ConsumeToken(); // consume identifier + } else { + if (Tok.is(tok::comma)) { + ConsumeToken(); // consume comma + if (!Tok.is(tok::identifier)) { + Diag(Tok.getLocation(), diag::err_expected) << tok::identifier; + SkipUntil(tok::r_paren, StopAtSemi); // skip through ) + return true; + } + unsigned RegisterSpaceVal = 0; + ParseSpaceForHLSL(Tok.getIdentifierInfo()->getName(), &RegisterSpaceVal, + &diagId); + if (diagId != 0) { + Diag(Tok.getLocation(), diagId); + r.setIsValid(false); + } else { + r.RegisterSpace = RegisterSpaceVal; + } + ConsumeToken(); // consume identifier + } + } + + if (ExpectAndConsume(tok::r_paren, diag::err_expected)) { + SkipUntil(tok::r_paren, StopAtSemi); // skip through ) + return true; + } + + return false; +} + bool Parser::MaybeParseHLSLAttributes(std::vector &target) { if (!getLangOpts().HLSL) { @@ -428,127 +565,9 @@ bool Parser::MaybeParseHLSLAttributes(std::vector &ta if (ExpectAndConsume(tok::l_paren, diag::err_expected_lparen_after, "register")) { return true; } - if (!Tok.is(tok::identifier)) { - Diag(Tok.getLocation(), diag::err_expected) << tok::identifier; - SkipUntil(tok::r_paren, StopAtSemi); // skip through ) - return true; - } - - StringRef identifierText = Tok.getIdentifierInfo()->getName(); - if (IsShaderProfileLike(identifierText) || IsShaderProfileShort(identifierText)) { - r.ShaderProfile = Tok.getIdentifierInfo()->getName(); - ConsumeToken(); // consume shader model - if (ExpectAndConsume(tok::comma, diag::err_expected)) { - SkipUntil(tok::r_paren, StopAtSemi); // skip through ) - return true; - } - } - - if (!Tok.is(tok::identifier)) { - Diag(Tok.getLocation(), diag::err_expected) << tok::identifier; - SkipUntil(tok::r_paren, StopAtSemi); // skip through ) - return true; - } - - DXASSERT(Tok.is(tok::identifier), "otherwise previous code should have failed"); - unsigned diagId; - - bool hasOnlySpace = false; - identifierText = Tok.getIdentifierInfo()->getName(); - if (identifierText.substr(0, sizeof("space")-1).equals("space")) { - hasOnlySpace = true; - } else { - ParseRegisterNumberForHLSL( - Tok.getIdentifierInfo()->getName(), &r.RegisterType, &r.RegisterNumber, &diagId); - if (diagId == 0) { - r.setIsValid(true); - } else { - r.setIsValid(false); - Diag(Tok.getLocation(), diagId); - } - - ConsumeToken(); // consume register (type'#') - - ExprResult subcomponentResult; - if (Tok.is(tok::l_square)) { - BalancedDelimiterTracker brackets(*this, tok::l_square); - brackets.consumeOpen(); - - ExprResult result; - if (Tok.isNot(tok::r_square)) { - subcomponentResult = ParseConstantExpression(); - r.IsValid = r.IsValid && !subcomponentResult.isInvalid(); - Expr::EvalResult evalResult; - if (!subcomponentResult.get()->EvaluateAsRValue(evalResult, context) || - evalResult.hasSideEffects() || - (!evalResult.Val.isInt() && !evalResult.Val.isFloat())) { - Diag(Tok.getLocation(), diag::err_hlsl_unsupported_register_noninteger); - r.setIsValid(false); - } else { - llvm::APSInt intResult; - if (evalResult.Val.isFloat()) { - bool isExact; - // TODO: consider what to do when convertToInteger fails - evalResult.Val.getFloat().convertToInteger(intResult, llvm::APFloat::roundingMode::rmTowardZero, &isExact); - } else { - DXASSERT(evalResult.Val.isInt(), "otherwise prior test in this function should have failed"); - intResult = evalResult.Val.getInt(); - } - - if (intResult.isNegative()) { - Diag(Tok.getLocation(), diag::err_hlsl_unsupported_register_noninteger); - r.setIsValid(false); - } else { - r.RegisterOffset = intResult.getLimitedValue(); - } - } - } else { - Diag(Tok.getLocation(), diag::err_expected_expression); - r.setIsValid(false); - } - if (brackets.consumeClose()) { - SkipUntil(tok::r_paren, StopAtSemi); // skip through ) - return true; - } - } - } - if (hasOnlySpace) { - uint32_t RegisterSpaceValue = 0; - ParseSpaceForHLSL(Tok.getIdentifierInfo()->getName(), &RegisterSpaceValue, &diagId); - if (diagId != 0) { - Diag(Tok.getLocation(), diagId); - r.setIsValid(false); - } else { - r.RegisterSpace = RegisterSpaceValue; - r.setIsValid(true); - } - ConsumeToken(); // consume identifier - } else { - if (Tok.is(tok::comma)) { - ConsumeToken(); // consume comma - if (!Tok.is(tok::identifier)) { - Diag(Tok.getLocation(), diag::err_expected) << tok::identifier; - SkipUntil(tok::r_paren, StopAtSemi); // skip through ) - return true; - } - unsigned RegisterSpaceVal = 0; - ParseSpaceForHLSL(Tok.getIdentifierInfo()->getName(), &RegisterSpaceVal, &diagId); - if (diagId != 0) { - Diag(Tok.getLocation(), diagId); - r.setIsValid(false); - } - else { - r.RegisterSpace = RegisterSpaceVal; - } - ConsumeToken(); // consume identifier - } - } - - if (ExpectAndConsume(tok::r_paren, diag::err_expected)) { - SkipUntil(tok::r_paren, StopAtSemi); // skip through ) + if (ConsumeRegisterAssignment(r)) return true; - } target.push_back(new (context) hlsl::RegisterAssignment(r)); } @@ -633,6 +652,21 @@ bool Parser::MaybeParseHLSLAttributes(std::vector &ta Actions.DiagnoseSemanticDecl(pUA); ConsumeToken(); // consume semantic + // Likely a misspell of register() or a mismatching macro: + // registers() would cause a crash without this fix. + // registers() + + if (Tok.is(tok::l_paren)) { + + Diag(Tok.getLocation(), diag::err_hlsl_register_is_misspelled) + << semanticName; + + ConsumeParen(); + hlsl::RegisterAssignment dummy; + if (ConsumeRegisterAssignment(dummy)) // Skip invalid syntax + return true; + } + target.push_back(pUA); } else { From 9e78ab327e4e7de2be946933e75052a9892619d9 Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Wed, 3 Sep 2025 17:17:58 +0200 Subject: [PATCH 2/6] Clang format --- tools/clang/include/clang/Parse/Parser.h | 4 +++- tools/clang/lib/Parse/ParseDecl.cpp | 1 - 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/clang/include/clang/Parse/Parser.h b/tools/clang/include/clang/Parse/Parser.h index e3039b8aee..ef52890896 100644 --- a/tools/clang/include/clang/Parse/Parser.h +++ b/tools/clang/include/clang/Parse/Parser.h @@ -2098,7 +2098,9 @@ class Parser : public CodeCompletionHandler { // HLSL Change Starts: Parse HLSL Attributes and append them to Declarator Object bool MaybeParseHLSLAttributes(std::vector &target); - bool ConsumeRegisterAssignment(hlsl::RegisterAssignment &asg); //Make sure it starts with register( first + bool ConsumeRegisterAssignment( + hlsl::RegisterAssignment + &asg); // Make sure it starts with register( first inline bool MaybeParseHLSLAttributes(Declarator &D) { return MaybeParseHLSLAttributes(D.UnusualAnnotations); } diff --git a/tools/clang/lib/Parse/ParseDecl.cpp b/tools/clang/lib/Parse/ParseDecl.cpp index 5ea3919a0e..ada99f1388 100644 --- a/tools/clang/lib/Parse/ParseDecl.cpp +++ b/tools/clang/lib/Parse/ParseDecl.cpp @@ -654,7 +654,6 @@ bool Parser::MaybeParseHLSLAttributes(std::vector &ta // Likely a misspell of register() or a mismatching macro: // registers() would cause a crash without this fix. - // registers() if (Tok.is(tok::l_paren)) { From fdcc8547a3f514df1a8a9653c623b367628bacc1 Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Tue, 4 Nov 2025 13:05:41 +0100 Subject: [PATCH 3/6] Added hlsl unit test to check if registers() crashes or produces the right error --- .../clang/test/SemaHLSL/register-mistype-errors.hlsl | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 tools/clang/test/SemaHLSL/register-mistype-errors.hlsl diff --git a/tools/clang/test/SemaHLSL/register-mistype-errors.hlsl b/tools/clang/test/SemaHLSL/register-mistype-errors.hlsl new file mode 100644 index 0000000000..cbe876b554 --- /dev/null +++ b/tools/clang/test/SemaHLSL/register-mistype-errors.hlsl @@ -0,0 +1,11 @@ +// RUN: %dxc -T lib_6_3 -verify %s + +// expected-error@+1 {{Syntax similar to : register() was used but unexpected keyword 'registers' was used instead.}} +RWStructuredBuffer uav1 : registers(u3); + +[shader("pixel")] +float4 main(): SV_Target +{ + uav1[0] = 2; + return 0.xxxx; +} From 5a0391d7ad2952819d577161bb755bf7f6433e21 Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Wed, 5 Nov 2025 20:12:47 +0100 Subject: [PATCH 4/6] Greatly simplified the invalid hlsl annotation check to allow it to work for packoffset and register. This simplifies it a ton, at the cost of being able to detect further register or packoffset errors, but at least it doesn't crash. --- .../clang/Basic/DiagnosticParseKinds.td | 4 +- tools/clang/include/clang/Parse/Parser.h | 3 - tools/clang/lib/Parse/ParseDecl.cpp | 270 ++++++++---------- .../hlsl-attribute-mistype-errors.hlsl | 35 +++ .../SemaHLSL/register-mistype-errors.hlsl | 11 - 5 files changed, 161 insertions(+), 162 deletions(-) create mode 100644 tools/clang/test/SemaHLSL/hlsl-attribute-mistype-errors.hlsl delete mode 100644 tools/clang/test/SemaHLSL/register-mistype-errors.hlsl diff --git a/tools/clang/include/clang/Basic/DiagnosticParseKinds.td b/tools/clang/include/clang/Basic/DiagnosticParseKinds.td index a722882071..dc2b76526d 100644 --- a/tools/clang/include/clang/Basic/DiagnosticParseKinds.td +++ b/tools/clang/include/clang/Basic/DiagnosticParseKinds.td @@ -1025,8 +1025,8 @@ def warn_hlsl_effect_technique : Warning < def warn_hlsl_semantic_identifier_collision : Warning < "'%0' interpreted as semantic; previous definition(s) ignored">, InGroup< HLSLSemanticIdentifierCollision >; -def err_hlsl_register_is_misspelled : Error < - "Syntax similar to : register() was used but unexpected keyword '%0' was used instead.">; +def err_hlsl_expected_hlsl_attribute : Error < + "Syntax indicated an hlsl attribute (: packoffset() or : register()), but unexpected attribute '%0' was used instead.">; def err_hlsl_enum : Error< "enum is unsupported in HLSL before 2017">; def warn_hlsl_new_feature : Warning < diff --git a/tools/clang/include/clang/Parse/Parser.h b/tools/clang/include/clang/Parse/Parser.h index ef52890896..1c8eca36ce 100644 --- a/tools/clang/include/clang/Parse/Parser.h +++ b/tools/clang/include/clang/Parse/Parser.h @@ -2098,9 +2098,6 @@ class Parser : public CodeCompletionHandler { // HLSL Change Starts: Parse HLSL Attributes and append them to Declarator Object bool MaybeParseHLSLAttributes(std::vector &target); - bool ConsumeRegisterAssignment( - hlsl::RegisterAssignment - &asg); // Make sure it starts with register( first inline bool MaybeParseHLSLAttributes(Declarator &D) { return MaybeParseHLSLAttributes(D.UnusualAnnotations); } diff --git a/tools/clang/lib/Parse/ParseDecl.cpp b/tools/clang/lib/Parse/ParseDecl.cpp index ada99f1388..26fdbca301 100644 --- a/tools/clang/lib/Parse/ParseDecl.cpp +++ b/tools/clang/lib/Parse/ParseDecl.cpp @@ -341,143 +341,6 @@ static void ParseSpaceForHLSL(const StringRef name, } } -bool Parser::ConsumeRegisterAssignment(hlsl::RegisterAssignment &r) { - - ASTContext &context = getActions().getASTContext(); - - if (!Tok.is(tok::identifier)) { - Diag(Tok.getLocation(), diag::err_expected) << tok::identifier; - SkipUntil(tok::r_paren, StopAtSemi); // skip through ) - return true; - } - - StringRef identifierText = Tok.getIdentifierInfo()->getName(); - if (IsShaderProfileLike(identifierText) || - IsShaderProfileShort(identifierText)) { - r.ShaderProfile = Tok.getIdentifierInfo()->getName(); - ConsumeToken(); // consume shader model - if (ExpectAndConsume(tok::comma, diag::err_expected)) { - SkipUntil(tok::r_paren, StopAtSemi); // skip through ) - return true; - } - } - - if (!Tok.is(tok::identifier)) { - Diag(Tok.getLocation(), diag::err_expected) << tok::identifier; - SkipUntil(tok::r_paren, StopAtSemi); // skip through ) - return true; - } - - DXASSERT(Tok.is(tok::identifier), - "otherwise previous code should have failed"); - unsigned diagId; - - bool hasOnlySpace = false; - identifierText = Tok.getIdentifierInfo()->getName(); - if (identifierText.substr(0, sizeof("space") - 1).equals("space")) { - hasOnlySpace = true; - } else { - ParseRegisterNumberForHLSL(Tok.getIdentifierInfo()->getName(), - &r.RegisterType, &r.RegisterNumber, &diagId); - if (diagId == 0) { - r.setIsValid(true); - } else { - r.setIsValid(false); - Diag(Tok.getLocation(), diagId); - } - - ConsumeToken(); // consume register (type'#') - - ExprResult subcomponentResult; - if (Tok.is(tok::l_square)) { - BalancedDelimiterTracker brackets(*this, tok::l_square); - brackets.consumeOpen(); - - ExprResult result; - if (Tok.isNot(tok::r_square)) { - subcomponentResult = ParseConstantExpression(); - r.IsValid = r.IsValid && !subcomponentResult.isInvalid(); - Expr::EvalResult evalResult; - if (!subcomponentResult.get()->EvaluateAsRValue(evalResult, context) || - evalResult.hasSideEffects() || - (!evalResult.Val.isInt() && !evalResult.Val.isFloat())) { - Diag(Tok.getLocation(), - diag::err_hlsl_unsupported_register_noninteger); - r.setIsValid(false); - } else { - llvm::APSInt intResult; - if (evalResult.Val.isFloat()) { - bool isExact; - // TODO: consider what to do when convertToInteger fails - evalResult.Val.getFloat().convertToInteger( - intResult, llvm::APFloat::roundingMode::rmTowardZero, &isExact); - } else { - DXASSERT( - evalResult.Val.isInt(), - "otherwise prior test in this function should have failed"); - intResult = evalResult.Val.getInt(); - } - - if (intResult.isNegative()) { - Diag(Tok.getLocation(), - diag::err_hlsl_unsupported_register_noninteger); - r.setIsValid(false); - } else { - r.RegisterOffset = intResult.getLimitedValue(); - } - } - } else { - Diag(Tok.getLocation(), diag::err_expected_expression); - r.setIsValid(false); - } - - if (brackets.consumeClose()) { - SkipUntil(tok::r_paren, StopAtSemi); // skip through ) - return true; - } - } - } - if (hasOnlySpace) { - uint32_t RegisterSpaceValue = 0; - ParseSpaceForHLSL(Tok.getIdentifierInfo()->getName(), &RegisterSpaceValue, - &diagId); - if (diagId != 0) { - Diag(Tok.getLocation(), diagId); - r.setIsValid(false); - } else { - r.RegisterSpace = RegisterSpaceValue; - r.setIsValid(true); - } - ConsumeToken(); // consume identifier - } else { - if (Tok.is(tok::comma)) { - ConsumeToken(); // consume comma - if (!Tok.is(tok::identifier)) { - Diag(Tok.getLocation(), diag::err_expected) << tok::identifier; - SkipUntil(tok::r_paren, StopAtSemi); // skip through ) - return true; - } - unsigned RegisterSpaceVal = 0; - ParseSpaceForHLSL(Tok.getIdentifierInfo()->getName(), &RegisterSpaceVal, - &diagId); - if (diagId != 0) { - Diag(Tok.getLocation(), diagId); - r.setIsValid(false); - } else { - r.RegisterSpace = RegisterSpaceVal; - } - ConsumeToken(); // consume identifier - } - } - - if (ExpectAndConsume(tok::r_paren, diag::err_expected)) { - SkipUntil(tok::r_paren, StopAtSemi); // skip through ) - return true; - } - - return false; -} - bool Parser::MaybeParseHLSLAttributes(std::vector &target) { if (!getLangOpts().HLSL) { @@ -565,9 +428,127 @@ bool Parser::MaybeParseHLSLAttributes(std::vector &ta if (ExpectAndConsume(tok::l_paren, diag::err_expected_lparen_after, "register")) { return true; } + if (!Tok.is(tok::identifier)) { + Diag(Tok.getLocation(), diag::err_expected) << tok::identifier; + SkipUntil(tok::r_paren, StopAtSemi); // skip through ) + return true; + } - if (ConsumeRegisterAssignment(r)) + StringRef identifierText = Tok.getIdentifierInfo()->getName(); + if (IsShaderProfileLike(identifierText) || IsShaderProfileShort(identifierText)) { + r.ShaderProfile = Tok.getIdentifierInfo()->getName(); + ConsumeToken(); // consume shader model + if (ExpectAndConsume(tok::comma, diag::err_expected)) { + SkipUntil(tok::r_paren, StopAtSemi); // skip through ) + return true; + } + } + + if (!Tok.is(tok::identifier)) { + Diag(Tok.getLocation(), diag::err_expected) << tok::identifier; + SkipUntil(tok::r_paren, StopAtSemi); // skip through ) return true; + } + + DXASSERT(Tok.is(tok::identifier), "otherwise previous code should have failed"); + unsigned diagId; + + bool hasOnlySpace = false; + identifierText = Tok.getIdentifierInfo()->getName(); + if (identifierText.substr(0, sizeof("space")-1).equals("space")) { + hasOnlySpace = true; + } else { + ParseRegisterNumberForHLSL( + Tok.getIdentifierInfo()->getName(), &r.RegisterType, &r.RegisterNumber, &diagId); + if (diagId == 0) { + r.setIsValid(true); + } else { + r.setIsValid(false); + Diag(Tok.getLocation(), diagId); + } + + ConsumeToken(); // consume register (type'#') + + ExprResult subcomponentResult; + if (Tok.is(tok::l_square)) { + BalancedDelimiterTracker brackets(*this, tok::l_square); + brackets.consumeOpen(); + + ExprResult result; + if (Tok.isNot(tok::r_square)) { + subcomponentResult = ParseConstantExpression(); + r.IsValid = r.IsValid && !subcomponentResult.isInvalid(); + Expr::EvalResult evalResult; + if (!subcomponentResult.get()->EvaluateAsRValue(evalResult, context) || + evalResult.hasSideEffects() || + (!evalResult.Val.isInt() && !evalResult.Val.isFloat())) { + Diag(Tok.getLocation(), diag::err_hlsl_unsupported_register_noninteger); + r.setIsValid(false); + } else { + llvm::APSInt intResult; + if (evalResult.Val.isFloat()) { + bool isExact; + // TODO: consider what to do when convertToInteger fails + evalResult.Val.getFloat().convertToInteger(intResult, llvm::APFloat::roundingMode::rmTowardZero, &isExact); + } else { + DXASSERT(evalResult.Val.isInt(), "otherwise prior test in this function should have failed"); + intResult = evalResult.Val.getInt(); + } + + if (intResult.isNegative()) { + Diag(Tok.getLocation(), diag::err_hlsl_unsupported_register_noninteger); + r.setIsValid(false); + } else { + r.RegisterOffset = intResult.getLimitedValue(); + } + } + } else { + Diag(Tok.getLocation(), diag::err_expected_expression); + r.setIsValid(false); + } + + if (brackets.consumeClose()) { + SkipUntil(tok::r_paren, StopAtSemi); // skip through ) + return true; + } + } + } + if (hasOnlySpace) { + uint32_t RegisterSpaceValue = 0; + ParseSpaceForHLSL(Tok.getIdentifierInfo()->getName(), &RegisterSpaceValue, &diagId); + if (diagId != 0) { + Diag(Tok.getLocation(), diagId); + r.setIsValid(false); + } else { + r.RegisterSpace = RegisterSpaceValue; + r.setIsValid(true); + } + ConsumeToken(); // consume identifier + } else { + if (Tok.is(tok::comma)) { + ConsumeToken(); // consume comma + if (!Tok.is(tok::identifier)) { + Diag(Tok.getLocation(), diag::err_expected) << tok::identifier; + SkipUntil(tok::r_paren, StopAtSemi); // skip through ) + return true; + } + unsigned RegisterSpaceVal = 0; + ParseSpaceForHLSL(Tok.getIdentifierInfo()->getName(), &RegisterSpaceVal, &diagId); + if (diagId != 0) { + Diag(Tok.getLocation(), diagId); + r.setIsValid(false); + } + else { + r.RegisterSpace = RegisterSpaceVal; + } + ConsumeToken(); // consume identifier + } + } + + if (ExpectAndConsume(tok::r_paren, diag::err_expected)) { + SkipUntil(tok::r_paren, StopAtSemi); // skip through ) + return true; + } target.push_back(new (context) hlsl::RegisterAssignment(r)); } @@ -652,18 +633,15 @@ bool Parser::MaybeParseHLSLAttributes(std::vector &ta Actions.DiagnoseSemanticDecl(pUA); ConsumeToken(); // consume semantic - // Likely a misspell of register() or a mismatching macro: - // registers() would cause a crash without this fix. + // Likely a misspell of register(), packoffset() or a mismatching macro: + // both registr() and packofset() would cause a crash without this fix. if (Tok.is(tok::l_paren)) { - - Diag(Tok.getLocation(), diag::err_hlsl_register_is_misspelled) + Diag(Tok.getLocation(), diag::err_hlsl_expected_hlsl_attribute) << semanticName; - ConsumeParen(); - hlsl::RegisterAssignment dummy; - if (ConsumeRegisterAssignment(dummy)) // Skip invalid syntax - return true; + SkipUntil(tok::r_paren, StopAtSemi); // skip through ) + return true; } target.push_back(pUA); diff --git a/tools/clang/test/SemaHLSL/hlsl-attribute-mistype-errors.hlsl b/tools/clang/test/SemaHLSL/hlsl-attribute-mistype-errors.hlsl new file mode 100644 index 0000000000..b35e1367dc --- /dev/null +++ b/tools/clang/test/SemaHLSL/hlsl-attribute-mistype-errors.hlsl @@ -0,0 +1,35 @@ +// RUN: %dxc -T lib_6_3 -verify %s + +// expected-error@+1 {{Syntax indicated an hlsl attribute (: packoffset() or : register()), but unexpected attribute 'registers' was used instead.}} +RWStructuredBuffer uav1 : registers(u3); + +// expected-error@+1 {{Syntax indicated an hlsl attribute (: packoffset() or : register()), but unexpected attribute 'registers' was used instead.}} +RWStructuredBuffer uav2 : registers(outer_space); + +// expected-error@+1 {{Syntax indicated an hlsl attribute (: packoffset() or : register()), but unexpected attribute 'UNDEFINED_MACRO1' was used instead.}} +RWStructuredBuffer uav3 : UNDEFINED_MACRO1(u3); + +// expected-error@+1 {{Syntax indicated an hlsl attribute (: packoffset() or : register()), but unexpected attribute 'UNDEFINED_MACRO' was used instead.}} +RWStructuredBuffer uav4 : UNDEFINED_MACRO(something, more, complex); + +cbuffer buf { + + // expected-error@+1 {{Syntax indicated an hlsl attribute (: packoffset() or : register()), but unexpected attribute 'packofset' was used instead.}} + float4 v0 : packofset(c0); + + // expected-error@+1 {{Syntax indicated an hlsl attribute (: packoffset() or : register()), but unexpected attribute 'UNDEFINED_MACRO2' was used instead.}} + float v1 : UNDEFINED_MACRO2(c0.w); + + // expected-error@+1 {{Syntax indicated an hlsl attribute (: packoffset() or : register()), but unexpected attribute 'UNDEFINED_MACRO' was used instead.}} + float v2 : UNDEFINED_MACRO(something, more, complex); +}; + +[shader("pixel")] +float4 main(): SV_Target +{ + uav1[0] = v0; + uav2[0] = v1; + uav3[0] = 2; + uav4[0] = 2; + return 0.xxxx; +} diff --git a/tools/clang/test/SemaHLSL/register-mistype-errors.hlsl b/tools/clang/test/SemaHLSL/register-mistype-errors.hlsl deleted file mode 100644 index cbe876b554..0000000000 --- a/tools/clang/test/SemaHLSL/register-mistype-errors.hlsl +++ /dev/null @@ -1,11 +0,0 @@ -// RUN: %dxc -T lib_6_3 -verify %s - -// expected-error@+1 {{Syntax similar to : register() was used but unexpected keyword 'registers' was used instead.}} -RWStructuredBuffer uav1 : registers(u3); - -[shader("pixel")] -float4 main(): SV_Target -{ - uav1[0] = 2; - return 0.xxxx; -} From f835f7b6938d6a221c1e86548ac2a25772611687 Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Wed, 5 Nov 2025 21:20:19 +0100 Subject: [PATCH 5/6] Changed packofset to packoffsets and added one where it also uses invalid packoffset syntax (same test as with uav2) --- .../SemaHLSL/hlsl-attribute-mistype-errors.hlsl | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/tools/clang/test/SemaHLSL/hlsl-attribute-mistype-errors.hlsl b/tools/clang/test/SemaHLSL/hlsl-attribute-mistype-errors.hlsl index b35e1367dc..5691ab68e1 100644 --- a/tools/clang/test/SemaHLSL/hlsl-attribute-mistype-errors.hlsl +++ b/tools/clang/test/SemaHLSL/hlsl-attribute-mistype-errors.hlsl @@ -14,14 +14,17 @@ RWStructuredBuffer uav4 : UNDEFINED_MACRO(something, more, complex); cbuffer buf { - // expected-error@+1 {{Syntax indicated an hlsl attribute (: packoffset() or : register()), but unexpected attribute 'packofset' was used instead.}} - float4 v0 : packofset(c0); + // expected-error@+1 {{Syntax indicated an hlsl attribute (: packoffset() or : register()), but unexpected attribute 'packoffsets' was used instead.}} + float4 v0 : packoffsets(c0); + + // expected-error@+1 {{Syntax indicated an hlsl attribute (: packoffset() or : register()), but unexpected attribute 'packoffsets' was used instead.}} + float4 v1 : packoffsets(invalid_syntax); // expected-error@+1 {{Syntax indicated an hlsl attribute (: packoffset() or : register()), but unexpected attribute 'UNDEFINED_MACRO2' was used instead.}} - float v1 : UNDEFINED_MACRO2(c0.w); + float v2 : UNDEFINED_MACRO2(c0.w); // expected-error@+1 {{Syntax indicated an hlsl attribute (: packoffset() or : register()), but unexpected attribute 'UNDEFINED_MACRO' was used instead.}} - float v2 : UNDEFINED_MACRO(something, more, complex); + float v3 : UNDEFINED_MACRO(something, more, complex); }; [shader("pixel")] @@ -29,7 +32,7 @@ float4 main(): SV_Target { uav1[0] = v0; uav2[0] = v1; - uav3[0] = 2; - uav4[0] = 2; + uav3[0] = v2; + uav4[0] = v3; return 0.xxxx; } From 6cfc632501a77c9a2ffbefbdb450b38d4828b662 Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Wed, 19 Nov 2025 22:39:16 +0100 Subject: [PATCH 6/6] Fixed the error message --- .../include/clang/Basic/DiagnosticParseKinds.td | 2 +- tools/clang/lib/Parse/ParseDecl.cpp | 3 +-- .../SemaHLSL/hlsl-attribute-mistype-errors.hlsl | 16 ++++++++-------- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/tools/clang/include/clang/Basic/DiagnosticParseKinds.td b/tools/clang/include/clang/Basic/DiagnosticParseKinds.td index dc2b76526d..0fe0d22bbf 100644 --- a/tools/clang/include/clang/Basic/DiagnosticParseKinds.td +++ b/tools/clang/include/clang/Basic/DiagnosticParseKinds.td @@ -1026,7 +1026,7 @@ def warn_hlsl_semantic_identifier_collision : Warning < "'%0' interpreted as semantic; previous definition(s) ignored">, InGroup< HLSLSemanticIdentifierCollision >; def err_hlsl_expected_hlsl_attribute : Error < - "Syntax indicated an hlsl attribute (: packoffset() or : register()), but unexpected attribute '%0' was used instead.">; + "Unexpected '(' in semantic annotation. Did you mean 'packoffset()' or 'register()'?">; def err_hlsl_enum : Error< "enum is unsupported in HLSL before 2017">; def warn_hlsl_new_feature : Warning < diff --git a/tools/clang/lib/Parse/ParseDecl.cpp b/tools/clang/lib/Parse/ParseDecl.cpp index 26fdbca301..d3f8d68443 100644 --- a/tools/clang/lib/Parse/ParseDecl.cpp +++ b/tools/clang/lib/Parse/ParseDecl.cpp @@ -637,8 +637,7 @@ bool Parser::MaybeParseHLSLAttributes(std::vector &ta // both registr() and packofset() would cause a crash without this fix. if (Tok.is(tok::l_paren)) { - Diag(Tok.getLocation(), diag::err_hlsl_expected_hlsl_attribute) - << semanticName; + Diag(Tok.getLocation(), diag::err_hlsl_expected_hlsl_attribute); ConsumeParen(); SkipUntil(tok::r_paren, StopAtSemi); // skip through ) return true; diff --git a/tools/clang/test/SemaHLSL/hlsl-attribute-mistype-errors.hlsl b/tools/clang/test/SemaHLSL/hlsl-attribute-mistype-errors.hlsl index 5691ab68e1..edfbd95799 100644 --- a/tools/clang/test/SemaHLSL/hlsl-attribute-mistype-errors.hlsl +++ b/tools/clang/test/SemaHLSL/hlsl-attribute-mistype-errors.hlsl @@ -1,29 +1,29 @@ // RUN: %dxc -T lib_6_3 -verify %s -// expected-error@+1 {{Syntax indicated an hlsl attribute (: packoffset() or : register()), but unexpected attribute 'registers' was used instead.}} +// expected-error@+1 {{Unexpected '(' in semantic annotation. Did you mean 'packoffset()' or 'register()'?}} RWStructuredBuffer uav1 : registers(u3); -// expected-error@+1 {{Syntax indicated an hlsl attribute (: packoffset() or : register()), but unexpected attribute 'registers' was used instead.}} +// expected-error@+1 {{Unexpected '(' in semantic annotation. Did you mean 'packoffset()' or 'register()'?}} RWStructuredBuffer uav2 : registers(outer_space); -// expected-error@+1 {{Syntax indicated an hlsl attribute (: packoffset() or : register()), but unexpected attribute 'UNDEFINED_MACRO1' was used instead.}} +// expected-error@+1 {{Unexpected '(' in semantic annotation. Did you mean 'packoffset()' or 'register()'?}} RWStructuredBuffer uav3 : UNDEFINED_MACRO1(u3); -// expected-error@+1 {{Syntax indicated an hlsl attribute (: packoffset() or : register()), but unexpected attribute 'UNDEFINED_MACRO' was used instead.}} +// expected-error@+1 {{Unexpected '(' in semantic annotation. Did you mean 'packoffset()' or 'register()'?}} RWStructuredBuffer uav4 : UNDEFINED_MACRO(something, more, complex); cbuffer buf { - // expected-error@+1 {{Syntax indicated an hlsl attribute (: packoffset() or : register()), but unexpected attribute 'packoffsets' was used instead.}} + // expected-error@+1 {{Unexpected '(' in semantic annotation. Did you mean 'packoffset()' or 'register()'?}} float4 v0 : packoffsets(c0); - // expected-error@+1 {{Syntax indicated an hlsl attribute (: packoffset() or : register()), but unexpected attribute 'packoffsets' was used instead.}} + // expected-error@+1 {{Unexpected '(' in semantic annotation. Did you mean 'packoffset()' or 'register()'?}} float4 v1 : packoffsets(invalid_syntax); - // expected-error@+1 {{Syntax indicated an hlsl attribute (: packoffset() or : register()), but unexpected attribute 'UNDEFINED_MACRO2' was used instead.}} + // expected-error@+1 {{Unexpected '(' in semantic annotation. Did you mean 'packoffset()' or 'register()'?}} float v2 : UNDEFINED_MACRO2(c0.w); - // expected-error@+1 {{Syntax indicated an hlsl attribute (: packoffset() or : register()), but unexpected attribute 'UNDEFINED_MACRO' was used instead.}} + // expected-error@+1 {{Unexpected '(' in semantic annotation. Did you mean 'packoffset()' or 'register()'?}} float v3 : UNDEFINED_MACRO(something, more, complex); };