review: rebase onto lexer api changes

- we have changed the api of the lexer to avoid pre-allocating all the
tokens and instead to have the parser only invoke
ConsumeToken/PeekNextToken when needed

- the end of stream diagnostic is also moved to the lexer

Author: inbelic

clang/include/clang/Basic/DiagnosticParseKinds.td

@@ -1807,7 +1807,6 @@ def err_hlsl_unsupported_component : Error<"invalid component '%0' used; expecte
 def err_hlsl_packoffset_invalid_reg : Error<"invalid resource class specifier '%0' for packoffset, expected 'c'">;
 
 // HLSL Root Signature Parser Diagnostics
-def err_hlsl_rootsig_unexpected_eos : Error<"unexpected end to token stream">;
 def err_hlsl_rootsig_unexpected_token_kind : Error<"expected the %select{following|one of the following}0 token kinds '%1'">;
 def err_hlsl_rootsig_repeat_param : Error<"specified the same parameter '%0' multiple times">;
 def err_hlsl_rootsig_non_zero_flag : Error<"specified a non-zero integer as a flag">;

clang/include/clang/Parse/ParseHLSLRootSignature.h

@@ -99,8 +99,7 @@ class RootSignatureLexer {
 class RootSignatureParser {
 public:
   RootSignatureParser(SmallVector<llvm::hlsl::rootsig::RootElement> &Elements,
-                      const SmallVector<RootSignatureToken> &Tokens,
-                      DiagnosticsEngine &Diags);
+                      RootSignatureLexer &Lexer, DiagnosticsEngine &Diags);
 
   /// Iterates over the provided tokens and constructs the in-memory
   /// representations of the RootElements.
@@ -112,7 +111,7 @@ class RootSignatureParser {
 
 private:
   // Root Element helpers
-  bool ParseRootElement(bool First);
+  bool ParseRootElement();
   bool ParseDescriptorTable();
   bool ParseDescriptorTableClause();
 
@@ -147,9 +146,17 @@ class RootSignatureParser {
   ParseDescriptorRangeFlags(llvm::hlsl::rootsig::DescriptorRangeFlags *Enum);
   bool ParseShaderVisibility(llvm::hlsl::rootsig::ShaderVisibility *Enum);
 
-  /// Increment the token iterator if we have not reached the end.
+  /// Invoke the lexer to consume a token and update CurToken with the result
+  ///
   /// Return value denotes if we were already at the last token.
-  bool ConsumeNextToken();
+  ///
+  /// This is used to avoid having to constantly access the Lexer's CurToken
+  bool ConsumeNextToken() {
+    if (Lexer.ConsumeToken())
+      return true; // Report lexer err
+    CurToken = Lexer.GetCurToken();
+    return false;
+  }
 
   /// Attempt to retrieve the next token, if TokenKind is invalid then there was
   /// no next token.
@@ -185,10 +192,10 @@ class RootSignatureParser {
 
 private:
   SmallVector<llvm::hlsl::rootsig::RootElement> &Elements;
-  SmallVector<RootSignatureToken>::const_iterator CurTok;
-  SmallVector<RootSignatureToken>::const_iterator LastTok;
-
+  RootSignatureLexer &Lexer;
   DiagnosticsEngine &Diags;
+
+  RootSignatureToken CurToken;
 };
 
 } // namespace hlsl

clang/lib/Parse/ParseHLSLRootSignature.cpp

@@ -18,6 +18,9 @@ static std::string FormatTokenKinds(ArrayRef<TokenKind> Kinds) {
       Out << ", ";
     switch (Kind) {
     case TokenKind::invalid:
+      Out << "uninitialized";
+      break;
+    case TokenKind::end_of_stream:
       break;
     case TokenKind::int_literal:
       Out << "integer literal";
@@ -243,42 +246,33 @@ std::optional<RootSignatureToken> RootSignatureLexer::PeekNextToken() {
 
 // Parser Definitions
 
-RootSignatureParser::RootSignatureParser(
-    SmallVector<RootElement> &Elements,
-    const SmallVector<RootSignatureToken> &Tokens, DiagnosticsEngine &Diags)
-    : Elements(Elements), Diags(Diags) {
-  CurTok = Tokens.begin();
-  LastTok = Tokens.end();
-}
+RootSignatureParser::RootSignatureParser(SmallVector<RootElement> &Elements,
+                                         RootSignatureLexer &Lexer,
+                                         DiagnosticsEngine &Diags)
+    : Elements(Elements), Lexer(Lexer), Diags(Diags) {}
 
 bool RootSignatureParser::Parse() {
   // Handle edge-case of empty RootSignature()
-  if (CurTok == LastTok)
+  if (Lexer.EndOfBuffer())
     return false;
 
-  bool First = true;
   // Iterate as many RootElements as possible
-  while (!ParseRootElement(First)) {
-    First = false;
-    // Avoid use of ConsumeNextToken here to skip incorrect end of tokens error
-    CurTok++;
-    if (CurTok == LastTok)
+  while (!ParseRootElement()) {
+    if (Lexer.EndOfBuffer())
       return false;
-    if (EnsureExpectedToken(TokenKind::pu_comma))
+    if (ConsumeExpectedToken(TokenKind::pu_comma))
       return true;
   }
 
   return true;
 }
 
-bool RootSignatureParser::ParseRootElement(bool First) {
-  if (First && EnsureExpectedToken(TokenKind::kw_DescriptorTable))
-    return true;
-  if (!First && ConsumeExpectedToken(TokenKind::kw_DescriptorTable))
+bool RootSignatureParser::ParseRootElement() {
+  if (ConsumeExpectedToken(TokenKind::kw_DescriptorTable))
     return true;
 
   // Dispatch onto the correct parse method
-  switch (CurTok->Kind) {
+  switch (CurToken.Kind) {
   case TokenKind::kw_DescriptorTable:
     return ParseDescriptorTable();
   default:
@@ -305,8 +299,8 @@ bool RootSignatureParser::ParseDescriptorTable() {
     // Handle the visibility parameter
     if (!TryConsumeExpectedToken(TokenKind::kw_visibility)) {
       if (SeenVisibility) {
-        Diags.Report(CurTok->TokLoc, diag::err_hlsl_rootsig_repeat_param)
-            << FormatTokenKinds(CurTok->Kind);
+        Diags.Report(CurToken.TokLoc, diag::err_hlsl_rootsig_repeat_param)
+            << FormatTokenKinds(CurToken.Kind);
         return true;
       }
       SeenVisibility = true;
@@ -334,7 +328,7 @@ bool RootSignatureParser::ParseDescriptorTableClause() {
     return true;
 
   DescriptorTableClause Clause;
-  switch (CurTok->Kind) {
+  switch (CurToken.Kind) {
   case TokenKind::kw_CBV:
     Clause.Type = ClauseType::CBuffer;
     break;
@@ -419,9 +413,9 @@ bool RootSignatureParser::ParseOptionalParams(
     if (ConsumeExpectedToken(ParamKeywords))
       return true;
 
-    TokenKind ParamKind = CurTok->Kind;
+    TokenKind ParamKind = CurToken.Kind;
     if (Seen.contains(ParamKind)) {
-      Diags.Report(CurTok->TokLoc, diag::err_hlsl_rootsig_repeat_param)
+      Diags.Report(CurToken.TokLoc, diag::err_hlsl_rootsig_repeat_param)
           << FormatTokenKinds(ParamKind);
       return true;
     }
@@ -440,25 +434,25 @@ bool RootSignatureParser::ParseDescriptorRangeOffset(DescriptorRangeOffset *X) {
     return true;
 
   // Edge case for the offset enum -> static value
-  if (CurTok->Kind == TokenKind::en_DescriptorRangeOffsetAppend) {
+  if (CurToken.Kind == TokenKind::en_DescriptorRangeOffsetAppend) {
     *X = DescriptorTableOffsetAppend;
     return false;
   }
 
-  *X = DescriptorRangeOffset(CurTok->NumLiteral.getInt().getExtValue());
+  *X = DescriptorRangeOffset(CurToken.NumLiteral.getInt().getExtValue());
   return false;
 }
 
 bool RootSignatureParser::ParseUInt(uint32_t *X) {
   if (ConsumeExpectedToken(TokenKind::int_literal))
     return true;
 
-  *X = CurTok->NumLiteral.getInt().getExtValue();
+  *X = CurToken.NumLiteral.getInt().getExtValue();
   return false;
 }
 
 bool RootSignatureParser::ParseRegister(Register *Register) {
-  switch (CurTok->Kind) {
+  switch (CurToken.Kind) {
   case TokenKind::bReg:
     Register->ViewType = RegisterType::BReg;
     break;
@@ -475,7 +469,7 @@ bool RootSignatureParser::ParseRegister(Register *Register) {
     llvm_unreachable("Switch for an expected token was not provided");
   }
 
-  Register->Number = CurTok->NumLiteral.getInt().getExtValue();
+  Register->Number = CurToken.NumLiteral.getInt().getExtValue();
 
   return false;
 }
@@ -494,9 +488,9 @@ bool RootSignatureParser::ParseEnum(
     return true;
 
   // Handle the edge case when '0' is used to specify None
-  if (CurTok->Kind == TokenKind::int_literal) {
-    if (CurTok->NumLiteral.getInt() != 0) {
-      Diags.Report(CurTok->TokLoc, diag::err_hlsl_rootsig_non_zero_flag);
+  if (CurToken.Kind == TokenKind::int_literal) {
+    if (CurToken.NumLiteral.getInt() != 0) {
+      Diags.Report(CurToken.TokLoc, diag::err_hlsl_rootsig_non_zero_flag);
       return true;
     }
     // Set enum to None equivalent
@@ -506,7 +500,7 @@ bool RootSignatureParser::ParseEnum(
 
   // Effectively a switch statement on the token kinds
   for (auto EnumPair : EnumMap)
-    if (CurTok->Kind == EnumPair.first) {
+    if (CurToken.Kind == EnumPair.first) {
       *Enum = EnumPair.second;
       return false;
     }
@@ -556,25 +550,6 @@ bool RootSignatureParser::ParseShaderVisibility(ShaderVisibility *Enum) {
   return ParseEnum(EnumMap, Enum);
 }
 
-RootSignatureToken RootSignatureParser::PeekNextToken() {
-  // Create an invalid token
-  RootSignatureToken Token = RootSignatureToken(SourceLocation());
-  if (CurTok != LastTok)
-    Token = *(CurTok + 1);
-  return Token;
-}
-
-bool RootSignatureParser::ConsumeNextToken() {
-  SourceLocation EndLoc = CurTok->TokLoc;
-  CurTok++;
-  if (LastTok == CurTok) {
-    // Report unexpected end of tokens error
-    Diags.Report(EndLoc, diag::err_hlsl_rootsig_unexpected_eos);
-    return true;
-  }
-  return false;
-}
-
 // Is given token one of the expected kinds
 static bool IsExpectedToken(TokenKind Kind, ArrayRef<TokenKind> AnyExpected) {
   for (auto Expected : AnyExpected)
@@ -588,11 +563,11 @@ bool RootSignatureParser::EnsureExpectedToken(TokenKind Expected) {
 }
 
 bool RootSignatureParser::EnsureExpectedToken(ArrayRef<TokenKind> AnyExpected) {
-  if (IsExpectedToken(CurTok->Kind, AnyExpected))
+  if (IsExpectedToken(CurToken.Kind, AnyExpected))
     return false;
 
   // Report unexpected token kind error
-  Diags.Report(CurTok->TokLoc, diag::err_hlsl_rootsig_unexpected_token_kind)
+  Diags.Report(CurToken.TokLoc, diag::err_hlsl_rootsig_unexpected_token_kind)
       << (unsigned)(AnyExpected.size() != 1) << FormatTokenKinds(AnyExpected);
   return true;
 }
@@ -602,10 +577,10 @@ bool RootSignatureParser::PeekExpectedToken(TokenKind Expected) {
 }
 
 bool RootSignatureParser::PeekExpectedToken(ArrayRef<TokenKind> AnyExpected) {
-  RootSignatureToken Token = PeekNextToken();
-  if (Token.Kind == TokenKind::invalid)
+  auto Result = Lexer.PeekNextToken();
+  if (!Result)
     return true;
-  if (IsExpectedToken(Token.Kind, AnyExpected))
+  if (IsExpectedToken(Result->Kind, AnyExpected))
     return false;
   return true;
 }