bogner wrote:
I'm not entirely convinced the generic `ParsedParamState` is the right level of
abstraction. In some ways the declarative nature is nice, but I think it's
quite a bit more complex than a simple recursive descent here.
Warning - lots of untested and never compiled code follows. Consider an API
like so:
```c++
struct ParsedParam {
std::optional<llvm::hlsl::rootsig::Register> Register;
std::optional<uint32_t> Space;
};
std::optional<ParsedParam> parseParameter(TokenKind RegType);
```
This could then be used in `parseDescriptorClause for the various parameter
kinds:
```c++
DescriptorTableClause Clause;
std::optional<ParsedParam> P;
switch (ParamKind) {
default:
llvm_unreachable("Switch for consumed token was not provided");
case TokenKind::kw_CBV:
Clause.Type = ClauseType::CBuffer;
P = parseParameter(TokenKind::bReg);
break;
// ...
}
if (!P.has_value())
// diagnose
// otherwise we have a parameter!
```
then parseParameter can just can follow the same pattern and just recurse into
the various members:
```c++
std::optional<ParsedParam> parseParameter(TokenKind RegType) {
if (consumeExpectedToken(TokenKind::pu_l_paren, diag::err_expected_after,
ParamKind))
return std::nullopt;
ParsedParam Result;
do {
if (tryConsumeExpectedToken(RegType)) {
if (Result.Register.has_value()) {
getDiags().Report(CurToken.TokLoc, diag::err_hlsl_rootsig_repeat_param)
<< CurToken.TokKind;
return true;
}
if (auto Register = parseRegister(Params.Register))
Result.Register = *Register;
}
if (tryConsumeExpectedToken(RootSignatureToken::Kind::kw_space)) {
if (Result.Space.has_value()) {
getDiags().Report(CurToken.TokLoc, diag::err_hlsl_rootsig_repeat_param)
<< CurToken.TokKind;
return true;
}
if (auto Space = parseUIntParam())
Result.Register = *Register;
}
} while (tryConsumeExpectedToken(TokenKind::pu_comma));
if (!consumeExpectedToken(TokenKind::pu_r_paren,
diag::err_hlsl_unexpected_end_of_params,
/*param of=*/ParamKind))
return std::nullopt;
if (!Result.Register.has_value()) {
getDiags().Report(CurToken.TokLoc, diag::err_hlsl_rootsig_missing_param)
<< ExpectedRegister;
return std::nullopt;
}
return Result;
```
This also makes the functions match the shape of the grammar formulations in
the EBNF notation. I suspect that any code duplication we find by doing it this
way would be fairly easy to clean up with a few helper functions here and
there, and since the functions should stay relatively short I think it keeps it
reasonably simple.
https://github.com/llvm/llvm-project/pull/133800
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
