NFC review: switch calling convention to use std::optional instead of bool

Finn Plummer · Finn Plummer · commit 47139b640854 · 2025-04-17T19:04:10.000Z
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -66,24 +66,26 @@ 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();
 
   struct ParsedParams {
     std::optional<llvm::hlsl::rootsig::Register> Register;
     std::optional<uint32_t> Space;
   };
   /// Parses out a `ParsedParams` for the caller to use for construction
   /// of the in-memory representation of a Root Element.
-  bool parseDescriptorTableClauseParams(ParsedParams &Params, RootSignatureToken::Kind RegType);
+  std::optional<ParsedParams>
+  parseDescriptorTableClauseParams(RootSignatureToken::Kind RegType);
 
   /// Parameter parse methods corresponding to a ParamType
-  bool parseUIntParam(uint32_t &X);
-  bool parseRegister(llvm::hlsl::rootsig::Register &Reg);
+  std::optional<uint32_t> parseUIntParam();
+  std::optional<llvm::hlsl::rootsig::Register> parseRegister();
 
   /// Use NumericLiteralParser to convert CurToken.NumSpelling into a unsigned
   /// 32-bit integer
-  bool handleUIntLiteral(uint32_t &X);
+  std::optional<uint32_t> handleUIntLiteral();
 
   /// 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
@@ -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())
         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,
@@ -51,147 +43,153 @@ 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;
+    return std::nullopt;
 
-  // 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++;
+  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 ||
           CurToken.TokKind == TokenKind::kw_Sampler) &&
          "Expects to only be invoked starting at given keyword");
-  TokenKind ParamKind = CurToken.TokKind; // retain for diagnostics
+
+  TokenKind ParamKind = CurToken.TokKind;
+
+  if (consumeExpectedToken(TokenKind::pu_l_paren, diag::err_expected_after,
+                           CurToken.TokKind))
+    return std::nullopt;
 
   DescriptorTableClause Clause;
-  TokenKind ExpectedRegister;
+  TokenKind ExpectedReg;
   switch (ParamKind) {
   default:
     llvm_unreachable("Switch for consumed token was not provided");
   case TokenKind::kw_CBV:
     Clause.Type = ClauseType::CBuffer;
-    ExpectedRegister = TokenKind::bReg;
+    ExpectedReg = TokenKind::bReg;
     break;
   case TokenKind::kw_SRV:
     Clause.Type = ClauseType::SRV;
-    ExpectedRegister = TokenKind::tReg;
+    ExpectedReg = TokenKind::tReg;
     break;
   case TokenKind::kw_UAV:
     Clause.Type = ClauseType::UAV;
-    ExpectedRegister = TokenKind::uReg;
+    ExpectedReg = TokenKind::uReg;
     break;
   case TokenKind::kw_Sampler:
     Clause.Type = ClauseType::Sampler;
-    ExpectedRegister = TokenKind::sReg;
+    ExpectedReg = TokenKind::sReg;
     break;
   }
 
-  if (consumeExpectedToken(TokenKind::pu_l_paren, diag::err_expected_after,
-                           ParamKind))
-    return true;
-
-  ParsedParams Result;
-  if (parseDescriptorTableClauseParams(Result, ExpectedRegister))
-    return true;
+  auto Params = parseDescriptorTableClauseParams(ExpectedReg);
+  if (!Params.has_value())
+    return std::nullopt;
 
   // Check mandatory parameters were provided
-  if (!Result.Register.has_value()) {
+  if (!Params->Register.has_value()) {
     getDiags().Report(CurToken.TokLoc, diag::err_hlsl_rootsig_missing_param)
-        << ExpectedRegister;
-    return true;
+        << ExpectedReg;
+    return std::nullopt;
   }
 
-  Clause.Register = *Result.Register;
+  Clause.Register = Params->Register.value();
 
-  if (Result.Space)
-    Clause.Space = *Result.Space;
+  // Fill in optional values
+  if (Params->Space.has_value())
+    Clause.Space = Params->Space.value();
 
   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;
 }
 
-bool RootSignatureParser::parseDescriptorTableClauseParams(ParsedParams &Params, TokenKind RegType) {
+std::optional<RootSignatureParser::ParsedParams>
+RootSignatureParser::parseDescriptorTableClauseParams(TokenKind RegType) {
   assert(CurToken.TokKind == TokenKind::pu_l_paren &&
          "Expects to only be invoked starting at given token");
 
+  ParsedParams Params;
   do {
     if (tryConsumeExpectedToken(RegType)) {
       if (Params.Register.has_value()) {
         getDiags().Report(CurToken.TokLoc, diag::err_hlsl_rootsig_repeat_param)
-          << CurToken.TokKind;
-        return true;
+            << CurToken.TokKind;
+        return std::nullopt;
       }
-      Register Reg;
-      if (parseRegister(Reg))
-        return true;
+      auto Reg = parseRegister();
+      if (!Reg.has_value())
+        return std::nullopt;
       Params.Register = Reg;
     }
+
     if (tryConsumeExpectedToken(TokenKind::kw_space)) {
       if (Params.Space.has_value()) {
         getDiags().Report(CurToken.TokLoc, diag::err_hlsl_rootsig_repeat_param)
-          << CurToken.TokKind;
-        return true;
+            << CurToken.TokKind;
+        return std::nullopt;
       }
       if (consumeExpectedToken(TokenKind::pu_equal))
-        return true;
-      uint32_t Space;
-      if (parseUIntParam(Space))
-        return true;
+        return std::nullopt;
+      auto Space = parseUIntParam();
+      if (!Space.has_value())
+        return std::nullopt;
       Params.Space = Space;
     }
   } while (tryConsumeExpectedToken(TokenKind::pu_comma));
 
-  return false;
+  return Params;
 }
 
-bool RootSignatureParser::parseUIntParam(uint32_t &X) {
+std::optional<uint32_t> RootSignatureParser::parseUIntParam() {
   assert(CurToken.TokKind == TokenKind::pu_equal &&
          "Expects to only be invoked starting at given keyword");
   tryConsumeExpectedToken(TokenKind::pu_plus);
-  return consumeExpectedToken(TokenKind::int_literal, diag::err_expected_after,
-                              CurToken.TokKind) ||
-         handleUIntLiteral(X);
+  if (consumeExpectedToken(TokenKind::int_literal, diag::err_expected_after,
+                           CurToken.TokKind))
+    return std::nullopt;
+  return handleUIntLiteral();
 }
 
-bool RootSignatureParser::parseRegister(Register &Register) {
+std::optional<Register> RootSignatureParser::parseRegister() {
   assert((CurToken.TokKind == TokenKind::bReg ||
           CurToken.TokKind == TokenKind::tReg ||
           CurToken.TokKind == TokenKind::uReg ||
           CurToken.TokKind == TokenKind::sReg) &&
          "Expects to only be invoked starting at given keyword");
 
+  Register Register;
   switch (CurToken.TokKind) {
   default:
     llvm_unreachable("Switch for consumed token was not provided");
@@ -209,13 +207,15 @@ bool RootSignatureParser::parseRegister(Register &Register) {
     break;
   }
 
-  if (handleUIntLiteral(Register.Number))
-    return true; // propogate NumericLiteralParser error
+  auto Number = handleUIntLiteral();
+  if (!Number.has_value())
+    return std::nullopt; // propogate NumericLiteralParser error
 
-  return false;
+  Register.Number = *Number;
+  return Register;
 }
 
-bool RootSignatureParser::handleUIntLiteral(uint32_t &X) {
+std::optional<uint32_t> RootSignatureParser::handleUIntLiteral() {
   // Parse the numeric value and do semantic checks on its specification
   clang::NumericLiteralParser Literal(CurToken.NumSpelling, CurToken.TokLoc,
                                       PP.getSourceManager(), PP.getLangOpts(),
@@ -231,11 +231,10 @@ bool RootSignatureParser::handleUIntLiteral(uint32_t &X) {
     PP.getDiagnostics().Report(CurToken.TokLoc,
                                diag::err_hlsl_number_literal_overflow)
         << 0 << CurToken.NumSpelling;
-    return true;
+    return std::nullopt;
   }
 
-  X = Val.getExtValue();
-  return false;
+  return Val.getExtValue();
 }
 
 bool RootSignatureParser::peekExpectedToken(TokenKind Expected) {
