Skip to content

[NFC][RootSignatures] Conform to new std::optional calling conventions #136747

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 24, 2025
Merged

[NFC][RootSignatures] Conform to new std::optional calling conventions #136747

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions clang/include/clang/Parse/ParseHLSLRootSignature.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,9 @@ class RootSignatureParser {
// expected, or, there is a lexing error

/// Root Element parse methods:
bool parseDescriptorTable();
bool parseDescriptorTableClause();
std::optional<llvm::hlsl::rootsig::DescriptorTable> parseDescriptorTable();
std::optional<llvm::hlsl::rootsig::DescriptorTableClause>
parseDescriptorTableClause();

/// Parameter arguments (eg. `bReg`, `space`, ...) can be specified in any
/// order and only exactly once. `ParsedClauseParams` denotes the current
Expand Down
67 changes: 29 additions & 38 deletions clang/lib/Parse/ParseHLSLRootSignature.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,22 +26,14 @@ RootSignatureParser::RootSignatureParser(SmallVector<RootElement> &Elements,

bool RootSignatureParser::parse() {
// Iterate as many RootElements as possible
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.TokKind) {
case TokenKind::kw_DescriptorTable:
if (parseDescriptorTable())
do {
if (tryConsumeExpectedToken(TokenKind::kw_DescriptorTable)) {
auto Table = parseDescriptorTable();
if (!Table.has_value())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: std::optional should already implement operator bool()

Suggested change
if (!Table.has_value())
if (!Table)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like there are a bunch of cases like this, I'll just leave the one comment but please update everywhere

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will merge as-is for now since this explicit version is already established in the file.

I can create a clean-up pr later to adjust this throughout the whole file.

return true;
break;
default:
llvm_unreachable("Switch for consumed token was not provided");
Elements.push_back(*Table);
}

if (!tryConsumeExpectedToken(TokenKind::pu_comma))
break;
}
} while (tryConsumeExpectedToken(TokenKind::pu_comma));

if (consumeExpectedToken(TokenKind::end_of_stream,
diag::err_hlsl_unexpected_end_of_params,
Expand All @@ -51,38 +43,38 @@ bool RootSignatureParser::parse() {
return false;
}

bool RootSignatureParser::parseDescriptorTable() {
std::optional<DescriptorTable> RootSignatureParser::parseDescriptorTable() {
assert(CurToken.TokKind == 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.TokKind))
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;
return std::nullopt;

Table.NumClauses++;
DescriptorTable Table;

if (!tryConsumeExpectedToken(TokenKind::pu_comma))
break;
}
// Iterate as many Clauses as possible
do {
if (tryConsumeExpectedToken({TokenKind::kw_CBV, TokenKind::kw_SRV,
TokenKind::kw_UAV, TokenKind::kw_Sampler})) {
auto Clause = parseDescriptorTableClause();
if (!Clause.has_value())
return std::nullopt;
Elements.push_back(*Clause);
Table.NumClauses++;
}
} while (tryConsumeExpectedToken(TokenKind::pu_comma));

if (consumeExpectedToken(TokenKind::pu_r_paren,
diag::err_hlsl_unexpected_end_of_params,
/*param of=*/TokenKind::kw_DescriptorTable))
return true;
return std::nullopt;

Elements.push_back(Table);
return false;
return Table;
}

bool RootSignatureParser::parseDescriptorTableClause() {
std::optional<DescriptorTableClause>
RootSignatureParser::parseDescriptorTableClause() {
assert((CurToken.TokKind == TokenKind::kw_CBV ||
CurToken.TokKind == TokenKind::kw_SRV ||
CurToken.TokKind == TokenKind::kw_UAV ||
Expand All @@ -93,7 +85,7 @@ bool RootSignatureParser::parseDescriptorTableClause() {

if (consumeExpectedToken(TokenKind::pu_l_paren, diag::err_expected_after,
CurToken.TokKind))
return true;
return std::nullopt;

DescriptorTableClause Clause;
TokenKind ExpectedReg;
Expand All @@ -120,13 +112,13 @@ bool RootSignatureParser::parseDescriptorTableClause() {

auto Params = parseDescriptorTableClauseParams(ExpectedReg);
if (!Params.has_value())
return true;
return std::nullopt;

// Check mandatory parameters were provided
if (!Params->Reg.has_value()) {
getDiags().Report(CurToken.TokLoc, diag::err_hlsl_rootsig_missing_param)
<< ExpectedReg;
return true;
return std::nullopt;
}

Clause.Reg = Params->Reg.value();
Expand All @@ -138,10 +130,9 @@ bool RootSignatureParser::parseDescriptorTableClause() {
if (consumeExpectedToken(TokenKind::pu_r_paren,
diag::err_hlsl_unexpected_end_of_params,
/*param of=*/ParamKind))
return true;
return std::nullopt;

Elements.push_back(Clause);
return false;
return Clause;
}

std::optional<RootSignatureParser::ParsedClauseParams>
Expand Down
Loading