[clang] [llvm] [HLSL][RootSignature] Implement parsing of a DescriptorTable with empty clauses (PR #133302)
Finn Plummer via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 28 13:31:45 PDT 2025
https://github.com/inbelic updated https://github.com/llvm/llvm-project/pull/133302
>From 3094705b94fab19ecc75cb501cceae506bb0128e Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Fri, 14 Feb 2025 19:44:36 +0000
Subject: [PATCH 01/14] [NFC][HLSL][RootSignature] add spelling of tokens to
tokenkind.def
---
.../clang/Lex/HLSLRootSignatureTokenKinds.def | 22 +++++++++----------
.../include/clang/Lex/LexHLSLRootSignature.h | 2 +-
.../Lex/LexHLSLRootSignatureTest.cpp | 2 +-
3 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/clang/include/clang/Lex/HLSLRootSignatureTokenKinds.def b/clang/include/clang/Lex/HLSLRootSignatureTokenKinds.def
index e6df763920430..697c6abc54efa 100644
--- a/clang/include/clang/Lex/HLSLRootSignatureTokenKinds.def
+++ b/clang/include/clang/Lex/HLSLRootSignatureTokenKinds.def
@@ -14,16 +14,16 @@
//===----------------------------------------------------------------------===//
#ifndef TOK
-#define TOK(X)
+#define TOK(X, SPELLING)
#endif
#ifndef PUNCTUATOR
-#define PUNCTUATOR(X,Y) TOK(pu_ ## X)
+#define PUNCTUATOR(X,Y) TOK(pu_ ## X, Y)
#endif
#ifndef KEYWORD
-#define KEYWORD(X) TOK(kw_ ## X)
+#define KEYWORD(X) TOK(kw_ ## X, #X)
#endif
#ifndef ENUM
-#define ENUM(NAME, LIT) TOK(en_ ## NAME)
+#define ENUM(NAME, LIT) TOK(en_ ## NAME, LIT)
#endif
// Defines the various types of enum
@@ -49,15 +49,15 @@
#endif
// General Tokens:
-TOK(invalid)
-TOK(end_of_stream)
-TOK(int_literal)
+TOK(invalid, "invalid identifier")
+TOK(end_of_stream, "end of stream")
+TOK(int_literal, "integer literal")
// Register Tokens:
-TOK(bReg)
-TOK(tReg)
-TOK(uReg)
-TOK(sReg)
+TOK(bReg, "b register")
+TOK(tReg, "t register")
+TOK(uReg, "u register")
+TOK(sReg, "s register")
// Punctuators:
PUNCTUATOR(l_paren, '(')
diff --git a/clang/include/clang/Lex/LexHLSLRootSignature.h b/clang/include/clang/Lex/LexHLSLRootSignature.h
index 21c44e0351d9e..b82237411b2ab 100644
--- a/clang/include/clang/Lex/LexHLSLRootSignature.h
+++ b/clang/include/clang/Lex/LexHLSLRootSignature.h
@@ -24,7 +24,7 @@ namespace hlsl {
struct RootSignatureToken {
enum Kind {
-#define TOK(X) X,
+#define TOK(X, SPELLING) X,
#include "clang/Lex/HLSLRootSignatureTokenKinds.def"
};
diff --git a/clang/unittests/Lex/LexHLSLRootSignatureTest.cpp b/clang/unittests/Lex/LexHLSLRootSignatureTest.cpp
index 0576f08c4c276..1e014f6c41c57 100644
--- a/clang/unittests/Lex/LexHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Lex/LexHLSLRootSignatureTest.cpp
@@ -113,7 +113,7 @@ TEST_F(LexHLSLRootSignatureTest, ValidLexAllTokensTest) {
SmallVector<hlsl::RootSignatureToken> Tokens;
SmallVector<hlsl::TokenKind> Expected = {
-#define TOK(NAME) hlsl::TokenKind::NAME,
+#define TOK(NAME, SPELLING) hlsl::TokenKind::NAME,
#include "clang/Lex/HLSLRootSignatureTokenKinds.def"
};
>From d744cd13c24726a9bd050c55c28d0cd03e5244b4 Mon Sep 17 00:00:00 2001
From: Finn Plummer <finnplummer at microsoft.com>
Date: Wed, 26 Mar 2025 19:10:15 +0000
Subject: [PATCH 02/14] [HLSL][RootSignature] Implement parsing of an empty
DescriptorTable
- defines the Parser class and an initial set of helper methods to
support consuming tokens. functionality is demonstrated through a simple
empty descriptor table test case
- defines an initial in-memory representation of a DescriptorTable
- implements a test harness that will be used to validate the correct
diagnostics are generated. it will construct a dummy pre-processor with
diagnostics consumer to do so
---
.../clang/Parse/ParseHLSLRootSignature.h | 84 +++++++
clang/lib/Parse/CMakeLists.txt | 1 +
clang/lib/Parse/ParseHLSLRootSignature.cpp | 135 +++++++++++
clang/unittests/CMakeLists.txt | 1 +
clang/unittests/Parse/CMakeLists.txt | 23 ++
.../Parse/ParseHLSLRootSignatureTest.cpp | 217 ++++++++++++++++++
.../llvm/Frontend/HLSL/HLSLRootSignature.h | 37 +++
7 files changed, 498 insertions(+)
create mode 100644 clang/include/clang/Parse/ParseHLSLRootSignature.h
create mode 100644 clang/lib/Parse/ParseHLSLRootSignature.cpp
create mode 100644 clang/unittests/Parse/CMakeLists.txt
create mode 100644 clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
create mode 100644 llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
new file mode 100644
index 0000000000000..95243747fbd6b
--- /dev/null
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -0,0 +1,84 @@
+//===--- ParseHLSLRootSignature.h -------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines the ParseHLSLRootSignature interface.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_PARSE_PARSEHLSLROOTSIGNATURE_H
+#define LLVM_CLANG_PARSE_PARSEHLSLROOTSIGNATURE_H
+
+#include "clang/Basic/DiagnosticParse.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Lex/LexHLSLRootSignature.h"
+
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+
+#include "llvm/Frontend/HLSL/HLSLRootSignature.h"
+
+namespace clang {
+namespace hlsl {
+
+class RootSignatureParser {
+public:
+ RootSignatureParser(SmallVector<llvm::hlsl::rootsig::RootElement> &Elements,
+ RootSignatureLexer &Lexer, clang::Preprocessor &PP);
+
+ /// Consumes tokens from the Lexer and constructs the in-memory
+ /// representations of the RootElements. Tokens are consumed until an
+ /// error is encountered or the end of the buffer.
+ ///
+ /// Returns true if a parsing error is encountered.
+ bool Parse();
+
+private:
+ DiagnosticsEngine &Diags() { return PP.getDiagnostics(); }
+
+ /// Root Element parse methods:
+ bool ParseDescriptorTable();
+
+ /// Invoke the Lexer to consume a token and update CurToken with the result
+ void ConsumeNextToken() { CurToken = Lexer.ConsumeToken(); }
+
+ /// Return true if the next token one of the expected kinds
+ bool PeekExpectedToken(TokenKind Expected);
+ bool PeekExpectedToken(ArrayRef<TokenKind> AnyExpected);
+
+ /// Consumes the next token and report an error if it is not of the expected
+ /// kind.
+ ///
+ /// Returns true if there was an error reported.
+ bool ConsumeExpectedToken(TokenKind Expected,
+ unsigned DiagID = diag::err_expected,
+ TokenKind Context = TokenKind::invalid);
+ bool ConsumeExpectedToken(ArrayRef<TokenKind> AnyExpected,
+ unsigned DiagID = diag::err_expected,
+ TokenKind Context = TokenKind::invalid);
+
+ /// Peek if the next token is of the expected kind and if it is then consume
+ /// it.
+ ///
+ /// Returns true if it successfully matches the expected kind and the token
+ /// was consumed.
+ bool TryConsumeExpectedToken(TokenKind Expected);
+ bool TryConsumeExpectedToken(ArrayRef<TokenKind> Expected);
+
+private:
+ SmallVector<llvm::hlsl::rootsig::RootElement> &Elements;
+ RootSignatureLexer &Lexer;
+
+ clang::Preprocessor &PP;
+
+ RootSignatureToken CurToken;
+};
+
+} // namespace hlsl
+} // namespace clang
+
+#endif // LLVM_CLANG_PARSE_PARSEHLSLROOTSIGNATURE_H
diff --git a/clang/lib/Parse/CMakeLists.txt b/clang/lib/Parse/CMakeLists.txt
index 22e902f7e1bc5..00fde537bb9c6 100644
--- a/clang/lib/Parse/CMakeLists.txt
+++ b/clang/lib/Parse/CMakeLists.txt
@@ -14,6 +14,7 @@ add_clang_library(clangParse
ParseExpr.cpp
ParseExprCXX.cpp
ParseHLSL.cpp
+ ParseHLSLRootSignature.cpp
ParseInit.cpp
ParseObjc.cpp
ParseOpenMP.cpp
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
new file mode 100644
index 0000000000000..86d587b10a8cb
--- /dev/null
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -0,0 +1,135 @@
+#include "clang/Parse/ParseHLSLRootSignature.h"
+
+#include "llvm/Support/raw_ostream.h"
+
+using namespace llvm::hlsl::rootsig;
+
+namespace clang {
+namespace hlsl {
+
+static std::string FormatTokenKinds(ArrayRef<TokenKind> Kinds) {
+ std::string TokenString;
+ llvm::raw_string_ostream Out(TokenString);
+ bool First = true;
+ for (auto Kind : Kinds) {
+ if (!First)
+ Out << ", ";
+ switch (Kind) {
+#define TOK(X, SPELLING) \
+ case TokenKind::X: \
+ Out << SPELLING; \
+ break;
+#include "clang/Lex/HLSLRootSignatureTokenKinds.def"
+ }
+ First = false;
+ }
+
+ return TokenString;
+}
+
+// Parser Definitions
+
+RootSignatureParser::RootSignatureParser(SmallVector<RootElement> &Elements,
+ RootSignatureLexer &Lexer,
+ Preprocessor &PP)
+ : Elements(Elements), Lexer(Lexer), PP(PP), CurToken(SourceLocation()) {}
+
+bool RootSignatureParser::Parse() {
+ // Iterate as many RootElements as possible
+ while (TryConsumeExpectedToken(TokenKind::kw_DescriptorTable)) {
+ bool Error = false;
+ // Dispatch onto parser method.
+ // We guard against the unreachable here as we just ensured that CurToken
+ // will be one of the kinds in the while condition
+ switch (CurToken.Kind) {
+ case TokenKind::kw_DescriptorTable:
+ Error = ParseDescriptorTable();
+ break;
+ default:
+ llvm_unreachable("Switch for consumed token was not provided");
+ }
+
+ if (Error) return true;
+
+ if (!TryConsumeExpectedToken(TokenKind::pu_comma))
+ break;
+ }
+
+ return ConsumeExpectedToken(TokenKind::end_of_stream, diag::err_expected);
+}
+
+bool RootSignatureParser::ParseDescriptorTable() {
+ DescriptorTable Table;
+
+ if (ConsumeExpectedToken(TokenKind::pu_l_paren, diag::err_expected_after,
+ CurToken.Kind))
+ return true;
+
+ if (ConsumeExpectedToken(TokenKind::pu_r_paren, diag::err_expected_after,
+ CurToken.Kind))
+ return true;
+
+ Elements.push_back(Table);
+ return false;
+}
+
+// Returns true when given token is one of the expected kinds
+static bool IsExpectedToken(TokenKind Kind, ArrayRef<TokenKind> AnyExpected) {
+ for (auto Expected : AnyExpected)
+ if (Kind == Expected)
+ return true;
+ return false;
+}
+
+bool RootSignatureParser::PeekExpectedToken(TokenKind Expected) {
+ return PeekExpectedToken(ArrayRef{Expected});
+}
+
+bool RootSignatureParser::PeekExpectedToken(ArrayRef<TokenKind> AnyExpected) {
+ RootSignatureToken Result = Lexer.PeekNextToken();
+ return IsExpectedToken(Result.Kind, AnyExpected);
+}
+
+bool RootSignatureParser::ConsumeExpectedToken(TokenKind Expected,
+ unsigned DiagID,
+ TokenKind Context) {
+ return ConsumeExpectedToken(ArrayRef{Expected}, DiagID, Context);
+}
+
+bool RootSignatureParser::ConsumeExpectedToken(ArrayRef<TokenKind> AnyExpected,
+ unsigned DiagID,
+ TokenKind Context) {
+ if (TryConsumeExpectedToken(AnyExpected))
+ return false;
+
+ // Report unexpected token kind error
+ DiagnosticBuilder DB = Diags().Report(CurToken.TokLoc, DiagID);
+ switch (DiagID) {
+ case diag::err_expected:
+ DB << FormatTokenKinds(AnyExpected);
+ break;
+ case diag::err_expected_either:
+ case diag::err_expected_after:
+ DB << FormatTokenKinds(AnyExpected) << FormatTokenKinds({Context});
+ break;
+ default:
+ break;
+ }
+ return true;
+}
+
+bool RootSignatureParser::TryConsumeExpectedToken(TokenKind Expected) {
+ return TryConsumeExpectedToken(ArrayRef{Expected});
+}
+
+bool RootSignatureParser::TryConsumeExpectedToken(
+ ArrayRef<TokenKind> AnyExpected) {
+ // If not the expected token just return
+ if (!PeekExpectedToken(AnyExpected))
+ return false;
+ ConsumeNextToken();
+ return true;
+}
+
+} // namespace hlsl
+} // namespace clang
diff --git a/clang/unittests/CMakeLists.txt b/clang/unittests/CMakeLists.txt
index 85d265426ec80..9b3ce8aa7de73 100644
--- a/clang/unittests/CMakeLists.txt
+++ b/clang/unittests/CMakeLists.txt
@@ -25,6 +25,7 @@ endfunction()
add_subdirectory(Basic)
add_subdirectory(Lex)
+add_subdirectory(Parse)
add_subdirectory(Driver)
if(CLANG_ENABLE_STATIC_ANALYZER)
add_subdirectory(Analysis)
diff --git a/clang/unittests/Parse/CMakeLists.txt b/clang/unittests/Parse/CMakeLists.txt
new file mode 100644
index 0000000000000..eeb58174568cd
--- /dev/null
+++ b/clang/unittests/Parse/CMakeLists.txt
@@ -0,0 +1,23 @@
+set(LLVM_LINK_COMPONENTS
+ Support
+ )
+add_clang_unittest(ParseTests
+ ParseHLSLRootSignatureTest.cpp
+ )
+clang_target_link_libraries(ParseTests
+ PRIVATE
+ clangAST
+ clangASTMatchers
+ clangBasic
+ clangFrontend
+ clangParse
+ clangSema
+ clangSerialization
+ clangTooling
+ )
+target_link_libraries(ParseTests
+ PRIVATE
+ LLVMTestingAnnotations
+ LLVMTestingSupport
+ clangTesting
+ )
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
new file mode 100644
index 0000000000000..35b44ebdcffda
--- /dev/null
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -0,0 +1,217 @@
+//=== ParseHLSLRootSignatureTest.cpp - Parse Root Signature tests ---------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TargetInfo.h"
+#include "clang/Lex/HeaderSearch.h"
+#include "clang/Lex/HeaderSearchOptions.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Lex/ModuleLoader.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Lex/PreprocessorOptions.h"
+
+#include "clang/Lex/LexHLSLRootSignature.h"
+#include "clang/Parse/ParseHLSLRootSignature.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace llvm::hlsl::rootsig;
+
+namespace {
+
+// Diagnostic helper for helper tests
+class ExpectedDiagConsumer : public DiagnosticConsumer {
+ virtual void anchor() {}
+
+ void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
+ const Diagnostic &Info) override {
+ if (!FirstDiag || !ExpectedDiagID.has_value()) {
+ Satisfied = false;
+ return;
+ }
+ FirstDiag = false;
+
+ Satisfied = ExpectedDiagID.value() == Info.getID();
+ }
+
+ bool FirstDiag = true;
+ bool Satisfied = false;
+ std::optional<unsigned> ExpectedDiagID;
+
+public:
+ void SetNoDiag() {
+ Satisfied = true;
+ ExpectedDiagID = std::nullopt;
+ }
+
+ void SetExpected(unsigned DiagID) {
+ Satisfied = false;
+ ExpectedDiagID = DiagID;
+ }
+
+ bool IsSatisfied() { return Satisfied; }
+};
+
+// The test fixture.
+class ParseHLSLRootSignatureTest : public ::testing::Test {
+protected:
+ ParseHLSLRootSignatureTest()
+ : FileMgr(FileMgrOpts), DiagID(new DiagnosticIDs()),
+ Consumer(new ExpectedDiagConsumer()),
+ Diags(DiagID, new DiagnosticOptions, Consumer),
+ SourceMgr(Diags, FileMgr), TargetOpts(new TargetOptions) {
+ // This is an arbitrarily chosen target triple to create the target info.
+ TargetOpts->Triple = "dxil";
+ Target = TargetInfo::CreateTargetInfo(Diags, TargetOpts);
+ }
+
+ std::unique_ptr<Preprocessor> CreatePP(StringRef Source,
+ TrivialModuleLoader &ModLoader) {
+ std::unique_ptr<llvm::MemoryBuffer> Buf =
+ llvm::MemoryBuffer::getMemBuffer(Source);
+ SourceMgr.setMainFileID(SourceMgr.createFileID(std::move(Buf)));
+
+ HeaderSearch HeaderInfo(std::make_shared<HeaderSearchOptions>(), SourceMgr,
+ Diags, LangOpts, Target.get());
+ std::unique_ptr<Preprocessor> PP = std::make_unique<Preprocessor>(
+ std::make_shared<PreprocessorOptions>(), Diags, LangOpts, SourceMgr,
+ HeaderInfo, ModLoader,
+ /*IILookup =*/nullptr,
+ /*OwnsHeaderSearch =*/false);
+ PP->Initialize(*Target);
+ PP->EnterMainSourceFile();
+ return PP;
+ }
+
+ FileSystemOptions FileMgrOpts;
+ FileManager FileMgr;
+ IntrusiveRefCntPtr<DiagnosticIDs> DiagID;
+ ExpectedDiagConsumer *Consumer;
+ DiagnosticsEngine Diags;
+ SourceManager SourceMgr;
+ LangOptions LangOpts;
+ std::shared_ptr<TargetOptions> TargetOpts;
+ IntrusiveRefCntPtr<TargetInfo> Target;
+};
+
+// Valid Parser Tests
+
+TEST_F(ParseHLSLRootSignatureTest, ValidParseEmptyTest) {
+ const llvm::StringLiteral Source = R"cc()cc";
+
+ TrivialModuleLoader ModLoader;
+ auto PP = CreatePP(Source, ModLoader);
+ auto TokLoc = SourceLocation();
+
+ hlsl::RootSignatureLexer Lexer(Source, TokLoc);
+ SmallVector<RootElement> Elements;
+ hlsl::RootSignatureParser Parser(Elements, Lexer, *PP);
+
+ // Test no diagnostics produced
+ Consumer->SetNoDiag();
+
+ ASSERT_FALSE(Parser.Parse());
+ ASSERT_EQ((int)Elements.size(), 0);
+
+ ASSERT_TRUE(Consumer->IsSatisfied());
+}
+
+TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
+ const llvm::StringLiteral Source = R"cc(
+ DescriptorTable()
+ )cc";
+
+ TrivialModuleLoader ModLoader;
+ auto PP = CreatePP(Source, ModLoader);
+ auto TokLoc = SourceLocation();
+
+ hlsl::RootSignatureLexer Lexer(Source, TokLoc);
+ SmallVector<RootElement> Elements;
+ hlsl::RootSignatureParser Parser(Elements, Lexer, *PP);
+
+ // Test no diagnostics produced
+ Consumer->SetNoDiag();
+
+ ASSERT_FALSE(Parser.Parse());
+ RootElement Elem = Elements[0];
+
+ // Empty Descriptor Table
+ ASSERT_TRUE(std::holds_alternative<DescriptorTable>(Elem));
+ ASSERT_EQ(std::get<DescriptorTable>(Elem).NumClauses, 0u);
+ ASSERT_TRUE(Consumer->IsSatisfied());
+}
+
+// Invalid Parser Tests
+
+TEST_F(ParseHLSLRootSignatureTest, InvalidParseUnexpectedTokenTest) {
+ const llvm::StringLiteral Source = R"cc(
+ DescriptorTable()
+ space
+ )cc";
+
+ TrivialModuleLoader ModLoader;
+ auto PP = CreatePP(Source, ModLoader);
+ auto TokLoc = SourceLocation();
+
+ hlsl::RootSignatureLexer Lexer(Source, TokLoc);
+ SmallVector<RootElement> Elements;
+ hlsl::RootSignatureParser Parser(Elements, Lexer, *PP);
+
+ // Test correct diagnostic produced
+ Consumer->SetExpected(diag::err_expected);
+ ASSERT_TRUE(Parser.Parse());
+
+ ASSERT_TRUE(Consumer->IsSatisfied());
+}
+
+TEST_F(ParseHLSLRootSignatureTest, InvalidParseInvalidTokenTest) {
+ const llvm::StringLiteral Source = R"cc(
+ notAnIdentifier
+ )cc";
+
+ TrivialModuleLoader ModLoader;
+ auto PP = CreatePP(Source, ModLoader);
+ auto TokLoc = SourceLocation();
+
+ hlsl::RootSignatureLexer Lexer(Source, TokLoc);
+ SmallVector<RootElement> Elements;
+ hlsl::RootSignatureParser Parser(Elements, Lexer, *PP);
+
+ // Test correct diagnostic produced - invalid token
+ Consumer->SetExpected(diag::err_expected);
+ ASSERT_TRUE(Parser.Parse());
+
+ ASSERT_TRUE(Consumer->IsSatisfied());
+}
+
+TEST_F(ParseHLSLRootSignatureTest, InvalidParseUnexpectedEndOfStreamTest) {
+ const llvm::StringLiteral Source = R"cc(
+ DescriptorTable
+ )cc";
+
+ TrivialModuleLoader ModLoader;
+ auto PP = CreatePP(Source, ModLoader);
+ auto TokLoc = SourceLocation();
+
+ hlsl::RootSignatureLexer Lexer(Source, TokLoc);
+ SmallVector<RootElement> Elements;
+ hlsl::RootSignatureParser Parser(Elements, Lexer, *PP);
+
+ // Test correct diagnostic produced - end of stream
+ Consumer->SetExpected(diag::err_expected_after);
+ ASSERT_TRUE(Parser.Parse());
+
+ ASSERT_TRUE(Consumer->IsSatisfied());
+}
+
+} // anonymous namespace
diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
new file mode 100644
index 0000000000000..9bdd41e6dec7b
--- /dev/null
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
@@ -0,0 +1,37 @@
+//===- HLSLRootSignature.h - HLSL Root Signature helper objects -----------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file This file contains helper objects for working with HLSL Root
+/// Signatures.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_FRONTEND_HLSL_HLSLROOTSIGNATURE_H
+#define LLVM_FRONTEND_HLSL_HLSLROOTSIGNATURE_H
+
+#include <variant>
+
+namespace llvm {
+namespace hlsl {
+namespace rootsig {
+
+// Definitions of the in-memory data layout structures
+
+// Models the end of a descriptor table and stores its visibility
+struct DescriptorTable {
+ uint32_t NumClauses = 0; // The number of clauses in the table
+};
+
+// Models RootElement : DescriptorTable
+using RootElement = std::variant<DescriptorTable>;
+
+} // namespace rootsig
+} // namespace hlsl
+} // namespace llvm
+
+#endif // LLVM_FRONTEND_HLSL_HLSLROOTSIGNATURE_H
>From a0999c783e834c8cb665e84535831c8ad905e6d9 Mon Sep 17 00:00:00 2001
From: Finn Plummer <finnplummer at microsoft.com>
Date: Thu, 27 Mar 2025 16:32:15 +0000
Subject: [PATCH 03/14] implement parsing of empty DescriptorTableClause
---
.../clang/Parse/ParseHLSLRootSignature.h | 1 +
clang/lib/Parse/ParseHLSLRootSignature.cpp | 52 +++++++++++++++++++
.../Parse/ParseHLSLRootSignatureTest.cpp | 27 ++++++++++
.../llvm/Frontend/HLSL/HLSLRootSignature.h | 11 +++-
4 files changed, 89 insertions(+), 2 deletions(-)
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index 95243747fbd6b..e7c4e0a5f0db8 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -42,6 +42,7 @@ class RootSignatureParser {
/// Root Element parse methods:
bool ParseDescriptorTable();
+ bool ParseDescriptorTableClause();
/// Invoke the Lexer to consume a token and update CurToken with the result
void ConsumeNextToken() { CurToken = Lexer.ConsumeToken(); }
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 86d587b10a8cb..1c45288bbd3d9 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -59,12 +59,27 @@ bool RootSignatureParser::Parse() {
}
bool RootSignatureParser::ParseDescriptorTable() {
+ assert(CurToken.Kind == TokenKind::kw_DescriptorTable &&
+ "Expects to only be invoked starting at given keyword");
+
DescriptorTable Table;
if (ConsumeExpectedToken(TokenKind::pu_l_paren, diag::err_expected_after,
CurToken.Kind))
return true;
+ // Iterate as many Clauses as possible
+ while (TryConsumeExpectedToken({TokenKind::kw_CBV, TokenKind::kw_SRV,
+ TokenKind::kw_UAV, TokenKind::kw_Sampler})) {
+ if (ParseDescriptorTableClause())
+ return true;
+
+ Table.NumClauses++;
+
+ if (!TryConsumeExpectedToken(TokenKind::pu_comma))
+ break;
+ }
+
if (ConsumeExpectedToken(TokenKind::pu_r_paren, diag::err_expected_after,
CurToken.Kind))
return true;
@@ -73,6 +88,43 @@ bool RootSignatureParser::ParseDescriptorTable() {
return false;
}
+bool RootSignatureParser::ParseDescriptorTableClause() {
+ assert((CurToken.Kind == TokenKind::kw_CBV ||
+ CurToken.Kind == TokenKind::kw_SRV ||
+ CurToken.Kind == TokenKind::kw_UAV ||
+ CurToken.Kind == TokenKind::kw_Sampler)
+ && "Expects to only be invoked starting at given keyword");
+
+ DescriptorTableClause Clause;
+ switch (CurToken.Kind) {
+ default: break; // Unreachable given Try + assert pattern
+ case TokenKind::kw_CBV:
+ Clause.Type = ClauseType::CBuffer;
+ break;
+ case TokenKind::kw_SRV:
+ Clause.Type = ClauseType::SRV;
+ break;
+ case TokenKind::kw_UAV:
+ Clause.Type = ClauseType::UAV;
+ break;
+ case TokenKind::kw_Sampler:
+ Clause.Type = ClauseType::Sampler;
+ break;
+ }
+
+ if (ConsumeExpectedToken(TokenKind::pu_l_paren, diag::err_expected_after,
+ CurToken.Kind))
+ return true;
+
+
+ if (ConsumeExpectedToken(TokenKind::pu_r_paren, diag::err_expected_after,
+ CurToken.Kind))
+ return true;
+
+ Elements.push_back(Clause);
+ return false;
+}
+
// Returns true when given token is one of the expected kinds
static bool IsExpectedToken(TokenKind Kind, ArrayRef<TokenKind> AnyExpected) {
for (auto Expected : AnyExpected)
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index 35b44ebdcffda..d5c3d9778987f 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -128,6 +128,12 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseEmptyTest) {
TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
const llvm::StringLiteral Source = R"cc(
+ DescriptorTable(
+ CBV(),
+ SRV(),
+ Sampler(),
+ UAV()
+ ),
DescriptorTable()
)cc";
@@ -143,9 +149,30 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
Consumer->SetNoDiag();
ASSERT_FALSE(Parser.Parse());
+
+ // First Descriptor Table with 4 elements
RootElement Elem = Elements[0];
+ ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::CBuffer);
+
+ Elem = Elements[1];
+ ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::SRV);
+
+ Elem = Elements[2];
+ ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::Sampler);
+
+ Elem = Elements[3];
+ ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::UAV);
+
+ Elem = Elements[4];
+ ASSERT_TRUE(std::holds_alternative<DescriptorTable>(Elem));
+ ASSERT_EQ(std::get<DescriptorTable>(Elem).NumClauses, (uint32_t)4);
// Empty Descriptor Table
+ Elem = Elements[5];
ASSERT_TRUE(std::holds_alternative<DescriptorTable>(Elem));
ASSERT_EQ(std::get<DescriptorTable>(Elem).NumClauses, 0u);
ASSERT_TRUE(Consumer->IsSatisfied());
diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
index 9bdd41e6dec7b..c1b67844c747f 100644
--- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
@@ -14,6 +14,7 @@
#ifndef LLVM_FRONTEND_HLSL_HLSLROOTSIGNATURE_H
#define LLVM_FRONTEND_HLSL_HLSLROOTSIGNATURE_H
+#include "llvm/Support/DXILABI.h"
#include <variant>
namespace llvm {
@@ -27,8 +28,14 @@ struct DescriptorTable {
uint32_t NumClauses = 0; // The number of clauses in the table
};
-// Models RootElement : DescriptorTable
-using RootElement = std::variant<DescriptorTable>;
+// Models DTClause : CBV | SRV | UAV | Sampler, by collecting like parameters
+using ClauseType = llvm::dxil::ResourceClass;
+struct DescriptorTableClause {
+ ClauseType Type;
+};
+
+// Models RootElement : DescriptorTable | DescriptorTableClause
+using RootElement = std::variant<DescriptorTable, DescriptorTableClause>;
} // namespace rootsig
} // namespace hlsl
>From 14c0479078f5ab5f05015fb8df4270cece5fdc26 Mon Sep 17 00:00:00 2001
From: Finn Plummer <finnplummer at microsoft.com>
Date: Thu, 27 Mar 2025 16:32:31 +0000
Subject: [PATCH 04/14] self-review: docs
---
clang/include/clang/Parse/ParseHLSLRootSignature.h | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index e7c4e0a5f0db8..09b520092fc9c 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -40,6 +40,20 @@ class RootSignatureParser {
private:
DiagnosticsEngine &Diags() { return PP.getDiagnostics(); }
+ /// All private Parse.* methods follow a similar pattern:
+ /// - Each method will start with an assert to denote what the CurToken is
+ /// expected to be and will parse from that token forward
+ /// - Therefore, it is the callers responsibility to ensure that you are
+ /// at the correct CurToken. This should be done with the pattern of:
+ /// if (TryConsumeExpectedToken(TokenKind)
+ /// if (Parse.*())
+ /// return true;
+ /// - All methods return true if a parsing error is encountered. It is the
+ /// callers responsibility to propogate this error up, or deal with it
+ /// otherwise
+ /// - An error will be raised if the proceeding tokens are not what is
+ /// expected, or, there is a lexing error
+
/// Root Element parse methods:
bool ParseDescriptorTable();
bool ParseDescriptorTableClause();
>From 9f0779502b9041ab734b96b904fb8ec261944198 Mon Sep 17 00:00:00 2001
From: Finn Plummer <finnplummer at microsoft.com>
Date: Thu, 27 Mar 2025 16:39:39 +0000
Subject: [PATCH 05/14] review: clang-format
---
clang/include/clang/Parse/ParseHLSLRootSignature.h | 2 +-
clang/lib/Parse/ParseHLSLRootSignature.cpp | 11 ++++++-----
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index 09b520092fc9c..2d1ec3dd7f0c7 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -14,8 +14,8 @@
#define LLVM_CLANG_PARSE_PARSEHLSLROOTSIGNATURE_H
#include "clang/Basic/DiagnosticParse.h"
-#include "clang/Lex/Preprocessor.h"
#include "clang/Lex/LexHLSLRootSignature.h"
+#include "clang/Lex/Preprocessor.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 1c45288bbd3d9..2dd01167eaf83 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -49,7 +49,8 @@ bool RootSignatureParser::Parse() {
llvm_unreachable("Switch for consumed token was not provided");
}
- if (Error) return true;
+ if (Error)
+ return true;
if (!TryConsumeExpectedToken(TokenKind::pu_comma))
break;
@@ -92,12 +93,13 @@ bool RootSignatureParser::ParseDescriptorTableClause() {
assert((CurToken.Kind == TokenKind::kw_CBV ||
CurToken.Kind == TokenKind::kw_SRV ||
CurToken.Kind == TokenKind::kw_UAV ||
- CurToken.Kind == TokenKind::kw_Sampler)
- && "Expects to only be invoked starting at given keyword");
+ CurToken.Kind == TokenKind::kw_Sampler) &&
+ "Expects to only be invoked starting at given keyword");
DescriptorTableClause Clause;
switch (CurToken.Kind) {
- default: break; // Unreachable given Try + assert pattern
+ default:
+ break; // Unreachable given Try + assert pattern
case TokenKind::kw_CBV:
Clause.Type = ClauseType::CBuffer;
break;
@@ -116,7 +118,6 @@ bool RootSignatureParser::ParseDescriptorTableClause() {
CurToken.Kind))
return true;
-
if (ConsumeExpectedToken(TokenKind::pu_r_paren, diag::err_expected_after,
CurToken.Kind))
return true;
>From 49cc48e0837a13673e3188bc7f195ebc8b1d4988 Mon Sep 17 00:00:00 2001
From: Finn Plummer <finnplummer at microsoft.com>
Date: Thu, 27 Mar 2025 19:14:06 +0000
Subject: [PATCH 06/14] self-review: fix up comment
---
.../clang/Parse/ParseHLSLRootSignature.h | 37 ++++++++++++-------
1 file changed, 24 insertions(+), 13 deletions(-)
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index 2d1ec3dd7f0c7..bd593fa3e8873 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -40,19 +40,30 @@ class RootSignatureParser {
private:
DiagnosticsEngine &Diags() { return PP.getDiagnostics(); }
- /// All private Parse.* methods follow a similar pattern:
- /// - Each method will start with an assert to denote what the CurToken is
- /// expected to be and will parse from that token forward
- /// - Therefore, it is the callers responsibility to ensure that you are
- /// at the correct CurToken. This should be done with the pattern of:
- /// if (TryConsumeExpectedToken(TokenKind)
- /// if (Parse.*())
- /// return true;
- /// - All methods return true if a parsing error is encountered. It is the
- /// callers responsibility to propogate this error up, or deal with it
- /// otherwise
- /// - An error will be raised if the proceeding tokens are not what is
- /// expected, or, there is a lexing error
+ // All private Parse.* methods follow a similar pattern:
+ // - Each method will start with an assert to denote what the CurToken is
+ // expected to be and will parse from that token forward
+ //
+ // - Therefore, it is the callers responsibility to ensure that you are
+ // at the correct CurToken. This should be done with the pattern of:
+ //
+ // if (TryConsumeExpectedToken(TokenKind))
+ // if (Parse.*())
+ // return true;
+ //
+ // or,
+ //
+ // if (ConsumeExpectedToken(TokenKind, ...))
+ // return true;
+ // if (Parse.*())
+ // return true;
+ //
+ // - All methods return true if a parsing error is encountered. It is the
+ // callers responsibility to propogate this error up, or deal with it
+ // otherwise
+ //
+ // - An error will be raised if the proceeding tokens are not what is
+ // expected, or, there is a lexing error
/// Root Element parse methods:
bool ParseDescriptorTable();
>From d87707f5dff59cd721a962551eade5a923a8ecb1 Mon Sep 17 00:00:00 2001
From: Finn Plummer <finnplummer at microsoft.com>
Date: Thu, 27 Mar 2025 19:45:05 +0000
Subject: [PATCH 07/14] review: fix build issue on api change
---
clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index d5c3d9778987f..b3a86e33f6293 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -81,8 +81,9 @@ class ParseHLSLRootSignatureTest : public ::testing::Test {
llvm::MemoryBuffer::getMemBuffer(Source);
SourceMgr.setMainFileID(SourceMgr.createFileID(std::move(Buf)));
- HeaderSearch HeaderInfo(std::make_shared<HeaderSearchOptions>(), SourceMgr,
- Diags, LangOpts, Target.get());
+ HeaderSearchOptions SearchOpts;
+ HeaderSearch HeaderInfo(SearchOpts, SourceMgr, Diags, LangOpts,
+ Target.get());
std::unique_ptr<Preprocessor> PP = std::make_unique<Preprocessor>(
std::make_shared<PreprocessorOptions>(), Diags, LangOpts, SourceMgr,
HeaderInfo, ModLoader,
>From d944fffb8384808d0c5403499be3d7b8b3eb7172 Mon Sep 17 00:00:00 2001
From: Finn Plummer <finnplummer at microsoft.com>
Date: Fri, 28 Mar 2025 17:03:06 +0000
Subject: [PATCH 08/14] review: return true from case
---
clang/lib/Parse/ParseHLSLRootSignature.cpp | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 2dd01167eaf83..05f9bc01af991 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -37,21 +37,18 @@ RootSignatureParser::RootSignatureParser(SmallVector<RootElement> &Elements,
bool RootSignatureParser::Parse() {
// Iterate as many RootElements as possible
while (TryConsumeExpectedToken(TokenKind::kw_DescriptorTable)) {
- bool Error = false;
// Dispatch onto parser method.
// We guard against the unreachable here as we just ensured that CurToken
// will be one of the kinds in the while condition
switch (CurToken.Kind) {
case TokenKind::kw_DescriptorTable:
- Error = ParseDescriptorTable();
+ if (ParseDescriptorTable())
+ return true;
break;
default:
llvm_unreachable("Switch for consumed token was not provided");
}
- if (Error)
- return true;
-
if (!TryConsumeExpectedToken(TokenKind::pu_comma))
break;
}
>From 2123282b556ed4a044b9175535f2528b44e4d686 Mon Sep 17 00:00:00 2001
From: Finn Plummer <finnplummer at microsoft.com>
Date: Fri, 28 Mar 2025 17:59:02 +0000
Subject: [PATCH 09/14] review: use unreachable
---
clang/lib/Parse/ParseHLSLRootSignature.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 05f9bc01af991..49a6a1fde3904 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -96,7 +96,7 @@ bool RootSignatureParser::ParseDescriptorTableClause() {
DescriptorTableClause Clause;
switch (CurToken.Kind) {
default:
- break; // Unreachable given Try + assert pattern
+ llvm_unreachable("Switch for consumed token was not provided");
case TokenKind::kw_CBV:
Clause.Type = ClauseType::CBuffer;
break;
>From 318f1ecd0bbe5f3a7af1cb32a336033ccdf16fb2 Mon Sep 17 00:00:00 2001
From: Finn Plummer <finnplummer at microsoft.com>
Date: Fri, 28 Mar 2025 18:09:24 +0000
Subject: [PATCH 10/14] review: conform to llvm method naming conventions
---
.../clang/Parse/ParseHLSLRootSignature.h | 22 ++++----
clang/lib/Parse/ParseHLSLRootSignature.cpp | 54 +++++++++----------
.../Parse/ParseHLSLRootSignatureTest.cpp | 48 ++++++++---------
3 files changed, 62 insertions(+), 62 deletions(-)
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index bd593fa3e8873..6a0d4936b4944 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -35,10 +35,10 @@ class RootSignatureParser {
/// error is encountered or the end of the buffer.
///
/// Returns true if a parsing error is encountered.
- bool Parse();
+ bool parse();
private:
- DiagnosticsEngine &Diags() { return PP.getDiagnostics(); }
+ DiagnosticsEngine &getDiags() { return PP.getDiagnostics(); }
// All private Parse.* methods follow a similar pattern:
// - Each method will start with an assert to denote what the CurToken is
@@ -66,24 +66,24 @@ class RootSignatureParser {
// expected, or, there is a lexing error
/// Root Element parse methods:
- bool ParseDescriptorTable();
- bool ParseDescriptorTableClause();
+ bool parseDescriptorTable();
+ bool parseDescriptorTableClause();
/// Invoke the Lexer to consume a token and update CurToken with the result
- void ConsumeNextToken() { CurToken = Lexer.ConsumeToken(); }
+ void consumeNextToken() { CurToken = Lexer.ConsumeToken(); }
/// Return true if the next token one of the expected kinds
- bool PeekExpectedToken(TokenKind Expected);
- bool PeekExpectedToken(ArrayRef<TokenKind> AnyExpected);
+ bool peekExpectedToken(TokenKind Expected);
+ bool peekExpectedToken(ArrayRef<TokenKind> AnyExpected);
/// Consumes the next token and report an error if it is not of the expected
/// kind.
///
/// Returns true if there was an error reported.
- bool ConsumeExpectedToken(TokenKind Expected,
+ bool consumeExpectedToken(TokenKind Expected,
unsigned DiagID = diag::err_expected,
TokenKind Context = TokenKind::invalid);
- bool ConsumeExpectedToken(ArrayRef<TokenKind> AnyExpected,
+ bool consumeExpectedToken(ArrayRef<TokenKind> AnyExpected,
unsigned DiagID = diag::err_expected,
TokenKind Context = TokenKind::invalid);
@@ -92,8 +92,8 @@ class RootSignatureParser {
///
/// Returns true if it successfully matches the expected kind and the token
/// was consumed.
- bool TryConsumeExpectedToken(TokenKind Expected);
- bool TryConsumeExpectedToken(ArrayRef<TokenKind> Expected);
+ bool tryConsumeExpectedToken(TokenKind Expected);
+ bool tryConsumeExpectedToken(ArrayRef<TokenKind> Expected);
private:
SmallVector<llvm::hlsl::rootsig::RootElement> &Elements;
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 49a6a1fde3904..860dbffbc4c65 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -34,51 +34,51 @@ RootSignatureParser::RootSignatureParser(SmallVector<RootElement> &Elements,
Preprocessor &PP)
: Elements(Elements), Lexer(Lexer), PP(PP), CurToken(SourceLocation()) {}
-bool RootSignatureParser::Parse() {
+bool RootSignatureParser::parse() {
// Iterate as many RootElements as possible
- while (TryConsumeExpectedToken(TokenKind::kw_DescriptorTable)) {
+ while (tryConsumeExpectedToken(TokenKind::kw_DescriptorTable)) {
// Dispatch onto parser method.
// We guard against the unreachable here as we just ensured that CurToken
// will be one of the kinds in the while condition
switch (CurToken.Kind) {
case TokenKind::kw_DescriptorTable:
- if (ParseDescriptorTable())
+ if (parseDescriptorTable())
return true;
break;
default:
llvm_unreachable("Switch for consumed token was not provided");
}
- if (!TryConsumeExpectedToken(TokenKind::pu_comma))
+ if (!tryConsumeExpectedToken(TokenKind::pu_comma))
break;
}
- return ConsumeExpectedToken(TokenKind::end_of_stream, diag::err_expected);
+ return consumeExpectedToken(TokenKind::end_of_stream, diag::err_expected);
}
-bool RootSignatureParser::ParseDescriptorTable() {
+bool RootSignatureParser::parseDescriptorTable() {
assert(CurToken.Kind == TokenKind::kw_DescriptorTable &&
"Expects to only be invoked starting at given keyword");
DescriptorTable Table;
- if (ConsumeExpectedToken(TokenKind::pu_l_paren, diag::err_expected_after,
+ if (consumeExpectedToken(TokenKind::pu_l_paren, diag::err_expected_after,
CurToken.Kind))
return true;
// Iterate as many Clauses as possible
- while (TryConsumeExpectedToken({TokenKind::kw_CBV, TokenKind::kw_SRV,
+ while (tryConsumeExpectedToken({TokenKind::kw_CBV, TokenKind::kw_SRV,
TokenKind::kw_UAV, TokenKind::kw_Sampler})) {
- if (ParseDescriptorTableClause())
+ if (parseDescriptorTableClause())
return true;
Table.NumClauses++;
- if (!TryConsumeExpectedToken(TokenKind::pu_comma))
+ if (!tryConsumeExpectedToken(TokenKind::pu_comma))
break;
}
- if (ConsumeExpectedToken(TokenKind::pu_r_paren, diag::err_expected_after,
+ if (consumeExpectedToken(TokenKind::pu_r_paren, diag::err_expected_after,
CurToken.Kind))
return true;
@@ -86,7 +86,7 @@ bool RootSignatureParser::ParseDescriptorTable() {
return false;
}
-bool RootSignatureParser::ParseDescriptorTableClause() {
+bool RootSignatureParser::parseDescriptorTableClause() {
assert((CurToken.Kind == TokenKind::kw_CBV ||
CurToken.Kind == TokenKind::kw_SRV ||
CurToken.Kind == TokenKind::kw_UAV ||
@@ -111,11 +111,11 @@ bool RootSignatureParser::ParseDescriptorTableClause() {
break;
}
- if (ConsumeExpectedToken(TokenKind::pu_l_paren, diag::err_expected_after,
+ if (consumeExpectedToken(TokenKind::pu_l_paren, diag::err_expected_after,
CurToken.Kind))
return true;
- if (ConsumeExpectedToken(TokenKind::pu_r_paren, diag::err_expected_after,
+ if (consumeExpectedToken(TokenKind::pu_r_paren, diag::err_expected_after,
CurToken.Kind))
return true;
@@ -131,29 +131,29 @@ static bool IsExpectedToken(TokenKind Kind, ArrayRef<TokenKind> AnyExpected) {
return false;
}
-bool RootSignatureParser::PeekExpectedToken(TokenKind Expected) {
- return PeekExpectedToken(ArrayRef{Expected});
+bool RootSignatureParser::peekExpectedToken(TokenKind Expected) {
+ return peekExpectedToken(ArrayRef{Expected});
}
-bool RootSignatureParser::PeekExpectedToken(ArrayRef<TokenKind> AnyExpected) {
+bool RootSignatureParser::peekExpectedToken(ArrayRef<TokenKind> AnyExpected) {
RootSignatureToken Result = Lexer.PeekNextToken();
return IsExpectedToken(Result.Kind, AnyExpected);
}
-bool RootSignatureParser::ConsumeExpectedToken(TokenKind Expected,
+bool RootSignatureParser::consumeExpectedToken(TokenKind Expected,
unsigned DiagID,
TokenKind Context) {
- return ConsumeExpectedToken(ArrayRef{Expected}, DiagID, Context);
+ return consumeExpectedToken(ArrayRef{Expected}, DiagID, Context);
}
-bool RootSignatureParser::ConsumeExpectedToken(ArrayRef<TokenKind> AnyExpected,
+bool RootSignatureParser::consumeExpectedToken(ArrayRef<TokenKind> AnyExpected,
unsigned DiagID,
TokenKind Context) {
- if (TryConsumeExpectedToken(AnyExpected))
+ if (tryConsumeExpectedToken(AnyExpected))
return false;
// Report unexpected token kind error
- DiagnosticBuilder DB = Diags().Report(CurToken.TokLoc, DiagID);
+ DiagnosticBuilder DB = getDiags().Report(CurToken.TokLoc, DiagID);
switch (DiagID) {
case diag::err_expected:
DB << FormatTokenKinds(AnyExpected);
@@ -168,16 +168,16 @@ bool RootSignatureParser::ConsumeExpectedToken(ArrayRef<TokenKind> AnyExpected,
return true;
}
-bool RootSignatureParser::TryConsumeExpectedToken(TokenKind Expected) {
- return TryConsumeExpectedToken(ArrayRef{Expected});
+bool RootSignatureParser::tryConsumeExpectedToken(TokenKind Expected) {
+ return tryConsumeExpectedToken(ArrayRef{Expected});
}
-bool RootSignatureParser::TryConsumeExpectedToken(
+bool RootSignatureParser::tryConsumeExpectedToken(
ArrayRef<TokenKind> AnyExpected) {
// If not the expected token just return
- if (!PeekExpectedToken(AnyExpected))
+ if (!peekExpectedToken(AnyExpected))
return false;
- ConsumeNextToken();
+ consumeNextToken();
return true;
}
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index b3a86e33f6293..cbd01170b38c9 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -49,17 +49,17 @@ class ExpectedDiagConsumer : public DiagnosticConsumer {
std::optional<unsigned> ExpectedDiagID;
public:
- void SetNoDiag() {
+ void setNoDiag() {
Satisfied = true;
ExpectedDiagID = std::nullopt;
}
- void SetExpected(unsigned DiagID) {
+ void setExpected(unsigned DiagID) {
Satisfied = false;
ExpectedDiagID = DiagID;
}
- bool IsSatisfied() { return Satisfied; }
+ bool isSatisfied() { return Satisfied; }
};
// The test fixture.
@@ -75,7 +75,7 @@ class ParseHLSLRootSignatureTest : public ::testing::Test {
Target = TargetInfo::CreateTargetInfo(Diags, TargetOpts);
}
- std::unique_ptr<Preprocessor> CreatePP(StringRef Source,
+ std::unique_ptr<Preprocessor> createPP(StringRef Source,
TrivialModuleLoader &ModLoader) {
std::unique_ptr<llvm::MemoryBuffer> Buf =
llvm::MemoryBuffer::getMemBuffer(Source);
@@ -111,7 +111,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseEmptyTest) {
const llvm::StringLiteral Source = R"cc()cc";
TrivialModuleLoader ModLoader;
- auto PP = CreatePP(Source, ModLoader);
+ auto PP = createPP(Source, ModLoader);
auto TokLoc = SourceLocation();
hlsl::RootSignatureLexer Lexer(Source, TokLoc);
@@ -119,12 +119,12 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseEmptyTest) {
hlsl::RootSignatureParser Parser(Elements, Lexer, *PP);
// Test no diagnostics produced
- Consumer->SetNoDiag();
+ Consumer->setNoDiag();
- ASSERT_FALSE(Parser.Parse());
+ ASSERT_FALSE(Parser.parse());
ASSERT_EQ((int)Elements.size(), 0);
- ASSERT_TRUE(Consumer->IsSatisfied());
+ ASSERT_TRUE(Consumer->isSatisfied());
}
TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
@@ -139,7 +139,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
)cc";
TrivialModuleLoader ModLoader;
- auto PP = CreatePP(Source, ModLoader);
+ auto PP = createPP(Source, ModLoader);
auto TokLoc = SourceLocation();
hlsl::RootSignatureLexer Lexer(Source, TokLoc);
@@ -147,9 +147,9 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
hlsl::RootSignatureParser Parser(Elements, Lexer, *PP);
// Test no diagnostics produced
- Consumer->SetNoDiag();
+ Consumer->setNoDiag();
- ASSERT_FALSE(Parser.Parse());
+ ASSERT_FALSE(Parser.parse());
// First Descriptor Table with 4 elements
RootElement Elem = Elements[0];
@@ -176,7 +176,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
Elem = Elements[5];
ASSERT_TRUE(std::holds_alternative<DescriptorTable>(Elem));
ASSERT_EQ(std::get<DescriptorTable>(Elem).NumClauses, 0u);
- ASSERT_TRUE(Consumer->IsSatisfied());
+ ASSERT_TRUE(Consumer->isSatisfied());
}
// Invalid Parser Tests
@@ -188,7 +188,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidParseUnexpectedTokenTest) {
)cc";
TrivialModuleLoader ModLoader;
- auto PP = CreatePP(Source, ModLoader);
+ auto PP = createPP(Source, ModLoader);
auto TokLoc = SourceLocation();
hlsl::RootSignatureLexer Lexer(Source, TokLoc);
@@ -196,10 +196,10 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidParseUnexpectedTokenTest) {
hlsl::RootSignatureParser Parser(Elements, Lexer, *PP);
// Test correct diagnostic produced
- Consumer->SetExpected(diag::err_expected);
- ASSERT_TRUE(Parser.Parse());
+ Consumer->setExpected(diag::err_expected);
+ ASSERT_TRUE(Parser.parse());
- ASSERT_TRUE(Consumer->IsSatisfied());
+ ASSERT_TRUE(Consumer->isSatisfied());
}
TEST_F(ParseHLSLRootSignatureTest, InvalidParseInvalidTokenTest) {
@@ -208,7 +208,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidParseInvalidTokenTest) {
)cc";
TrivialModuleLoader ModLoader;
- auto PP = CreatePP(Source, ModLoader);
+ auto PP = createPP(Source, ModLoader);
auto TokLoc = SourceLocation();
hlsl::RootSignatureLexer Lexer(Source, TokLoc);
@@ -216,10 +216,10 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidParseInvalidTokenTest) {
hlsl::RootSignatureParser Parser(Elements, Lexer, *PP);
// Test correct diagnostic produced - invalid token
- Consumer->SetExpected(diag::err_expected);
- ASSERT_TRUE(Parser.Parse());
+ Consumer->setExpected(diag::err_expected);
+ ASSERT_TRUE(Parser.parse());
- ASSERT_TRUE(Consumer->IsSatisfied());
+ ASSERT_TRUE(Consumer->isSatisfied());
}
TEST_F(ParseHLSLRootSignatureTest, InvalidParseUnexpectedEndOfStreamTest) {
@@ -228,7 +228,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidParseUnexpectedEndOfStreamTest) {
)cc";
TrivialModuleLoader ModLoader;
- auto PP = CreatePP(Source, ModLoader);
+ auto PP = createPP(Source, ModLoader);
auto TokLoc = SourceLocation();
hlsl::RootSignatureLexer Lexer(Source, TokLoc);
@@ -236,10 +236,10 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidParseUnexpectedEndOfStreamTest) {
hlsl::RootSignatureParser Parser(Elements, Lexer, *PP);
// Test correct diagnostic produced - end of stream
- Consumer->SetExpected(diag::err_expected_after);
- ASSERT_TRUE(Parser.Parse());
+ Consumer->setExpected(diag::err_expected_after);
+ ASSERT_TRUE(Parser.parse());
- ASSERT_TRUE(Consumer->IsSatisfied());
+ ASSERT_TRUE(Consumer->isSatisfied());
}
} // anonymous namespace
>From 2af4de6becbe9efe67822b162743222f5cd29286 Mon Sep 17 00:00:00 2001
From: Finn Plummer <finnplummer at microsoft.com>
Date: Fri, 28 Mar 2025 18:20:18 +0000
Subject: [PATCH 11/14] review: use llvm::is_contained instead of custom
function
---
clang/lib/Parse/ParseHLSLRootSignature.cpp | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 860dbffbc4c65..58af7ad0327c4 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -123,21 +123,13 @@ bool RootSignatureParser::parseDescriptorTableClause() {
return false;
}
-// Returns true when given token is one of the expected kinds
-static bool IsExpectedToken(TokenKind Kind, ArrayRef<TokenKind> AnyExpected) {
- for (auto Expected : AnyExpected)
- if (Kind == Expected)
- return true;
- return false;
-}
-
bool RootSignatureParser::peekExpectedToken(TokenKind Expected) {
return peekExpectedToken(ArrayRef{Expected});
}
bool RootSignatureParser::peekExpectedToken(ArrayRef<TokenKind> AnyExpected) {
RootSignatureToken Result = Lexer.PeekNextToken();
- return IsExpectedToken(Result.Kind, AnyExpected);
+ return llvm::is_contained(AnyExpected, Result.Kind);
}
bool RootSignatureParser::consumeExpectedToken(TokenKind Expected,
>From 78c3837604a0afb0267bf5d7d18f1b7aa4516bfb Mon Sep 17 00:00:00 2001
From: Finn Plummer <finnplummer at microsoft.com>
Date: Fri, 28 Mar 2025 18:26:00 +0000
Subject: [PATCH 12/14] review: typos
---
clang/include/clang/Parse/ParseHLSLRootSignature.h | 2 +-
clang/lib/Parse/ParseHLSLRootSignature.cpp | 10 ++++++++--
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index 6a0d4936b4944..0fcfc9a80ba31 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -6,7 +6,7 @@
//
//===----------------------------------------------------------------------===//
//
-// This file defines the ParseHLSLRootSignature interface.
+// This file defines the RootSignatureParser interface.
//
//===----------------------------------------------------------------------===//
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 58af7ad0327c4..01b33993541ca 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -1,3 +1,11 @@
+//=== ParseHLSLRootSignature.cpp - Parse Root Signature -------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
#include "clang/Parse/ParseHLSLRootSignature.h"
#include "llvm/Support/raw_ostream.h"
@@ -27,8 +35,6 @@ static std::string FormatTokenKinds(ArrayRef<TokenKind> Kinds) {
return TokenString;
}
-// Parser Definitions
-
RootSignatureParser::RootSignatureParser(SmallVector<RootElement> &Elements,
RootSignatureLexer &Lexer,
Preprocessor &PP)
>From 262eeedc16262a2ea7f8e4c67abe1677456fb53e Mon Sep 17 00:00:00 2001
From: Finn Plummer <finnplummer at microsoft.com>
Date: Fri, 28 Mar 2025 19:16:14 +0000
Subject: [PATCH 13/14] review: remove api of `consumeExpectedToken` for
multiple token kinds
- using `consumeExpectedToken` with multiple expected tokens leads to
poor handling of diagnostic messaging and would largely be used in cases
where more specific errors could be done
- instead we should limit the api to just using `consumeExpectedToken`
with only a specific token and to only be used when it is very clear
that only one specific token should follow after another
---
.../include/clang/Lex/LexHLSLRootSignature.h | 13 ++++++++
.../clang/Parse/ParseHLSLRootSignature.h | 3 --
clang/lib/Parse/ParseHLSLRootSignature.cpp | 32 ++-----------------
3 files changed, 16 insertions(+), 32 deletions(-)
diff --git a/clang/include/clang/Lex/LexHLSLRootSignature.h b/clang/include/clang/Lex/LexHLSLRootSignature.h
index b82237411b2ab..a7e1f782b767f 100644
--- a/clang/include/clang/Lex/LexHLSLRootSignature.h
+++ b/clang/include/clang/Lex/LexHLSLRootSignature.h
@@ -13,6 +13,7 @@
#ifndef LLVM_CLANG_LEX_LEXHLSLROOTSIGNATURE_H
#define LLVM_CLANG_LEX_LEXHLSLROOTSIGNATURE_H
+#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/SourceLocation.h"
#include "llvm/ADT/SmallVector.h"
@@ -43,6 +44,18 @@ struct RootSignatureToken {
};
using TokenKind = enum RootSignatureToken::Kind;
+inline const DiagnosticBuilder &operator<<(const DiagnosticBuilder &DB,
+ const TokenKind Kind) {
+ switch (Kind) {
+#define TOK(X, SPELLING) \
+ case TokenKind::X: \
+ DB << SPELLING; \
+ break;
+#include "clang/Lex/HLSLRootSignatureTokenKinds.def"
+ }
+ return DB;
+}
+
class RootSignatureLexer {
public:
RootSignatureLexer(StringRef Signature, clang::SourceLocation SourceLoc)
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index 0fcfc9a80ba31..43b41315b88b5 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -83,9 +83,6 @@ class RootSignatureParser {
bool consumeExpectedToken(TokenKind Expected,
unsigned DiagID = diag::err_expected,
TokenKind Context = TokenKind::invalid);
- bool consumeExpectedToken(ArrayRef<TokenKind> AnyExpected,
- unsigned DiagID = diag::err_expected,
- TokenKind Context = TokenKind::invalid);
/// Peek if the next token is of the expected kind and if it is then consume
/// it.
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 01b33993541ca..48ccc73a78358 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -15,26 +15,6 @@ using namespace llvm::hlsl::rootsig;
namespace clang {
namespace hlsl {
-static std::string FormatTokenKinds(ArrayRef<TokenKind> Kinds) {
- std::string TokenString;
- llvm::raw_string_ostream Out(TokenString);
- bool First = true;
- for (auto Kind : Kinds) {
- if (!First)
- Out << ", ";
- switch (Kind) {
-#define TOK(X, SPELLING) \
- case TokenKind::X: \
- Out << SPELLING; \
- break;
-#include "clang/Lex/HLSLRootSignatureTokenKinds.def"
- }
- First = false;
- }
-
- return TokenString;
-}
-
RootSignatureParser::RootSignatureParser(SmallVector<RootElement> &Elements,
RootSignatureLexer &Lexer,
Preprocessor &PP)
@@ -141,24 +121,18 @@ bool RootSignatureParser::peekExpectedToken(ArrayRef<TokenKind> AnyExpected) {
bool RootSignatureParser::consumeExpectedToken(TokenKind Expected,
unsigned DiagID,
TokenKind Context) {
- return consumeExpectedToken(ArrayRef{Expected}, DiagID, Context);
-}
-
-bool RootSignatureParser::consumeExpectedToken(ArrayRef<TokenKind> AnyExpected,
- unsigned DiagID,
- TokenKind Context) {
- if (tryConsumeExpectedToken(AnyExpected))
+ if (tryConsumeExpectedToken(Expected))
return false;
// Report unexpected token kind error
DiagnosticBuilder DB = getDiags().Report(CurToken.TokLoc, DiagID);
switch (DiagID) {
case diag::err_expected:
- DB << FormatTokenKinds(AnyExpected);
+ DB << Expected;
break;
case diag::err_expected_either:
case diag::err_expected_after:
- DB << FormatTokenKinds(AnyExpected) << FormatTokenKinds({Context});
+ DB << Expected << Context;
break;
default:
break;
>From 2eeda54370906a5eca76e343323ce27f7ed36a88 Mon Sep 17 00:00:00 2001
From: Finn Plummer <finnplummer at microsoft.com>
Date: Fri, 28 Mar 2025 20:23:15 +0000
Subject: [PATCH 14/14] self-review: improve error messaging for expected end
of parameters
---
clang/include/clang/Basic/DiagnosticParseKinds.td | 4 ++++
.../clang/Lex/HLSLRootSignatureTokenKinds.def | 1 +
clang/lib/Parse/ParseHLSLRootSignature.cpp | 15 ++++++++++++---
clang/unittests/Lex/LexHLSLRootSignatureTest.cpp | 2 ++
.../Parse/ParseHLSLRootSignatureTest.cpp | 4 ++--
5 files changed, 21 insertions(+), 5 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 86c361b4dbcf7..2582e1e5ef0f6 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1830,4 +1830,8 @@ def err_hlsl_virtual_function
def err_hlsl_virtual_inheritance
: Error<"virtual inheritance is unsupported in HLSL">;
+// HLSL Root Siganture diagnostic messages
+def err_hlsl_unexpected_end_of_params
+ : Error<"expected %0 to denote end of parameters, or, another valid parameter of %1">;
+
} // end of Parser diagnostics
diff --git a/clang/include/clang/Lex/HLSLRootSignatureTokenKinds.def b/clang/include/clang/Lex/HLSLRootSignatureTokenKinds.def
index 697c6abc54efa..c514d3456146a 100644
--- a/clang/include/clang/Lex/HLSLRootSignatureTokenKinds.def
+++ b/clang/include/clang/Lex/HLSLRootSignatureTokenKinds.def
@@ -69,6 +69,7 @@ PUNCTUATOR(plus, '+')
PUNCTUATOR(minus, '-')
// RootElement Keywords:
+KEYWORD(RootSignature) // used only for diagnostic messaging
KEYWORD(DescriptorTable)
// DescriptorTable Keywords:
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 48ccc73a78358..33caca5fa1c82 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -39,7 +39,13 @@ bool RootSignatureParser::parse() {
break;
}
- return consumeExpectedToken(TokenKind::end_of_stream, diag::err_expected);
+ if (!tryConsumeExpectedToken(TokenKind::end_of_stream)) {
+ getDiags().Report(CurToken.TokLoc, diag::err_hlsl_unexpected_end_of_params)
+ << /*expected=*/TokenKind::end_of_stream
+ << /*param of=*/TokenKind::kw_RootSignature;
+ return true;
+ }
+ return false;
}
bool RootSignatureParser::parseDescriptorTable() {
@@ -64,9 +70,12 @@ bool RootSignatureParser::parseDescriptorTable() {
break;
}
- if (consumeExpectedToken(TokenKind::pu_r_paren, diag::err_expected_after,
- CurToken.Kind))
+ if (!tryConsumeExpectedToken(TokenKind::pu_r_paren)) {
+ getDiags().Report(CurToken.TokLoc, diag::err_hlsl_unexpected_end_of_params)
+ << /*expected=*/TokenKind::pu_r_paren
+ << /*param of=*/TokenKind::kw_DescriptorTable;
return true;
+ }
Elements.push_back(Table);
return false;
diff --git a/clang/unittests/Lex/LexHLSLRootSignatureTest.cpp b/clang/unittests/Lex/LexHLSLRootSignatureTest.cpp
index 1e014f6c41c57..3b13e295f7c87 100644
--- a/clang/unittests/Lex/LexHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Lex/LexHLSLRootSignatureTest.cpp
@@ -85,6 +85,8 @@ TEST_F(LexHLSLRootSignatureTest, ValidLexAllTokensTest) {
(),|=+-
+ RootSignature
+
DescriptorTable
CBV SRV UAV Sampler
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index cbd01170b38c9..acdf455a5d6aa 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -196,7 +196,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidParseUnexpectedTokenTest) {
hlsl::RootSignatureParser Parser(Elements, Lexer, *PP);
// Test correct diagnostic produced
- Consumer->setExpected(diag::err_expected);
+ Consumer->setExpected(diag::err_hlsl_unexpected_end_of_params);
ASSERT_TRUE(Parser.parse());
ASSERT_TRUE(Consumer->isSatisfied());
@@ -216,7 +216,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidParseInvalidTokenTest) {
hlsl::RootSignatureParser Parser(Elements, Lexer, *PP);
// Test correct diagnostic produced - invalid token
- Consumer->setExpected(diag::err_expected);
+ Consumer->setExpected(diag::err_hlsl_unexpected_end_of_params);
ASSERT_TRUE(Parser.parse());
ASSERT_TRUE(Consumer->isSatisfied());
More information about the llvm-commits
mailing list