https://github.com/inbelic updated
https://github.com/llvm/llvm-project/pull/147832
>From 02e7ad8a92e01b19d85f9bedf831aac161439ccb Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienf...@gmail.com>
Date: Wed, 9 Jul 2025 21:21:53 +0000
Subject: [PATCH 1/3] [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 870cb88f40963..416709defacd1 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(ElementLoc, *Flags);
+ if (Flags.has_value())
+ Elements.emplace_back(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(ElementLoc, *Constants);
+ if (Constants.has_value())
+ Elements.emplace_back(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(ElementLoc, *Table);
+ if (Table.has_value())
+ Elements.emplace_back(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(ElementLoc, *Descriptor);
+ if (Descriptor.has_value())
+ Elements.emplace_back(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(ElementLoc, *Sampler);
+ if (Sampler.has_value())
+ Elements.emplace_back(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(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 4b53a127d2adb..7f06ff0bef689 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@+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@+7 {{expected integer literal after '='}}
+// expected-error@+6 {{did not specify mandatory parameter 's register'}}
+// expected-error@+5 {{specified the same parameter 'visibility' multiple
times}}
+// expected-error@+4 {{integer literal is too large to be represented as a
32-bit signed integer type}}
+// expected-error@+3 {{flag value is neither a literal 0 nor a named value}}
+// expected-error@+2 {{expected ')' or ','}}
+// expected-error@+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@+4 {{invalid parameter of CBV}}
+// expected-error@+3 {{invalid parameter of UAV}}
+// expected-error@+2 {{invalid parameter of StaticSampler}}
+// expected-error@+1 {{invalid parameter of SRV}}
+[RootSignature(DemoGranularityRootSignature)]
+void granularity_errors() {}
>From f589f36804d09fe85ab9ee3785040ba408bc1864 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienf...@gmail.com>
Date: Fri, 11 Jul 2025 23:31:10 +0000
Subject: [PATCH 2/3] review: correctly exit the table scope before reporting
more errors
---
.../clang/Parse/ParseHLSLRootSignature.h | 7 +++++++
clang/lib/Parse/ParseHLSLRootSignature.cpp | 20 ++++++++++++++++++-
clang/test/SemaHLSL/RootSignature-err.hlsl | 13 ++++++++++++
3 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h
b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index 9f46158b963f0..ad66a26798847 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -206,6 +206,13 @@ class RootSignatureParser {
bool skipUntilExpectedToken(RootSignatureToken::Kind Expected);
bool skipUntilExpectedToken(ArrayRef<RootSignatureToken::Kind> Expected);
+ /// Consume tokens until we reach a closing right paren, ')', or, until we
+ /// have reached the end of the stream. This will place the current token
+ /// to be the end of stream or the right paren.
+ ///
+ /// Returns true if it is closed before the end of stream.
+ bool skipUntilClosedParens(uint32_t NumParens = 1);
+
/// 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 416709defacd1..3cbe5f7fd2c21 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -62,7 +62,7 @@ bool RootSignatureParser::parse() {
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);
+ skipUntilClosedParens();
consumeNextToken();
}
} else if (tryConsumeExpectedToken(
@@ -1418,6 +1418,24 @@ bool RootSignatureParser::skipUntilExpectedToken(
return true;
}
+bool RootSignatureParser::skipUntilClosedParens(uint32_t NumParens) {
+ TokenKind ParenKinds[] = {
+ TokenKind::pu_l_paren,
+ TokenKind::pu_r_paren,
+ };
+ while (skipUntilExpectedToken(ParenKinds)) {
+ consumeNextToken();
+ if (CurToken.TokKind == TokenKind::pu_r_paren)
+ NumParens--;
+ else
+ NumParens++;
+ if (NumParens == 0)
+ return true;
+ }
+
+ return false;
+}
+
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 7f06ff0bef689..0e5e84fcddd1f 100644
--- a/clang/test/SemaHLSL/RootSignature-err.hlsl
+++ b/clang/test/SemaHLSL/RootSignature-err.hlsl
@@ -141,3 +141,16 @@ void multiple_errors() {}
// expected-error@+1 {{invalid parameter of SRV}}
[RootSignature(DemoGranularityRootSignature)]
void granularity_errors() {}
+
+#define TestTableScope \
+ "DescriptorTable( " \
+ " UAV(u0, reported_diag), " \
+ " SRV(t0, skipped_diag), " \
+ " Sampler(s0, skipped_diag), " \
+ ")," \
+ "CBV(s0, reported_diag)"
+
+// expected-error@+2 {{invalid parameter of UAV}}
+// expected-error@+1 {{invalid parameter of CBV}}
+[RootSignature(TestTableScope)]
+void recover_scope_errors() {}
>From 39e450b04fdb525f980e58ddb027b4db15ee85f8 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienf...@gmail.com>
Date: Fri, 11 Jul 2025 23:43:41 +0000
Subject: [PATCH 3/3] nfc, review: update loop logic to remove use of local
bool
---
clang/lib/Parse/ParseHLSLRootSignature.cpp | 60 ++++++++++++----------
1 file changed, 34 insertions(+), 26 deletions(-)
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp
b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 3cbe5f7fd2c21..db9ed8373d01d 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -38,59 +38,67 @@ bool RootSignatureParser::parse() {
// 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())
- Elements.emplace_back(ElementLoc, *Flags);
- else
- HadLocalError = true;
+ if (!Flags.has_value()) {
+ HadError = true;
+ skipUntilExpectedToken(RootElementKeywords);
+ continue;
+ }
+
+ Elements.emplace_back(ElementLoc, *Flags);
} else if (tryConsumeExpectedToken(TokenKind::kw_RootConstants)) {
SourceLocation ElementLoc = getTokenLocation(CurToken);
auto Constants = parseRootConstants();
- if (Constants.has_value())
- Elements.emplace_back(ElementLoc, *Constants);
- else
- HadLocalError = true;
+ if (!Constants.has_value()) {
+ HadError = true;
+ skipUntilExpectedToken(RootElementKeywords);
+ continue;
+ }
+ Elements.emplace_back(ElementLoc, *Constants);
} else if (tryConsumeExpectedToken(TokenKind::kw_DescriptorTable)) {
SourceLocation ElementLoc = getTokenLocation(CurToken);
auto Table = parseDescriptorTable();
- if (Table.has_value())
- Elements.emplace_back(ElementLoc, *Table);
- else {
- HadLocalError = true;
+ if (!Table.has_value()) {
+ HadError = true;
// We are within a DescriptorTable, we will do our best to recover
// by skipping until we encounter the expected closing ')'.
skipUntilClosedParens();
consumeNextToken();
+ skipUntilExpectedToken(RootElementKeywords);
+ continue;
}
+ Elements.emplace_back(ElementLoc, *Table);
} else if (tryConsumeExpectedToken(
{TokenKind::kw_CBV, TokenKind::kw_SRV, TokenKind::kw_UAV}))
{
SourceLocation ElementLoc = getTokenLocation(CurToken);
auto Descriptor = parseRootDescriptor();
- if (Descriptor.has_value())
- Elements.emplace_back(ElementLoc, *Descriptor);
- else
- HadLocalError = true;
+ if (!Descriptor.has_value()) {
+ HadError = true;
+ skipUntilExpectedToken(RootElementKeywords);
+ continue;
+ }
+ Elements.emplace_back(ElementLoc, *Descriptor);
} else if (tryConsumeExpectedToken(TokenKind::kw_StaticSampler)) {
SourceLocation ElementLoc = getTokenLocation(CurToken);
auto Sampler = parseStaticSampler();
- if (Sampler.has_value())
- Elements.emplace_back(ElementLoc, *Sampler);
- else
- HadLocalError = true;
+ if (!Sampler.has_value()) {
+ HadError = true;
+ skipUntilExpectedToken(RootElementKeywords);
+ continue;
+ }
+ Elements.emplace_back(ElementLoc, *Sampler);
} else {
- HadLocalError = true;
+ HadError = true;
consumeNextToken(); // let diagnostic be at the start of invalid token
reportDiag(diag::err_hlsl_invalid_token)
<< /*parameter=*/0 << /*param of*/ TokenKind::kw_RootSignature;
+ skipUntilExpectedToken(RootElementKeywords);
+ continue;
}
- if (HadLocalError) {
- HadError = true;
- skipUntilExpectedToken(RootElementKeywords);
- } else if (!tryConsumeExpectedToken(TokenKind::pu_comma)) {
+ if (!tryConsumeExpectedToken(TokenKind::pu_comma)) {
// ',' denotes another element, otherwise, expected to be at end of
stream
break;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
