review: punt parsing of a signed integer to the parser

Author: inbelic

clang/include/clang/Basic/DiagnosticLexKinds.td

@@ -1020,7 +1020,6 @@ Error<"expected 'begin' or 'end'">;
 
 // HLSL Root Signature Lexing Errors
 let CategoryName = "Root Signature Lexical Issue" in {
-  def err_hlsl_expected_number_literal: Error<"expected numberic literal">;
   def err_hlsl_number_literal_overflow :
     Error<"integer literal '%0' is too large to be represented in a 32-bit integer type">;
   def err_hlsl_invalid_register_literal: Error<"expected unsigned integer literal as part of register">;

clang/include/clang/Parse/HLSLRootSignatureTokenKinds.def

@@ -66,6 +66,8 @@ PUNCTUATOR(r_paren, ')')
 PUNCTUATOR(comma,   ',')
 PUNCTUATOR(or,      '|')
 PUNCTUATOR(equal,   '=')
+PUNCTUATOR(plus,    '+')
+PUNCTUATOR(minus,   '-')
 
 // RootElement Keywords:
 KEYWORD(DescriptorTable)

clang/lib/Parse/ParseHLSLRootSignature.cpp

@@ -11,23 +11,9 @@ static bool IsNumberChar(char C) {
 }
 
 bool RootSignatureLexer::LexNumber(RootSignatureToken &Result) {
-  // NumericLiteralParser does not handle the sign so we will manually apply it
-  bool Negative = Buffer.front() == '-';
-  bool Signed = Negative || Buffer.front() == '+';
-  if (Signed)
-    AdvanceBuffer();
-
   // Retrieve the possible number
   StringRef NumSpelling = Buffer.take_while(IsNumberChar);
 
-  // Catch when there is a '+' or '-' specified but no literal value after.
-  // This is invalid but the NumericLiteralParser will accept this as valid.
-  if (NumSpelling.empty()) {
-    PP.getDiagnostics().Report(Result.TokLoc,
-                               diag::err_hlsl_expected_number_literal);
-    return true;
-  }
-
   // Parse the numeric value and do semantic checks on its specification
   clang::NumericLiteralParser Literal(NumSpelling, SourceLoc,
                                       PP.getSourceManager(), PP.getLangOpts(),
@@ -40,20 +26,17 @@ bool RootSignatureLexer::LexNumber(RootSignatureToken &Result) {
   assert(Literal.isIntegerLiteral() && "IsNumberChar will only support digits");
 
   // Retrieve the number value to store into the token
-  Result.Kind = TokenKind::int_literal;
 
-  // NOTE: for compabibility with DXC, we will treat any integer with '+' as an
-  // unsigned integer
-  llvm::APSInt X = llvm::APSInt(32, !Negative);
+  llvm::APSInt X = llvm::APSInt(32, true);
   if (Literal.GetIntegerValue(X)) {
     // Report that the value has overflowed
     PP.getDiagnostics().Report(Result.TokLoc,
                                diag::err_hlsl_number_literal_overflow)
-        << (unsigned)!Signed << NumSpelling;
+        << NumSpelling;
     return true;
   }
 
-  X = Negative ? -X : X;
+  Result.Kind = TokenKind::int_literal;
   Result.NumLiteral = APValue(X);
 
   AdvanceBuffer(NumSpelling.size());
@@ -83,7 +66,7 @@ bool RootSignatureLexer::LexToken(RootSignatureToken &Result) {
   }
 
   // Numeric constant
-  if (isdigit(C) || C == '-' || C == '+')
+  if (isdigit(C))
     return LexNumber(Result);
 
   // All following tokens require at least one additional character

clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp

@@ -136,32 +136,32 @@ TEST_F(ParseHLSLRootSignatureTest, ValidLexNumbersTest) {
 
   SmallVector<hlsl::RootSignatureToken> Tokens;
   SmallVector<hlsl::TokenKind> Expected = {
-      hlsl::TokenKind::int_literal,
-      hlsl::TokenKind::int_literal,
-      hlsl::TokenKind::int_literal,
+      hlsl::TokenKind::pu_minus,    hlsl::TokenKind::int_literal,
+      hlsl::TokenKind::int_literal, hlsl::TokenKind::pu_plus,
+      hlsl::TokenKind::int_literal, hlsl::TokenKind::pu_plus,
       hlsl::TokenKind::int_literal,
   };
   CheckTokens(Lexer, Tokens, Expected);
   ASSERT_TRUE(Consumer->IsSatisfied());
 
-  // Sample negative int
-  hlsl::RootSignatureToken IntToken = Tokens[0];
-  ASSERT_TRUE(IntToken.NumLiteral.getInt().isSigned());
-  ASSERT_EQ(IntToken.NumLiteral.getInt().getExtValue(), -42);
+  // // Sample negative: int component
+  hlsl::RootSignatureToken IntToken = Tokens[1];
+  ASSERT_FALSE(IntToken.NumLiteral.getInt().isSigned());
+  ASSERT_EQ(IntToken.NumLiteral.getInt().getExtValue(), 42);
 
   // Sample unsigned int
-  IntToken = Tokens[1];
+  IntToken = Tokens[2];
   ASSERT_FALSE(IntToken.NumLiteral.getInt().isSigned());
   ASSERT_EQ(IntToken.NumLiteral.getInt().getExtValue(), 42);
 
-  // Sample positive int that is treated as unsigned
-  IntToken = Tokens[2];
+  // Sample positive: int component
+  IntToken = Tokens[4];
   ASSERT_FALSE(IntToken.NumLiteral.getInt().isSigned());
   ASSERT_EQ(IntToken.NumLiteral.getInt().getExtValue(), 42);
 
   // Sample positive int that would overflow the signed representation but
   // is treated as an unsigned integer instead
-  IntToken = Tokens[3];
+  IntToken = Tokens[6];
   ASSERT_FALSE(IntToken.NumLiteral.getInt().isSigned());
   ASSERT_EQ(IntToken.NumLiteral.getInt().getExtValue(), 2147483648);
 }
@@ -174,7 +174,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidLexAllTokensTest) {
 
     b0 t43 u987 s234
 
-    (),|=
+    (),|=+-
 
     DescriptorTable
 
@@ -286,25 +286,6 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidLexOverflowedNumberTest) {
   ASSERT_TRUE(Consumer->IsSatisfied());
 }
 
-TEST_F(ParseHLSLRootSignatureTest, InvalidLexEmptyNumberTest) {
-  // This test will check that the lexing fails due to no integer being provided
-  const llvm::StringLiteral Source = R"cc(
-    -
-  )cc";
-
-  TrivialModuleLoader ModLoader;
-  auto PP = CreatePP(Source, ModLoader);
-  auto TokLoc = SourceLocation();
-
-  // Test correct diagnostic produced
-  Consumer->SetExpected(diag::err_hlsl_expected_number_literal);
-
-  hlsl::RootSignatureLexer Lexer(Source, TokLoc, *PP);
-
-  ASSERT_TRUE(Lexer.ConsumeToken());
-  ASSERT_TRUE(Consumer->IsSatisfied());
-}
-
 TEST_F(ParseHLSLRootSignatureTest, InvalidLexRegNumberTest) {
   // This test will check that the lexing fails due to no integer being provided
   const llvm::StringLiteral Source = R"cc(