review prototype: allow for more context to ConsumeNextToken error

Author: inbelic

clang/include/clang/Basic/DiagnosticParseKinds.td

@@ -1819,7 +1819,7 @@ 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_token_kind : Error<"got %1 but expected %select{|one of}0 the following token kind%select{|s}0: %2">;
+def err_hlsl_expected : Error<"expected %0 as %1">;
 def err_hlsl_number_literal_overflow : Error<"integer literal is too large to be represented as a 32-bit %select{signed |}0 integer type">;
 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

@@ -114,8 +114,12 @@ class RootSignatureParser {
   /// kind.
   ///
   /// Returns true if there was an error reported.
-  bool ConsumeExpectedToken(TokenKind Expected);
-  bool ConsumeExpectedToken(ArrayRef<TokenKind> AnyExpected);
+  bool ConsumeExpectedToken(TokenKind Expected,
+                            unsigned DiagID = diag::err_expected,
+                            StringRef DiagMsg = "");
+  bool ConsumeExpectedToken(ArrayRef<TokenKind> AnyExpected,
+                            unsigned DiagID = diag::err_expected,
+                            StringRef DiagMsg = "");
 
   /// Peek if the next token is of the expected kind and if it is then consume
   /// it.

clang/lib/Parse/ParseHLSLRootSignature.cpp

@@ -53,15 +53,17 @@ bool RootSignatureParser::Parse() {
   while (!ParseRootElement()) {
     if (Lexer.EndOfBuffer())
       return false;
-    if (ConsumeExpectedToken(TokenKind::pu_comma))
+    if (ConsumeExpectedToken(TokenKind::pu_comma, diag::err_expected_either,
+                             "end of root signature string"))
       return true;
   }
 
   return true;
 }
 
 bool RootSignatureParser::ParseRootElement() {
-  if (ConsumeExpectedToken(TokenKind::kw_DescriptorTable))
+  if (ConsumeExpectedToken(TokenKind::kw_DescriptorTable,
+                           diag::err_hlsl_expected, "root element"))
     return true;
 
   // Dispatch onto the correct parse method
@@ -77,7 +79,8 @@ bool RootSignatureParser::ParseRootElement() {
 bool RootSignatureParser::ParseDescriptorTable() {
   DescriptorTable Table;
 
-  if (ConsumeExpectedToken(TokenKind::pu_l_paren))
+  if (ConsumeExpectedToken(TokenKind::pu_l_paren, diag::err_expected_after,
+                           "DescriptorTable"))
     return true;
 
   // Empty case:
@@ -108,7 +111,8 @@ bool RootSignatureParser::ParseDescriptorTable() {
     Table.NumClauses++;
   } while (TryConsumeExpectedToken(TokenKind::pu_comma));
 
-  if (ConsumeExpectedToken(TokenKind::pu_r_paren))
+  if (ConsumeExpectedToken(TokenKind::pu_r_paren, diag::err_expected_after,
+                           "descriptor table clauses"))
     return true;
 
   Elements.push_back(Table);
@@ -117,7 +121,8 @@ bool RootSignatureParser::ParseDescriptorTable() {
 
 bool RootSignatureParser::ParseDescriptorTableClause() {
   if (ConsumeExpectedToken({TokenKind::kw_CBV, TokenKind::kw_SRV,
-                            TokenKind::kw_UAV, TokenKind::kw_Sampler}))
+                            TokenKind::kw_UAV, TokenKind::kw_Sampler},
+                           diag::err_hlsl_expected, "descriptor table clause"))
     return true;
 
   DescriptorTableClause Clause;
@@ -139,7 +144,8 @@ bool RootSignatureParser::ParseDescriptorTableClause() {
   }
   Clause.SetDefaultFlags();
 
-  if (ConsumeExpectedToken(TokenKind::pu_l_paren))
+  if (ConsumeExpectedToken(TokenKind::pu_l_paren, diag::err_expected_after,
+                           FormatTokenKinds({CurToken.Kind})))
     return true;
 
   // Consume mandatory Register paramater
@@ -156,7 +162,8 @@ bool RootSignatureParser::ParseDescriptorTableClause() {
   if (ParseOptionalParams({RefMap}))
     return true;
 
-  if (ConsumeExpectedToken(TokenKind::pu_r_paren))
+  if (ConsumeExpectedToken(TokenKind::pu_r_paren, diag::err_expected_after,
+                           "clause parameters"))
     return true;
 
   Elements.push_back(Clause);
@@ -168,7 +175,8 @@ template <class... Ts> struct ParseMethods : Ts... { using Ts::operator()...; };
 template <class... Ts> ParseMethods(Ts...) -> ParseMethods<Ts...>;
 
 bool RootSignatureParser::ParseParam(ParamType Ref) {
-  if (ConsumeExpectedToken(TokenKind::pu_equal))
+  if (ConsumeExpectedToken(TokenKind::pu_equal, diag::err_expected_after,
+                           FormatTokenKinds(CurToken.Kind)))
     return true;
 
   bool Error;
@@ -198,7 +206,8 @@ bool RootSignatureParser::ParseOptionalParams(
   llvm::SmallDenseSet<TokenKind> Seen;
 
   while (TryConsumeExpectedToken(TokenKind::pu_comma)) {
-    if (ConsumeExpectedToken(ParamKeywords))
+    if (ConsumeExpectedToken(ParamKeywords, diag::err_hlsl_expected,
+                             "optional parameter"))
       return true;
 
     TokenKind ParamKind = CurToken.Kind;
@@ -241,7 +250,8 @@ bool RootSignatureParser::HandleUIntLiteral(uint32_t &X) {
 
 bool RootSignatureParser::ParseRegister(Register *Register) {
   if (ConsumeExpectedToken(
-          {TokenKind::bReg, TokenKind::tReg, TokenKind::uReg, TokenKind::sReg}))
+          {TokenKind::bReg, TokenKind::tReg, TokenKind::uReg, TokenKind::sReg},
+          diag::err_hlsl_expected, "a register"))
     return true;
 
   switch (CurToken.Kind) {
@@ -270,7 +280,8 @@ bool RootSignatureParser::ParseRegister(Register *Register) {
 bool RootSignatureParser::ParseUInt(uint32_t *X) {
   // Treat a postively signed integer as though it is unsigned to match DXC
   TryConsumeExpectedToken(TokenKind::pu_plus);
-  if (ConsumeExpectedToken(TokenKind::int_literal))
+  if (ConsumeExpectedToken(TokenKind::int_literal, diag::err_hlsl_expected,
+                           "unsigned integer"))
     return true;
 
   if (HandleUIntLiteral(*X))
@@ -281,7 +292,8 @@ bool RootSignatureParser::ParseUInt(uint32_t *X) {
 
 bool RootSignatureParser::ParseDescriptorRangeOffset(DescriptorRangeOffset *X) {
   if (ConsumeExpectedToken(
-          {TokenKind::int_literal, TokenKind::en_DescriptorRangeOffsetAppend}))
+          {TokenKind::int_literal, TokenKind::en_DescriptorRangeOffsetAppend},
+          diag::err_hlsl_expected, "descriptor range offset"))
     return true;
 
   // Edge case for the offset enum -> static value
@@ -307,7 +319,8 @@ bool RootSignatureParser::ParseEnum(
     EnumToks.push_back(EnumPair.first);
 
   // If invoked we expect to have an enum
-  if (ConsumeExpectedToken(EnumToks))
+  if (ConsumeExpectedToken(EnumToks, diag::err_hlsl_expected,
+                           "parameter value"))
     return true;
 
   // Handle the edge case when '0' is used to specify None
@@ -391,20 +404,33 @@ bool RootSignatureParser::PeekExpectedToken(ArrayRef<TokenKind> AnyExpected) {
   return IsExpectedToken(Result.Kind, AnyExpected);
 }
 
-bool RootSignatureParser::ConsumeExpectedToken(TokenKind Expected) {
-  return ConsumeExpectedToken(ArrayRef{Expected});
+bool RootSignatureParser::ConsumeExpectedToken(TokenKind Expected,
+                                               unsigned DiagID,
+                                               StringRef DiagMsg) {
+  return ConsumeExpectedToken(ArrayRef{Expected}, DiagID, DiagMsg);
 }
 
-bool RootSignatureParser::ConsumeExpectedToken(
-    ArrayRef<TokenKind> AnyExpected) {
-  ConsumeNextToken();
-  if (IsExpectedToken(CurToken.Kind, AnyExpected))
+bool RootSignatureParser::ConsumeExpectedToken(ArrayRef<TokenKind> AnyExpected,
+                                               unsigned DiagID,
+                                               StringRef DiagMsg) {
+  if (TryConsumeExpectedToken(AnyExpected))
     return false;
 
   // Report unexpected token kind error
-  Diags().Report(CurToken.TokLoc, diag::err_hlsl_rootsig_unexpected_token_kind)
-      << (unsigned)(AnyExpected.size() != 1)
-      << FormatTokenKinds({CurToken.Kind}) << FormatTokenKinds(AnyExpected);
+  DiagnosticBuilder DB = Diags().Report(CurToken.TokLoc, DiagID);
+  switch (DiagID) {
+  case diag::err_expected:
+    DB << FormatTokenKinds(AnyExpected);
+    break;
+  case diag::err_hlsl_expected:
+  case diag::err_expected_either:
+  case diag::err_expected_after:
+    DB << FormatTokenKinds(AnyExpected) << DiagMsg;
+    break;
+  default:
+    DB << DiagMsg;
+    break;
+  }
   return true;
 }
 

clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp

@@ -238,7 +238,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidParseUnexpectedTokenTest) {
   hlsl::RootSignatureParser Parser(Elements, Lexer, *PP);
 
   // Test correct diagnostic produced
-  Consumer->SetExpected(diag::err_hlsl_rootsig_unexpected_token_kind);
+  Consumer->SetExpected(diag::err_expected_either);
   ASSERT_TRUE(Parser.Parse());
 
   ASSERT_TRUE(Consumer->IsSatisfied());
@@ -258,7 +258,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidParseInvalidTokenTest) {
   hlsl::RootSignatureParser Parser(Elements, Lexer, *PP);
 
   // Test correct diagnostic produced - invalid token
-  Consumer->SetExpected(diag::err_hlsl_rootsig_unexpected_token_kind);
+  Consumer->SetExpected(diag::err_hlsl_expected);
   ASSERT_TRUE(Parser.Parse());
 
   ASSERT_TRUE(Consumer->IsSatisfied());
@@ -278,7 +278,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidParseUnexpectedEndOfStreamTest) {
   hlsl::RootSignatureParser Parser(Elements, Lexer, *PP);
 
   // Test correct diagnostic produced - end of stream
-  Consumer->SetExpected(diag::err_hlsl_rootsig_unexpected_token_kind);
+  Consumer->SetExpected(diag::err_hlsl_expected);
   ASSERT_TRUE(Parser.Parse());
 
   ASSERT_TRUE(Consumer->IsSatisfied());