[llvm-branch-commits] [clang] [HLSL][RootSignature] Allow for multiple parsing errors in `RootSignatureParser` (PR #147832)
Finn Plummer via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Wed Jul 9 15:04:06 PDT 2025
https://github.com/inbelic updated https://github.com/llvm/llvm-project/pull/147832
>From 7ec7e32d2ac4945a489d5463b9fb700b0cceff9d Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Wed, 9 Jul 2025 21:21:53 +0000
Subject: [PATCH] [HLSL][RootSignature] Implement multiple diagnostics in
`RootSignatureParser`
---
.../clang/Parse/ParseHLSLRootSignature.h | 8 ++
clang/lib/Parse/ParseHLSLRootSignature.cpp | 87 ++++++++++++++-----
clang/test/SemaHLSL/RootSignature-err.hlsl | 38 ++++++++
3 files changed, 113 insertions(+), 20 deletions(-)
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index 404bccea10c99..9f46158b963f0 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -198,6 +198,14 @@ class RootSignatureParser {
bool tryConsumeExpectedToken(RootSignatureToken::Kind Expected);
bool tryConsumeExpectedToken(ArrayRef<RootSignatureToken::Kind> Expected);
+ /// Consume tokens until the expected token has been peeked to be next
+ /// or we have reached the end of the stream. Note that this means the
+ /// expected token will be the next token not CurToken.
+ ///
+ /// Returns true if it found a token of the given type.
+ bool skipUntilExpectedToken(RootSignatureToken::Kind Expected);
+ bool skipUntilExpectedToken(ArrayRef<RootSignatureToken::Kind> Expected);
+
/// Convert the token's offset in the signature string to its SourceLocation
///
/// This allows to currently retrieve the location for multi-token
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 87de627a98fb7..ceeb4c9fa8c0f 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -17,6 +17,15 @@ namespace hlsl {
using TokenKind = RootSignatureToken::Kind;
+static const TokenKind RootElementKeywords[] = {
+ TokenKind::kw_RootFlags,
+ TokenKind::kw_CBV,
+ TokenKind::kw_UAV,
+ TokenKind::kw_SRV,
+ TokenKind::kw_DescriptorTable,
+ TokenKind::kw_StaticSampler,
+};
+
RootSignatureParser::RootSignatureParser(
llvm::dxbc::RootSignatureVersion Version,
SmallVector<RootSignatureElement> &Elements, StringLiteral *Signature,
@@ -27,51 +36,68 @@ RootSignatureParser::RootSignatureParser(
bool RootSignatureParser::parse() {
// Iterate as many RootSignatureElements as possible, until we hit the
// end of the stream
+ bool HadError = false;
while (!peekExpectedToken(TokenKind::end_of_stream)) {
+ bool HadLocalError = false;
if (tryConsumeExpectedToken(TokenKind::kw_RootFlags)) {
SourceLocation ElementLoc = getTokenLocation(CurToken);
auto Flags = parseRootFlags();
- if (!Flags.has_value())
- return true;
- Elements.emplace_back(RootSignatureElement(ElementLoc, *Flags));
+ if (Flags.has_value())
+ Elements.emplace_back(RootSignatureElement(ElementLoc, *Flags));
+ else
+ HadLocalError = true;
} else if (tryConsumeExpectedToken(TokenKind::kw_RootConstants)) {
SourceLocation ElementLoc = getTokenLocation(CurToken);
auto Constants = parseRootConstants();
- if (!Constants.has_value())
- return true;
- Elements.emplace_back(RootSignatureElement(ElementLoc, *Constants));
+ if (Constants.has_value())
+ Elements.emplace_back(RootSignatureElement(ElementLoc, *Constants));
+ else
+ HadLocalError = true;
} else if (tryConsumeExpectedToken(TokenKind::kw_DescriptorTable)) {
SourceLocation ElementLoc = getTokenLocation(CurToken);
auto Table = parseDescriptorTable();
- if (!Table.has_value())
- return true;
- Elements.emplace_back(RootSignatureElement(ElementLoc, *Table));
+ if (Table.has_value())
+ Elements.emplace_back(RootSignatureElement(ElementLoc, *Table));
+ else {
+ HadLocalError = true;
+ // We are within a DescriptorTable, we will do our best to recover
+ // by skipping until we encounter the expected closing ')'.
+ skipUntilExpectedToken(TokenKind::pu_r_paren);
+ consumeNextToken();
+ }
} else if (tryConsumeExpectedToken(
{TokenKind::kw_CBV, TokenKind::kw_SRV, TokenKind::kw_UAV})) {
SourceLocation ElementLoc = getTokenLocation(CurToken);
auto Descriptor = parseRootDescriptor();
- if (!Descriptor.has_value())
- return true;
- Elements.emplace_back(RootSignatureElement(ElementLoc, *Descriptor));
+ if (Descriptor.has_value())
+ Elements.emplace_back(RootSignatureElement(ElementLoc, *Descriptor));
+ else
+ HadLocalError = true;
} else if (tryConsumeExpectedToken(TokenKind::kw_StaticSampler)) {
SourceLocation ElementLoc = getTokenLocation(CurToken);
auto Sampler = parseStaticSampler();
- if (!Sampler.has_value())
- return true;
- Elements.emplace_back(RootSignatureElement(ElementLoc, *Sampler));
+ if (Sampler.has_value())
+ Elements.emplace_back(RootSignatureElement(ElementLoc, *Sampler));
+ else
+ HadLocalError = true;
} else {
+ HadLocalError = true;
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
- if (!tryConsumeExpectedToken(TokenKind::pu_comma))
+ if (HadLocalError) {
+ HadError = true;
+ skipUntilExpectedToken(RootElementKeywords);
+ } else if (!tryConsumeExpectedToken(TokenKind::pu_comma)) {
+ // ',' denotes another element, otherwise, expected to be at end of stream
break;
+ }
}
- return consumeExpectedToken(TokenKind::end_of_stream,
+ return HadError ||
+ consumeExpectedToken(TokenKind::end_of_stream,
diag::err_expected_either, TokenKind::pu_comma);
}
@@ -262,8 +288,13 @@ std::optional<DescriptorTable> RootSignatureParser::parseDescriptorTable() {
// DescriptorTableClause - CBV, SRV, UAV, or Sampler
SourceLocation ElementLoc = getTokenLocation(CurToken);
auto Clause = parseDescriptorTableClause();
- if (!Clause.has_value())
+ if (!Clause.has_value()) {
+ // We are within a DescriptorTableClause, we will do our best to recover
+ // by skipping until we encounter the expected closing ')'
+ skipUntilExpectedToken(TokenKind::pu_r_paren);
+ consumeNextToken();
return std::nullopt;
+ }
Elements.emplace_back(RootSignatureElement(ElementLoc, *Clause));
Table.NumClauses++;
} else if (tryConsumeExpectedToken(TokenKind::kw_visibility)) {
@@ -1371,6 +1402,22 @@ bool RootSignatureParser::tryConsumeExpectedToken(
return true;
}
+bool RootSignatureParser::skipUntilExpectedToken(TokenKind Expected) {
+ return skipUntilExpectedToken(ArrayRef{Expected});
+}
+
+bool RootSignatureParser::skipUntilExpectedToken(
+ ArrayRef<TokenKind> AnyExpected) {
+
+ while (!peekExpectedToken(AnyExpected)) {
+ if (peekExpectedToken(TokenKind::end_of_stream))
+ return false;
+ consumeNextToken();
+ }
+
+ return true;
+}
+
SourceLocation RootSignatureParser::getTokenLocation(RootSignatureToken Tok) {
return Signature->getLocationOfByte(Tok.LocOffset, PP.getSourceManager(),
PP.getLangOpts(), PP.getTargetInfo());
diff --git a/clang/test/SemaHLSL/RootSignature-err.hlsl b/clang/test/SemaHLSL/RootSignature-err.hlsl
index b69807132e795..bc6e956f4166b 100644
--- a/clang/test/SemaHLSL/RootSignature-err.hlsl
+++ b/clang/test/SemaHLSL/RootSignature-err.hlsl
@@ -103,3 +103,41 @@ 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() {}
+
+#define DemoMultipleErrorsRootSignature \
+ "CBV(b0, space = invalid)," \
+ "StaticSampler()" \
+ "DescriptorTable(" \
+ " visibility = SHADER_VISIBILITY_ALL," \
+ " visibility = SHADER_VISIBILITY_DOMAIN," \
+ ")," \
+ "SRV(t0, space = 28947298374912374098172)" \
+ "UAV(u0, flags = 3)" \
+ "DescriptorTable(Sampler(s0 flags = DATA_VOLATILE))," \
+ "CBV(b0),,"
+
+// expected-error at +7 {{expected integer literal after '='}}
+// expected-error at +6 {{did not specify mandatory parameter 's register'}}
+// expected-error at +5 {{specified the same parameter 'visibility' multiple times}}
+// expected-error at +4 {{integer literal is too large to be represented as a 32-bit signed integer type}}
+// expected-error at +3 {{flag value is neither a literal 0 nor a named value}}
+// expected-error at +2 {{expected ')' or ','}}
+// expected-error at +1 {{invalid parameter of RootSignature}}
+[RootSignature(DemoMultipleErrorsRootSignature)]
+void multiple_errors() {}
+
+#define DemoGranularityRootSignature \
+ "CBV(b0, reported_diag, flags = skipped_diag)," \
+ "DescriptorTable( " \
+ " UAV(u0, reported_diag), " \
+ " SRV(t0, skipped_diag), " \
+ ")," \
+ "StaticSampler(s0, reported_diag, SRV(t0, reported_diag)" \
+ ""
+
+// expected-error at +4 {{invalid parameter of CBV}}
+// expected-error at +3 {{invalid parameter of UAV}}
+// expected-error at +2 {{invalid parameter of StaticSampler}}
+// expected-error at +1 {{invalid parameter of SRV}}
+[RootSignature(DemoGranularityRootSignature)]
+void granularity_errors() {}
More information about the llvm-branch-commits
mailing list