Skip to content

Commit f03bcb7

Browse files
authored
[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](#147350 (comment)) and [here](#145827 (comment)). 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: #147799
1 parent 7ecb37b commit f03bcb7

File tree

5 files changed

+558
-77
lines changed

5 files changed

+558
-77
lines changed

clang/include/clang/Basic/DiagnosticParseKinds.td

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1863,8 +1863,7 @@ def err_hlsl_virtual_inheritance
18631863
: Error<"virtual inheritance is unsupported in HLSL">;
18641864

18651865
// HLSL Root Signature Parser Diagnostics
1866-
def err_hlsl_unexpected_end_of_params
1867-
: Error<"expected %0 to denote end of parameters, or, another valid parameter of %1">;
1866+
def err_hlsl_invalid_token : Error<"invalid %select{parameter|value}0 of %1">;
18681867
def err_hlsl_rootsig_repeat_param : Error<"specified the same parameter '%0' multiple times">;
18691868
def err_hlsl_rootsig_missing_param : Error<"did not specify mandatory parameter '%0'">;
18701869
def err_hlsl_number_literal_overflow : Error<

clang/include/clang/Parse/ParseHLSLRootSignature.h

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,8 @@ class RootSignatureParser {
100100
std::optional<llvm::dxbc::RootDescriptorFlags> Flags;
101101
};
102102
std::optional<ParsedRootDescriptorParams>
103-
parseRootDescriptorParams(RootSignatureToken::Kind RegType);
103+
parseRootDescriptorParams(RootSignatureToken::Kind DescKind,
104+
RootSignatureToken::Kind RegType);
104105

105106
struct ParsedClauseParams {
106107
std::optional<llvm::hlsl::rootsig::Register> Reg;
@@ -110,7 +111,8 @@ class RootSignatureParser {
110111
std::optional<llvm::dxbc::DescriptorRangeFlags> Flags;
111112
};
112113
std::optional<ParsedClauseParams>
113-
parseDescriptorTableClauseParams(RootSignatureToken::Kind RegType);
114+
parseDescriptorTableClauseParams(RootSignatureToken::Kind ClauseKind,
115+
RootSignatureToken::Kind RegType);
114116

115117
struct ParsedStaticSamplerParams {
116118
std::optional<llvm::hlsl::rootsig::Register> Reg;
@@ -135,13 +137,20 @@ class RootSignatureParser {
135137
std::optional<float> parseFloatParam();
136138

137139
/// Parsing methods of various enums
138-
std::optional<llvm::dxbc::ShaderVisibility> parseShaderVisibility();
139-
std::optional<llvm::dxbc::SamplerFilter> parseSamplerFilter();
140-
std::optional<llvm::dxbc::TextureAddressMode> parseTextureAddressMode();
141-
std::optional<llvm::dxbc::ComparisonFunc> parseComparisonFunc();
142-
std::optional<llvm::dxbc::StaticBorderColor> parseStaticBorderColor();
143-
std::optional<llvm::dxbc::RootDescriptorFlags> parseRootDescriptorFlags();
144-
std::optional<llvm::dxbc::DescriptorRangeFlags> parseDescriptorRangeFlags();
140+
std::optional<llvm::dxbc::ShaderVisibility>
141+
parseShaderVisibility(RootSignatureToken::Kind Context);
142+
std::optional<llvm::dxbc::SamplerFilter>
143+
parseSamplerFilter(RootSignatureToken::Kind Context);
144+
std::optional<llvm::dxbc::TextureAddressMode>
145+
parseTextureAddressMode(RootSignatureToken::Kind Context);
146+
std::optional<llvm::dxbc::ComparisonFunc>
147+
parseComparisonFunc(RootSignatureToken::Kind Context);
148+
std::optional<llvm::dxbc::StaticBorderColor>
149+
parseStaticBorderColor(RootSignatureToken::Kind Context);
150+
std::optional<llvm::dxbc::RootDescriptorFlags>
151+
parseRootDescriptorFlags(RootSignatureToken::Kind Context);
152+
std::optional<llvm::dxbc::DescriptorRangeFlags>
153+
parseDescriptorRangeFlags(RootSignatureToken::Kind Context);
145154

146155
/// Use NumericLiteralParser to convert CurToken.NumSpelling into a unsigned
147156
/// 32-bit integer

0 commit comments

Comments
 (0)