[clang] f03bcb7 - [HLSL][RootSignature] Audit `RootSignatureParser` diagnostic production (#147800)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Jul 11 19:40:14 PDT 2025
Author: Finn Plummer
Date: 2025-07-11T19:40:10-07:00
New Revision: f03bcb7594254b91098f16a339dd6ba4c55a0c8e
URL: https://github.com/llvm/llvm-project/commit/f03bcb7594254b91098f16a339dd6ba4c55a0c8e
DIFF: https://github.com/llvm/llvm-project/commit/f03bcb7594254b91098f16a339dd6ba4c55a0c8e.diff
LOG: [HLSL][RootSignature] Audit `RootSignatureParser` diagnostic production (#147800)
This pr audits the diagnostics produced in `RootSignatureParser`
diagnostics.
First, it has been noted more than once that the current
`diag::err_hlsl_unexpected_end_of_params` is not direct and can be
misleading. For instance,
[here](https://github.com/llvm/llvm-project/pull/147350#discussion_r2193717272)
and
[here](https://github.com/llvm/llvm-project/pull/145827#discussion_r2169406679).
This pr address this by removing this diagnostic and replacing it with a
more direct `diag::err_expected_either`. However, doing so removes the
nuance between the case where it is a missing comma and when it was an
invalid parameter.
Hence, we introduce the `diag::err_hlsl_invalid_token` for the latter
case, which does so in a direct way. Further, we can apply the same
diagnostic to the parsing of parameter values.
As part of this, we see that there was a test gap in testing the
diagnostics produced from `diag::err_expected_after` and for the parsing
of enum/flag values. As such, these are also addressed here to provide
sufficient unit/sema test coverage.
- Removes all uses of `diag::err_hlsl_unexpected_end_of_params` in
`RootSigantureParser`
- Introduce `diag::err_hlsl_invalid_token` to provide a direct
diagnostic
- In each of these cases, replace the diagnostic with either a
`diag::err_hlsl_invalid_token` or `diag::err_expected_either`
accordingly
- Update `HLSLRootSignatureParserTest` to account for diagnostic changes
- Increase test coverage of `HLSLRootSignatureParserTest` for enum/flag
diagnostics
- Increase test coverage of `RootSignatures-err` for enum/flag
diagnostics
- Small fix-up of the `diag::err_expected_after` and add test to
demonstrate usage
Resolves: https://github.com/llvm/llvm-project/issues/147799
Added:
Modified:
clang/include/clang/Basic/DiagnosticParseKinds.td
clang/include/clang/Parse/ParseHLSLRootSignature.h
clang/lib/Parse/ParseHLSLRootSignature.cpp
clang/test/SemaHLSL/RootSignature-err.hlsl
clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 178f14d9fdd84..35903af998fbe 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1863,8 +1863,7 @@ def err_hlsl_virtual_inheritance
: Error<"virtual inheritance is unsupported in HLSL">;
// HLSL Root Signature Parser Diagnostics
-def err_hlsl_unexpected_end_of_params
- : Error<"expected %0 to denote end of parameters, or, another valid parameter of %1">;
+def err_hlsl_invalid_token : Error<"invalid %select{parameter|value}0 of %1">;
def err_hlsl_rootsig_repeat_param : Error<"specified the same parameter '%0' multiple times">;
def err_hlsl_rootsig_missing_param : Error<"did not specify mandatory parameter '%0'">;
def err_hlsl_number_literal_overflow : Error<
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index 500e79a7ba574..404bccea10c99 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -100,7 +100,8 @@ class RootSignatureParser {
std::optional<llvm::dxbc::RootDescriptorFlags> Flags;
};
std::optional<ParsedRootDescriptorParams>
- parseRootDescriptorParams(RootSignatureToken::Kind RegType);
+ parseRootDescriptorParams(RootSignatureToken::Kind DescKind,
+ RootSignatureToken::Kind RegType);
struct ParsedClauseParams {
std::optional<llvm::hlsl::rootsig::Register> Reg;
@@ -110,7 +111,8 @@ class RootSignatureParser {
std::optional<llvm::dxbc::DescriptorRangeFlags> Flags;
};
std::optional<ParsedClauseParams>
- parseDescriptorTableClauseParams(RootSignatureToken::Kind RegType);
+ parseDescriptorTableClauseParams(RootSignatureToken::Kind ClauseKind,
+ RootSignatureToken::Kind RegType);
struct ParsedStaticSamplerParams {
std::optional<llvm::hlsl::rootsig::Register> Reg;
@@ -135,13 +137,20 @@ class RootSignatureParser {
std::optional<float> parseFloatParam();
/// Parsing methods of various enums
- std::optional<llvm::dxbc::ShaderVisibility> parseShaderVisibility();
- std::optional<llvm::dxbc::SamplerFilter> parseSamplerFilter();
- std::optional<llvm::dxbc::TextureAddressMode> parseTextureAddressMode();
- std::optional<llvm::dxbc::ComparisonFunc> parseComparisonFunc();
- std::optional<llvm::dxbc::StaticBorderColor> parseStaticBorderColor();
- std::optional<llvm::dxbc::RootDescriptorFlags> parseRootDescriptorFlags();
- std::optional<llvm::dxbc::DescriptorRangeFlags> parseDescriptorRangeFlags();
+ std::optional<llvm::dxbc::ShaderVisibility>
+ parseShaderVisibility(RootSignatureToken::Kind Context);
+ std::optional<llvm::dxbc::SamplerFilter>
+ parseSamplerFilter(RootSignatureToken::Kind Context);
+ std::optional<llvm::dxbc::TextureAddressMode>
+ parseTextureAddressMode(RootSignatureToken::Kind Context);
+ std::optional<llvm::dxbc::ComparisonFunc>
+ parseComparisonFunc(RootSignatureToken::Kind Context);
+ std::optional<llvm::dxbc::StaticBorderColor>
+ parseStaticBorderColor(RootSignatureToken::Kind Context);
+ std::optional<llvm::dxbc::RootDescriptorFlags>
+ parseRootDescriptorFlags(RootSignatureToken::Kind Context);
+ std::optional<llvm::dxbc::DescriptorRangeFlags>
+ parseDescriptorRangeFlags(RootSignatureToken::Kind Context);
/// Use NumericLiteralParser to convert CurToken.NumSpelling into a unsigned
/// 32-bit integer
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 4f4bbb5ced13b..870cb88f40963 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -59,6 +59,11 @@ bool RootSignatureParser::parse() {
if (!Sampler.has_value())
return true;
Elements.emplace_back(ElementLoc, *Sampler);
+ } else {
+ consumeNextToken(); // let diagnostic be at the start of invalid token
+ reportDiag(diag::err_hlsl_invalid_token)
+ << /*parameter=*/0 << /*param of*/ TokenKind::kw_RootSignature;
+ return true;
}
// ',' denotes another element, otherwise, expected to be at end of stream
@@ -67,8 +72,7 @@ bool RootSignatureParser::parse() {
}
return consumeExpectedToken(TokenKind::end_of_stream,
- diag::err_hlsl_unexpected_end_of_params,
- /*param of=*/TokenKind::kw_RootSignature);
+ diag::err_expected_either, TokenKind::pu_comma);
}
template <typename FlagType>
@@ -90,6 +94,10 @@ std::optional<llvm::dxbc::RootFlags> RootSignatureParser::parseRootFlags() {
std::optional<llvm::dxbc::RootFlags> Flags = llvm::dxbc::RootFlags::None;
+ // Handle valid empty case
+ if (tryConsumeExpectedToken(TokenKind::pu_r_paren))
+ return Flags;
+
// Handle the edge-case of '0' to specify no flags set
if (tryConsumeExpectedToken(TokenKind::int_literal)) {
if (!verifyZeroFlag()) {
@@ -115,13 +123,17 @@ std::optional<llvm::dxbc::RootFlags> RootSignatureParser::parseRootFlags() {
default:
llvm_unreachable("Switch for consumed enum token was not provided");
}
+ } else {
+ consumeNextToken(); // consume token to point at invalid token
+ reportDiag(diag::err_hlsl_invalid_token)
+ << /*value=*/1 << /*value of*/ TokenKind::kw_RootFlags;
+ return std::nullopt;
}
} while (tryConsumeExpectedToken(TokenKind::pu_or));
}
- if (consumeExpectedToken(TokenKind::pu_r_paren,
- diag::err_hlsl_unexpected_end_of_params,
- /*param of=*/TokenKind::kw_RootFlags))
+ if (consumeExpectedToken(TokenKind::pu_r_paren, diag::err_expected_either,
+ TokenKind::pu_comma))
return std::nullopt;
return Flags;
@@ -141,9 +153,8 @@ std::optional<RootConstants> RootSignatureParser::parseRootConstants() {
if (!Params.has_value())
return std::nullopt;
- if (consumeExpectedToken(TokenKind::pu_r_paren,
- diag::err_hlsl_unexpected_end_of_params,
- /*param of=*/TokenKind::kw_RootConstants))
+ if (consumeExpectedToken(TokenKind::pu_r_paren, diag::err_expected_either,
+ TokenKind::pu_comma))
return std::nullopt;
// Check mandatory parameters where provided
@@ -204,13 +215,12 @@ std::optional<RootDescriptor> RootSignatureParser::parseRootDescriptor() {
}
Descriptor.setDefaultFlags(Version);
- auto Params = parseRootDescriptorParams(ExpectedReg);
+ auto Params = parseRootDescriptorParams(DescriptorKind, ExpectedReg);
if (!Params.has_value())
return std::nullopt;
- if (consumeExpectedToken(TokenKind::pu_r_paren,
- diag::err_hlsl_unexpected_end_of_params,
- /*param of=*/DescriptorKind))
+ if (consumeExpectedToken(TokenKind::pu_r_paren, diag::err_expected_either,
+ TokenKind::pu_comma))
return std::nullopt;
// Check mandatory parameters were provided
@@ -266,9 +276,14 @@ std::optional<DescriptorTable> RootSignatureParser::parseDescriptorTable() {
if (consumeExpectedToken(TokenKind::pu_equal))
return std::nullopt;
- Visibility = parseShaderVisibility();
+ Visibility = parseShaderVisibility(TokenKind::kw_visibility);
if (!Visibility.has_value())
return std::nullopt;
+ } else {
+ consumeNextToken(); // let diagnostic be at the start of invalid token
+ reportDiag(diag::err_hlsl_invalid_token)
+ << /*parameter=*/0 << /*param of*/ TokenKind::kw_DescriptorTable;
+ return std::nullopt;
}
// ',' denotes another element, otherwise, expected to be at ')'
@@ -276,9 +291,8 @@ std::optional<DescriptorTable> RootSignatureParser::parseDescriptorTable() {
break;
}
- if (consumeExpectedToken(TokenKind::pu_r_paren,
- diag::err_hlsl_unexpected_end_of_params,
- /*param of=*/TokenKind::kw_DescriptorTable))
+ if (consumeExpectedToken(TokenKind::pu_r_paren, diag::err_expected_either,
+ TokenKind::pu_comma))
return std::nullopt;
// Fill in optional visibility
@@ -326,13 +340,12 @@ RootSignatureParser::parseDescriptorTableClause() {
}
Clause.setDefaultFlags(Version);
- auto Params = parseDescriptorTableClauseParams(ExpectedReg);
+ auto Params = parseDescriptorTableClauseParams(ParamKind, ExpectedReg);
if (!Params.has_value())
return std::nullopt;
- if (consumeExpectedToken(TokenKind::pu_r_paren,
- diag::err_hlsl_unexpected_end_of_params,
- /*param of=*/ParamKind))
+ if (consumeExpectedToken(TokenKind::pu_r_paren, diag::err_expected_either,
+ TokenKind::pu_comma))
return std::nullopt;
// Check mandatory parameters were provided
@@ -373,9 +386,8 @@ std::optional<StaticSampler> RootSignatureParser::parseStaticSampler() {
if (!Params.has_value())
return std::nullopt;
- if (consumeExpectedToken(TokenKind::pu_r_paren,
- diag::err_hlsl_unexpected_end_of_params,
- /*param of=*/TokenKind::kw_StaticSampler))
+ if (consumeExpectedToken(TokenKind::pu_r_paren, diag::err_expected_either,
+ TokenKind::pu_comma))
return std::nullopt;
// Check mandatory parameters were provided
@@ -484,10 +496,15 @@ RootSignatureParser::parseRootConstantParams() {
if (consumeExpectedToken(TokenKind::pu_equal))
return std::nullopt;
- auto Visibility = parseShaderVisibility();
+ auto Visibility = parseShaderVisibility(TokenKind::kw_visibility);
if (!Visibility.has_value())
return std::nullopt;
Params.Visibility = Visibility;
+ } else {
+ consumeNextToken(); // let diagnostic be at the start of invalid token
+ reportDiag(diag::err_hlsl_invalid_token)
+ << /*parameter=*/0 << /*param of*/ TokenKind::kw_RootConstants;
+ return std::nullopt;
}
// ',' denotes another element, otherwise, expected to be at ')'
@@ -499,7 +516,8 @@ RootSignatureParser::parseRootConstantParams() {
}
std::optional<RootSignatureParser::ParsedRootDescriptorParams>
-RootSignatureParser::parseRootDescriptorParams(TokenKind RegType) {
+RootSignatureParser::parseRootDescriptorParams(TokenKind DescKind,
+ TokenKind RegType) {
assert(CurToken.TokKind == TokenKind::pu_l_paren &&
"Expects to only be invoked starting at given token");
@@ -539,7 +557,7 @@ RootSignatureParser::parseRootDescriptorParams(TokenKind RegType) {
if (consumeExpectedToken(TokenKind::pu_equal))
return std::nullopt;
- auto Visibility = parseShaderVisibility();
+ auto Visibility = parseShaderVisibility(TokenKind::kw_visibility);
if (!Visibility.has_value())
return std::nullopt;
Params.Visibility = Visibility;
@@ -553,10 +571,15 @@ RootSignatureParser::parseRootDescriptorParams(TokenKind RegType) {
if (consumeExpectedToken(TokenKind::pu_equal))
return std::nullopt;
- auto Flags = parseRootDescriptorFlags();
+ auto Flags = parseRootDescriptorFlags(TokenKind::kw_flags);
if (!Flags.has_value())
return std::nullopt;
Params.Flags = Flags;
+ } else {
+ consumeNextToken(); // let diagnostic be at the start of invalid token
+ reportDiag(diag::err_hlsl_invalid_token)
+ << /*parameter=*/0 << /*param of*/ DescKind;
+ return std::nullopt;
}
// ',' denotes another element, otherwise, expected to be at ')'
@@ -568,7 +591,8 @@ RootSignatureParser::parseRootDescriptorParams(TokenKind RegType) {
}
std::optional<RootSignatureParser::ParsedClauseParams>
-RootSignatureParser::parseDescriptorTableClauseParams(TokenKind RegType) {
+RootSignatureParser::parseDescriptorTableClauseParams(TokenKind ClauseKind,
+ TokenKind RegType) {
assert(CurToken.TokKind == TokenKind::pu_l_paren &&
"Expects to only be invoked starting at given token");
@@ -648,10 +672,15 @@ RootSignatureParser::parseDescriptorTableClauseParams(TokenKind RegType) {
if (consumeExpectedToken(TokenKind::pu_equal))
return std::nullopt;
- auto Flags = parseDescriptorRangeFlags();
+ auto Flags = parseDescriptorRangeFlags(TokenKind::kw_flags);
if (!Flags.has_value())
return std::nullopt;
Params.Flags = Flags;
+ } else {
+ consumeNextToken(); // let diagnostic be at the start of invalid token
+ reportDiag(diag::err_hlsl_invalid_token)
+ << /*parameter=*/0 << /*param of*/ ClauseKind;
+ return std::nullopt;
}
// ',' denotes another element, otherwise, expected to be at ')'
@@ -689,7 +718,7 @@ RootSignatureParser::parseStaticSamplerParams() {
if (consumeExpectedToken(TokenKind::pu_equal))
return std::nullopt;
- auto Filter = parseSamplerFilter();
+ auto Filter = parseSamplerFilter(TokenKind::kw_filter);
if (!Filter.has_value())
return std::nullopt;
Params.Filter = Filter;
@@ -703,7 +732,7 @@ RootSignatureParser::parseStaticSamplerParams() {
if (consumeExpectedToken(TokenKind::pu_equal))
return std::nullopt;
- auto AddressU = parseTextureAddressMode();
+ auto AddressU = parseTextureAddressMode(TokenKind::kw_addressU);
if (!AddressU.has_value())
return std::nullopt;
Params.AddressU = AddressU;
@@ -717,7 +746,7 @@ RootSignatureParser::parseStaticSamplerParams() {
if (consumeExpectedToken(TokenKind::pu_equal))
return std::nullopt;
- auto AddressV = parseTextureAddressMode();
+ auto AddressV = parseTextureAddressMode(TokenKind::kw_addressV);
if (!AddressV.has_value())
return std::nullopt;
Params.AddressV = AddressV;
@@ -731,7 +760,7 @@ RootSignatureParser::parseStaticSamplerParams() {
if (consumeExpectedToken(TokenKind::pu_equal))
return std::nullopt;
- auto AddressW = parseTextureAddressMode();
+ auto AddressW = parseTextureAddressMode(TokenKind::kw_addressW);
if (!AddressW.has_value())
return std::nullopt;
Params.AddressW = AddressW;
@@ -773,7 +802,7 @@ RootSignatureParser::parseStaticSamplerParams() {
if (consumeExpectedToken(TokenKind::pu_equal))
return std::nullopt;
- auto CompFunc = parseComparisonFunc();
+ auto CompFunc = parseComparisonFunc(TokenKind::kw_comparisonFunc);
if (!CompFunc.has_value())
return std::nullopt;
Params.CompFunc = CompFunc;
@@ -787,7 +816,7 @@ RootSignatureParser::parseStaticSamplerParams() {
if (consumeExpectedToken(TokenKind::pu_equal))
return std::nullopt;
- auto BorderColor = parseStaticBorderColor();
+ auto BorderColor = parseStaticBorderColor(TokenKind::kw_borderColor);
if (!BorderColor.has_value())
return std::nullopt;
Params.BorderColor = BorderColor;
@@ -843,10 +872,15 @@ RootSignatureParser::parseStaticSamplerParams() {
if (consumeExpectedToken(TokenKind::pu_equal))
return std::nullopt;
- auto Visibility = parseShaderVisibility();
+ auto Visibility = parseShaderVisibility(TokenKind::kw_visibility);
if (!Visibility.has_value())
return std::nullopt;
Params.Visibility = Visibility;
+ } else {
+ consumeNextToken(); // let diagnostic be at the start of invalid token
+ reportDiag(diag::err_hlsl_invalid_token)
+ << /*parameter=*/0 << /*param of*/ TokenKind::kw_StaticSampler;
+ return std::nullopt;
}
// ',' denotes another element, otherwise, expected to be at ')'
@@ -934,7 +968,7 @@ std::optional<float> RootSignatureParser::parseFloatParam() {
}
std::optional<llvm::dxbc::ShaderVisibility>
-RootSignatureParser::parseShaderVisibility() {
+RootSignatureParser::parseShaderVisibility(TokenKind Context) {
assert(CurToken.TokKind == TokenKind::pu_equal &&
"Expects to only be invoked starting at given keyword");
@@ -943,8 +977,12 @@ RootSignatureParser::parseShaderVisibility() {
#include "clang/Lex/HLSLRootSignatureTokenKinds.def"
};
- if (!tryConsumeExpectedToken(Expected))
+ if (!tryConsumeExpectedToken(Expected)) {
+ consumeNextToken(); // consume token to point at invalid token
+ reportDiag(diag::err_hlsl_invalid_token)
+ << /*value=*/1 << /*value of*/ Context;
return std::nullopt;
+ }
switch (CurToken.TokKind) {
#define SHADER_VISIBILITY_ENUM(NAME, LIT) \
@@ -960,7 +998,7 @@ RootSignatureParser::parseShaderVisibility() {
}
std::optional<llvm::dxbc::SamplerFilter>
-RootSignatureParser::parseSamplerFilter() {
+RootSignatureParser::parseSamplerFilter(TokenKind Context) {
assert(CurToken.TokKind == TokenKind::pu_equal &&
"Expects to only be invoked starting at given keyword");
@@ -969,8 +1007,12 @@ RootSignatureParser::parseSamplerFilter() {
#include "clang/Lex/HLSLRootSignatureTokenKinds.def"
};
- if (!tryConsumeExpectedToken(Expected))
+ if (!tryConsumeExpectedToken(Expected)) {
+ consumeNextToken(); // consume token to point at invalid token
+ reportDiag(diag::err_hlsl_invalid_token)
+ << /*value=*/1 << /*value of*/ Context;
return std::nullopt;
+ }
switch (CurToken.TokKind) {
#define FILTER_ENUM(NAME, LIT) \
@@ -986,7 +1028,7 @@ RootSignatureParser::parseSamplerFilter() {
}
std::optional<llvm::dxbc::TextureAddressMode>
-RootSignatureParser::parseTextureAddressMode() {
+RootSignatureParser::parseTextureAddressMode(TokenKind Context) {
assert(CurToken.TokKind == TokenKind::pu_equal &&
"Expects to only be invoked starting at given keyword");
@@ -995,8 +1037,12 @@ RootSignatureParser::parseTextureAddressMode() {
#include "clang/Lex/HLSLRootSignatureTokenKinds.def"
};
- if (!tryConsumeExpectedToken(Expected))
+ if (!tryConsumeExpectedToken(Expected)) {
+ consumeNextToken(); // consume token to point at invalid token
+ reportDiag(diag::err_hlsl_invalid_token)
+ << /*value=*/1 << /*value of*/ Context;
return std::nullopt;
+ }
switch (CurToken.TokKind) {
#define TEXTURE_ADDRESS_MODE_ENUM(NAME, LIT) \
@@ -1012,7 +1058,7 @@ RootSignatureParser::parseTextureAddressMode() {
}
std::optional<llvm::dxbc::ComparisonFunc>
-RootSignatureParser::parseComparisonFunc() {
+RootSignatureParser::parseComparisonFunc(TokenKind Context) {
assert(CurToken.TokKind == TokenKind::pu_equal &&
"Expects to only be invoked starting at given keyword");
@@ -1021,8 +1067,12 @@ RootSignatureParser::parseComparisonFunc() {
#include "clang/Lex/HLSLRootSignatureTokenKinds.def"
};
- if (!tryConsumeExpectedToken(Expected))
+ if (!tryConsumeExpectedToken(Expected)) {
+ consumeNextToken(); // consume token to point at invalid token
+ reportDiag(diag::err_hlsl_invalid_token)
+ << /*value=*/1 << /*value of*/ Context;
return std::nullopt;
+ }
switch (CurToken.TokKind) {
#define COMPARISON_FUNC_ENUM(NAME, LIT) \
@@ -1038,7 +1088,7 @@ RootSignatureParser::parseComparisonFunc() {
}
std::optional<llvm::dxbc::StaticBorderColor>
-RootSignatureParser::parseStaticBorderColor() {
+RootSignatureParser::parseStaticBorderColor(TokenKind Context) {
assert(CurToken.TokKind == TokenKind::pu_equal &&
"Expects to only be invoked starting at given keyword");
@@ -1047,8 +1097,12 @@ RootSignatureParser::parseStaticBorderColor() {
#include "clang/Lex/HLSLRootSignatureTokenKinds.def"
};
- if (!tryConsumeExpectedToken(Expected))
+ if (!tryConsumeExpectedToken(Expected)) {
+ consumeNextToken(); // consume token to point at invalid token
+ reportDiag(diag::err_hlsl_invalid_token)
+ << /*value=*/1 << /*value of*/ Context;
return std::nullopt;
+ }
switch (CurToken.TokKind) {
#define STATIC_BORDER_COLOR_ENUM(NAME, LIT) \
@@ -1064,7 +1118,7 @@ RootSignatureParser::parseStaticBorderColor() {
}
std::optional<llvm::dxbc::RootDescriptorFlags>
-RootSignatureParser::parseRootDescriptorFlags() {
+RootSignatureParser::parseRootDescriptorFlags(TokenKind Context) {
assert(CurToken.TokKind == TokenKind::pu_equal &&
"Expects to only be invoked starting at given keyword");
@@ -1096,6 +1150,11 @@ RootSignatureParser::parseRootDescriptorFlags() {
default:
llvm_unreachable("Switch for consumed enum token was not provided");
}
+ } else {
+ consumeNextToken(); // consume token to point at invalid token
+ reportDiag(diag::err_hlsl_invalid_token)
+ << /*value=*/1 << /*value of*/ Context;
+ return std::nullopt;
}
} while (tryConsumeExpectedToken(TokenKind::pu_or));
@@ -1103,7 +1162,7 @@ RootSignatureParser::parseRootDescriptorFlags() {
}
std::optional<llvm::dxbc::DescriptorRangeFlags>
-RootSignatureParser::parseDescriptorRangeFlags() {
+RootSignatureParser::parseDescriptorRangeFlags(TokenKind Context) {
assert(CurToken.TokKind == TokenKind::pu_equal &&
"Expects to only be invoked starting at given keyword");
@@ -1135,6 +1194,11 @@ RootSignatureParser::parseDescriptorRangeFlags() {
default:
llvm_unreachable("Switch for consumed enum token was not provided");
}
+ } else {
+ consumeNextToken(); // consume token to point at invalid token
+ reportDiag(diag::err_hlsl_invalid_token)
+ << /*value=*/1 << /*value of*/ Context;
+ return std::nullopt;
}
} while (tryConsumeExpectedToken(TokenKind::pu_or));
@@ -1282,11 +1346,12 @@ bool RootSignatureParser::consumeExpectedToken(TokenKind Expected,
case diag::err_expected:
DB << Expected;
break;
- case diag::err_hlsl_unexpected_end_of_params:
case diag::err_expected_either:
- case diag::err_expected_after:
DB << Expected << Context;
break;
+ case diag::err_expected_after:
+ DB << Context << Expected;
+ break;
default:
break;
}
diff --git a/clang/test/SemaHLSL/RootSignature-err.hlsl b/clang/test/SemaHLSL/RootSignature-err.hlsl
index 04013974d28b9..4b53a127d2adb 100644
--- a/clang/test/SemaHLSL/RootSignature-err.hlsl
+++ b/clang/test/SemaHLSL/RootSignature-err.hlsl
@@ -17,24 +17,89 @@ void bad_root_signature_2() {}
[RootSignature(""), RootSignature("")] // expected-warning {{attribute 'RootSignature' is already applied}}
void bad_root_signature_3() {}
-[RootSignature("DescriptorTable(), invalid")] // expected-error {{expected end of stream to denote end of parameters, or, another valid parameter of RootSignature}}
+// expected-error at +1 {{invalid parameter of RootSignature}}
+[RootSignature("DescriptorTable(), invalid")]
void bad_root_signature_4() {}
-// expected-error at +1 {{expected ')' to denote end of parameters, or, another valid parameter of RootConstants}}
-[RootSignature("RootConstants(b0, num32BitConstants = 1, invalid)")]
+// expected-error at +1 {{expected ')' or ','}}
+[RootSignature("RootConstants(b0 num32BitConstants = 1)")]
void bad_root_signature_5() {}
#define MultiLineRootSignature \
"CBV(b0)," \
"RootConstants(num32BitConstants = 3, b0, invalid)"
-// CHECK: [[@LINE-2]]:42: note: expanded from macro 'MultiLineRootSignature'
+// CHECK: [[@LINE-2]]:44: note: expanded from macro 'MultiLineRootSignature'
// CHECK-NEXT: [[@LINE-3]] | "RootConstants(num32BitConstants = 3, b0, invalid)"
-// CHECK-NEXT: | ^
-// expected-error at +1 {{expected ')' to denote end of parameters, or, another valid parameter of RootConstants}}
+// CHECK-NEXT: | ^
+// expected-error at +1 {{invalid parameter of RootConstants}}
[RootSignature(MultiLineRootSignature)]
void bad_root_signature_6() {}
-// expected-error at +1 {{expected end of stream to denote end of parameters, or, another valid parameter of RootSignature}}
+// expected-error at +1 {{expected end of stream or ','}}
[RootSignature("RootFlags() RootConstants(b0, num32BitConstants = 1)")]
void bad_root_signature_7() {}
+
+// expected-error at +1 {{invalid parameter of RootConstants}}
+[RootSignature("RootConstants(b0, num32BitConstantsTypo = 1))")]
+void bad_root_signature_8() {}
+
+// expected-error at +1 {{invalid parameter of UAV}}
+[RootSignature("UAV(b3")]
+void bad_root_signature_9() {}
+
+// expected-error at +1 {{invalid parameter of SRV}}
+[RootSignature("DescriptorTable(SRV(s1, invalid))")]
+void bad_root_signature_10() {}
+
+// expected-error at +1 {{invalid parameter of DescriptorTable}}
+[RootSignature("DescriptorTable(invalid))")]
+void bad_root_signature_11() {}
+
+// expected-error at +1 {{expected integer literal after '+'}}
+[RootSignature("CBV(space = +invalid))")]
+void bad_root_signature_12() {}
+
+// expected-error at +1 {{expected integer literal after '='}}
+[RootSignature("CBV(space = invalid))")]
+void bad_root_signature_13() {}
+
+// expected-error at +1 {{expected '(' after UAV}}
+[RootSignature("UAV invalid")]
+void bad_root_signature_14() {}
+
+// expected-error at +1 {{invalid value of visibility}}
+[RootSignature("StaticSampler(s0, visibility = visibility_typo)")]
+void bad_root_signature_15() {}
+
+// expected-error at +1 {{invalid value of filter}}
+[RootSignature("StaticSampler(s0, filter = filter_typo)")]
+void bad_root_signature_16() {}
+
+// expected-error at +1 {{invalid value of addressU}}
+[RootSignature("StaticSampler(s0, addressU = addressU_typo)")]
+void bad_root_signature_17() {}
+
+// expected-error at +1 {{invalid value of addressV}}
+[RootSignature("StaticSampler(s0, addressV = addressV_typo)")]
+void bad_root_signature_18() {}
+
+// expected-error at +1 {{invalid value of comparisonFunc}}
+[RootSignature("StaticSampler(s0, comparisonFunc = comparisonFunc_typo)")]
+void bad_root_signature_19() {}
+
+// expected-error at +1 {{invalid value of borderColor}}
+[RootSignature("StaticSampler(s0, borderColor = borderColor_typo)")]
+void bad_root_signature_20() {}
+
+// expected-error at +1 {{invalid value of flags}}
+[RootSignature("CBV(b0, flags = DATA_VOLATILE | root_descriptor_flag_typo)")]
+void bad_root_signature_21() {}
+
+// expected-error at +1 {{invalid value of flags}}
+[RootSignature("DescriptorTable(SRV(t0, flags = descriptor_range_flag_typo)")]
+void bad_root_signature_22() {}
+
+// expected-error at +1 {{invalid value of RootFlags}}
+[RootSignature("RootFlags(local_root_signature | root_flag_typo)")]
+void bad_root_signature_23() {}
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index 5b1ab149f4497..4b24800489c8e 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -843,7 +843,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidParseUnexpectedTokenTest) {
Signature, *PP);
// Test correct diagnostic produced
- Consumer->setExpected(diag::err_hlsl_unexpected_end_of_params);
+ Consumer->setExpected(diag::err_expected_either);
ASSERT_TRUE(Parser.parse());
ASSERT_TRUE(Consumer->isSatisfied());
@@ -865,7 +865,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidParseInvalidTokenTest) {
Signature, *PP);
// Test correct diagnostic produced - invalid token
- Consumer->setExpected(diag::err_hlsl_unexpected_end_of_params);
+ Consumer->setExpected(diag::err_hlsl_invalid_token);
ASSERT_TRUE(Parser.parse());
ASSERT_TRUE(Consumer->isSatisfied());
@@ -886,7 +886,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidParseUnexpectedEndOfStreamTest) {
hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements,
Signature, *PP);
- // Test correct diagnostic produced - end of stream
+ // Test correct diagnostic produced - expected '(' after DescriptorTable
Consumer->setExpected(diag::err_expected_after);
ASSERT_TRUE(Parser.parse());
@@ -1258,7 +1258,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidRootElementMissingCommaTest) {
Signature, *PP);
// Test correct diagnostic produced
- Consumer->setExpected(diag::err_hlsl_unexpected_end_of_params);
+ Consumer->setExpected(diag::err_expected_either);
ASSERT_TRUE(Parser.parse());
ASSERT_TRUE(Consumer->isSatisfied());
@@ -1285,7 +1285,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidDescriptorTableMissingCommaTest) {
Signature, *PP);
// Test correct diagnostic produced
- Consumer->setExpected(diag::err_hlsl_unexpected_end_of_params);
+ Consumer->setExpected(diag::err_expected_either);
ASSERT_TRUE(Parser.parse());
ASSERT_TRUE(Consumer->isSatisfied());
@@ -1312,7 +1312,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidRootConstantParamsCommaTest) {
Signature, *PP);
// Test correct diagnostic produced
- Consumer->setExpected(diag::err_hlsl_unexpected_end_of_params);
+ Consumer->setExpected(diag::err_expected_either);
ASSERT_TRUE(Parser.parse());
ASSERT_TRUE(Consumer->isSatisfied());
@@ -1339,7 +1339,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidRootDescriptorParamsCommaTest) {
Signature, *PP);
// Test correct diagnostic produced
- Consumer->setExpected(diag::err_hlsl_unexpected_end_of_params);
+ Consumer->setExpected(diag::err_expected_either);
ASSERT_TRUE(Parser.parse());
ASSERT_TRUE(Consumer->isSatisfied());
@@ -1368,7 +1368,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidDescriptorClauseParamsCommaTest) {
Signature, *PP);
// Test correct diagnostic produced
- Consumer->setExpected(diag::err_hlsl_unexpected_end_of_params);
+ Consumer->setExpected(diag::err_expected_either);
ASSERT_TRUE(Parser.parse());
ASSERT_TRUE(Consumer->isSatisfied());
@@ -1395,7 +1395,350 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidStaticSamplerCommaTest) {
Signature, *PP);
// Test correct diagnostic produced
- Consumer->setExpected(diag::err_hlsl_unexpected_end_of_params);
+ Consumer->setExpected(diag::err_expected_either);
+ ASSERT_TRUE(Parser.parse());
+
+ ASSERT_TRUE(Consumer->isSatisfied());
+}
+
+TEST_F(ParseHLSLRootSignatureTest, InvalidRootDescriptorParamTest) {
+ // This test will check that an error is produced when there is a invalid
+ // value of a parameter
+ const llvm::StringLiteral Source = R"cc(
+ SRV(t0, invalid)
+ )cc";
+
+ auto Ctx = createMinimalASTContext();
+ StringLiteral *Signature = wrapSource(Ctx, Source);
+
+ TrivialModuleLoader ModLoader;
+ auto PP = createPP(Source, ModLoader);
+
+ SmallVector<RootSignatureElement> Elements;
+ hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements,
+ Signature, *PP);
+
+ // Test correct diagnostic produced
+ Consumer->setExpected(diag::err_hlsl_invalid_token);
+ ASSERT_TRUE(Parser.parse());
+
+ ASSERT_TRUE(Consumer->isSatisfied());
+}
+
+TEST_F(ParseHLSLRootSignatureTest, InvalidDescriptorTableParamTest) {
+ // This test will check that an error is produced when there is a invalid
+ // value of a parameter
+ const llvm::StringLiteral Source = R"cc(
+ DescriptorTable(
+ visibility = SHADER_VISIBILITY_ALL,
+ invalid
+ )
+ )cc";
+
+ auto Ctx = createMinimalASTContext();
+ StringLiteral *Signature = wrapSource(Ctx, Source);
+
+ TrivialModuleLoader ModLoader;
+ auto PP = createPP(Source, ModLoader);
+
+ SmallVector<RootSignatureElement> Elements;
+ hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements,
+ Signature, *PP);
+
+ // Test correct diagnostic produced
+ Consumer->setExpected(diag::err_hlsl_invalid_token);
+ ASSERT_TRUE(Parser.parse());
+
+ ASSERT_TRUE(Consumer->isSatisfied());
+}
+
+TEST_F(ParseHLSLRootSignatureTest, InvalidDescriptorTableClauseParamTest) {
+ // This test will check that an error is produced when there is a invalid
+ // value of a parameter
+ const llvm::StringLiteral Source = R"cc(
+ DescriptorTable(
+ CBV(invalid)
+ )
+ )cc";
+
+ auto Ctx = createMinimalASTContext();
+ StringLiteral *Signature = wrapSource(Ctx, Source);
+
+ TrivialModuleLoader ModLoader;
+ auto PP = createPP(Source, ModLoader);
+
+ SmallVector<RootSignatureElement> Elements;
+ hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements,
+ Signature, *PP);
+
+ // Test correct diagnostic produced
+ Consumer->setExpected(diag::err_hlsl_invalid_token);
+ ASSERT_TRUE(Parser.parse());
+
+ ASSERT_TRUE(Consumer->isSatisfied());
+}
+
+TEST_F(ParseHLSLRootSignatureTest, InvalidStaticSamplerParamTest) {
+ // This test will check that an error is produced when there is a invalid
+ // value of a parameter
+ const llvm::StringLiteral Source = R"cc(
+ StaticSampler(
+ s0,
+ filter = FILTER_MAXIMUM_MIN_POINT_MAG_LINEAR_MIP_POINT,
+ invalid,
+ comparisonFunc = COMPARISON_EQUAL,
+ )
+ )cc";
+
+ auto Ctx = createMinimalASTContext();
+ StringLiteral *Signature = wrapSource(Ctx, Source);
+
+ TrivialModuleLoader ModLoader;
+ auto PP = createPP(Source, ModLoader);
+
+ SmallVector<RootSignatureElement> Elements;
+ hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements,
+ Signature, *PP);
+
+ // Test correct diagnostic produced
+ Consumer->setExpected(diag::err_hlsl_invalid_token);
+ ASSERT_TRUE(Parser.parse());
+
+ ASSERT_TRUE(Consumer->isSatisfied());
+}
+
+TEST_F(ParseHLSLRootSignatureTest, InvalidVisibilityValueTest) {
+ // This test will check that an error is produced when there is a invalid
+ // value of a parameter
+ const llvm::StringLiteral Source = R"cc(
+ UAV(
+ u0,
+ visibility = SHADER_VISIBILITY_TYPO
+ )
+ )cc";
+
+ auto Ctx = createMinimalASTContext();
+ StringLiteral *Signature = wrapSource(Ctx, Source);
+
+ TrivialModuleLoader ModLoader;
+ auto PP = createPP(Source, ModLoader);
+
+ SmallVector<RootSignatureElement> Elements;
+ hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements,
+ Signature, *PP);
+
+ // Test correct diagnostic produced
+ Consumer->setExpected(diag::err_hlsl_invalid_token);
+ ASSERT_TRUE(Parser.parse());
+
+ ASSERT_TRUE(Consumer->isSatisfied());
+}
+
+TEST_F(ParseHLSLRootSignatureTest, InvalidRegisterValueTest) {
+ // This test will check that an error is produced when there is a invalid
+ // value of a parameter
+ const llvm::StringLiteral Source = R"cc(
+ StaticSampler(
+ b0
+ )
+ )cc";
+
+ auto Ctx = createMinimalASTContext();
+ StringLiteral *Signature = wrapSource(Ctx, Source);
+
+ TrivialModuleLoader ModLoader;
+ auto PP = createPP(Source, ModLoader);
+
+ SmallVector<RootSignatureElement> Elements;
+ hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements,
+ Signature, *PP);
+
+ // Test correct diagnostic produced
+ Consumer->setExpected(diag::err_hlsl_invalid_token);
+ ASSERT_TRUE(Parser.parse());
+
+ ASSERT_TRUE(Consumer->isSatisfied());
+}
+
+TEST_F(ParseHLSLRootSignatureTest, InvalidFilterValueTest) {
+ // This test will check that an error is produced when there is a invalid
+ // value of a parameter
+ const llvm::StringLiteral Source = R"cc(
+ StaticSampler(
+ s0,
+ filter = FILTER_TYPO
+ )
+ )cc";
+
+ auto Ctx = createMinimalASTContext();
+ StringLiteral *Signature = wrapSource(Ctx, Source);
+
+ TrivialModuleLoader ModLoader;
+ auto PP = createPP(Source, ModLoader);
+
+ SmallVector<RootSignatureElement> Elements;
+ hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements,
+ Signature, *PP);
+
+ // Test correct diagnostic produced
+ Consumer->setExpected(diag::err_hlsl_invalid_token);
+ ASSERT_TRUE(Parser.parse());
+
+ ASSERT_TRUE(Consumer->isSatisfied());
+}
+
+TEST_F(ParseHLSLRootSignatureTest, InvalidTextureAddressModeValueTest) {
+ // This test will check that an error is produced when there is a invalid
+ // value of a parameter
+ const llvm::StringLiteral Source = R"cc(
+ StaticSampler(
+ s0,
+ addressU = TEXTURE_ADDRESS_MODE_TYPO
+ )
+ )cc";
+
+ auto Ctx = createMinimalASTContext();
+ StringLiteral *Signature = wrapSource(Ctx, Source);
+
+ TrivialModuleLoader ModLoader;
+ auto PP = createPP(Source, ModLoader);
+
+ SmallVector<RootSignatureElement> Elements;
+ hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements,
+ Signature, *PP);
+
+ // Test correct diagnostic produced
+ Consumer->setExpected(diag::err_hlsl_invalid_token);
+ ASSERT_TRUE(Parser.parse());
+
+ ASSERT_TRUE(Consumer->isSatisfied());
+}
+
+TEST_F(ParseHLSLRootSignatureTest, InvalidComparisonFuncValueTest) {
+ // This test will check that an error is produced when there is a invalid
+ // value of a parameter
+ const llvm::StringLiteral Source = R"cc(
+ StaticSampler(
+ s0,
+ comparisonFunc = COMPARISON_FUNC_TYPO
+ )
+ )cc";
+
+ auto Ctx = createMinimalASTContext();
+ StringLiteral *Signature = wrapSource(Ctx, Source);
+
+ TrivialModuleLoader ModLoader;
+ auto PP = createPP(Source, ModLoader);
+
+ SmallVector<RootSignatureElement> Elements;
+ hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements,
+ Signature, *PP);
+
+ // Test correct diagnostic produced
+ Consumer->setExpected(diag::err_hlsl_invalid_token);
+ ASSERT_TRUE(Parser.parse());
+
+ ASSERT_TRUE(Consumer->isSatisfied());
+}
+
+TEST_F(ParseHLSLRootSignatureTest, InvalidStaticBorderColorValueTest) {
+ // This test will check that an error is produced when there is a invalid
+ // value of a parameter
+ const llvm::StringLiteral Source = R"cc(
+ StaticSampler(
+ s0,
+ borderColor = STATIC_BORDER_COLOR_TYPO
+ )
+ )cc";
+
+ auto Ctx = createMinimalASTContext();
+ StringLiteral *Signature = wrapSource(Ctx, Source);
+
+ TrivialModuleLoader ModLoader;
+ auto PP = createPP(Source, ModLoader);
+
+ SmallVector<RootSignatureElement> Elements;
+ hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements,
+ Signature, *PP);
+
+ // Test correct diagnostic produced
+ Consumer->setExpected(diag::err_hlsl_invalid_token);
+ ASSERT_TRUE(Parser.parse());
+
+ ASSERT_TRUE(Consumer->isSatisfied());
+}
+
+TEST_F(ParseHLSLRootSignatureTest, InvalidRootFlagsValueTest) {
+ // This test will check that an error is produced when there is a invalid
+ // value of a parameter
+ const llvm::StringLiteral Source = R"cc(
+ RootFlags( ROOT_FLAG_TYPO )
+ )cc";
+
+ auto Ctx = createMinimalASTContext();
+ StringLiteral *Signature = wrapSource(Ctx, Source);
+
+ TrivialModuleLoader ModLoader;
+ auto PP = createPP(Source, ModLoader);
+
+ SmallVector<RootSignatureElement> Elements;
+ hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements,
+ Signature, *PP);
+
+ // Test correct diagnostic produced
+ Consumer->setExpected(diag::err_hlsl_invalid_token);
+ ASSERT_TRUE(Parser.parse());
+
+ ASSERT_TRUE(Consumer->isSatisfied());
+}
+
+TEST_F(ParseHLSLRootSignatureTest, InvalidRootDescriptorFlagsValueTest) {
+ // This test will check that an error is produced when there is a invalid
+ // value of a parameter
+ const llvm::StringLiteral Source = R"cc(
+ CBV( flags = DATA_STATIC | ROOT_DESRIPTOR_FLAG_TYPO )
+ )cc";
+
+ auto Ctx = createMinimalASTContext();
+ StringLiteral *Signature = wrapSource(Ctx, Source);
+
+ TrivialModuleLoader ModLoader;
+ auto PP = createPP(Source, ModLoader);
+
+ SmallVector<RootSignatureElement> Elements;
+ hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements,
+ Signature, *PP);
+
+ // Test correct diagnostic produced
+ Consumer->setExpected(diag::err_hlsl_invalid_token);
+ ASSERT_TRUE(Parser.parse());
+
+ ASSERT_TRUE(Consumer->isSatisfied());
+}
+
+TEST_F(ParseHLSLRootSignatureTest, InvalidDescriptorRangeFlagsValueTest) {
+ // This test will check that an error is produced when there is a invalid
+ // value of a parameter
+ const llvm::StringLiteral Source = R"cc(
+ DescriptorTable(
+ CBV(
+ flags = DATA_STATIC | DESRIPTOR_RANGE_FLAG_TYPO | DESCRIPTORS_VOLATILE
+ )
+ )
+ )cc";
+
+ auto Ctx = createMinimalASTContext();
+ StringLiteral *Signature = wrapSource(Ctx, Source);
+
+ TrivialModuleLoader ModLoader;
+ auto PP = createPP(Source, ModLoader);
+
+ SmallVector<RootSignatureElement> Elements;
+ hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements,
+ Signature, *PP);
+
+ // Test correct diagnostic produced
+ Consumer->setExpected(diag::err_hlsl_invalid_token);
ASSERT_TRUE(Parser.parse());
ASSERT_TRUE(Consumer->isSatisfied());
More information about the cfe-commits
mailing list