Add `BoolLiteralExpression` to SyntaxTree

[eduucaldas]

clang/include/clang/Tooling/Syntax/Nodes.h

268

Homogenize this comments for other literals, and possibly other expressions. To be done in future change

269

Unify Literals under a Literal class, following the grammar, [lex.literal]
To be done in a future change

clang/unittests/Tooling/Syntax/TreeTest.cpp

54

We could use this to extend the coverage of our tests from isCXX to !isC.
To be done in a future change

[gribozavr2]

clang/unittests/Tooling/Syntax/TreeTest.cpp

1243

C99 has bool literals, but the program should include stdbool.h.

I feel like it is better to make the predicate something like "hasBoolType()" and change the test to include stdbool.h.

[eduucaldas]

clang/unittests/Tooling/Syntax/TreeTest.cpp

1243

stdbool consists on macro definitions, mapping booleans to integers. true is preprocessed into 1 and false to 0 .
I don't think there is a reasonable way of getting the proper SyntaxTree from that macro expansion

Additional problem, we don't have the test infrastructure for includes, AFAIK ^^

Finally, regarding the predicates, I prefer if they relate to languages, otherwise we might create many predicates that test for exactly the same thing, e.g. we would have hasBoolType() and hasNullPtr() that ultimately do the same thing, test if Language is not C

[gribozavr2]

clang/unittests/Tooling/Syntax/TreeTest.cpp

1243

true is preprocessed into 1 and false to 0 .

I see -- yes, that would make a completely different syntax tree.

Finally, regarding the predicates, I prefer if they relate to languages, otherwise we might create many predicates that test for exactly the same thing

Generally, testing for features instead of product versions is considered better, as that leads to more future-proof code (for example, we learned this lesson again in the area of web extensions and standards). In future, Clang can start supporting a language feature in other language modes as an extension (for example, supporting _Atomic in C++ is already a thing), or the language standard itself can decide to incorporate the feature (for example, C 2035 could adopt the nullptr keyword). It is highly unlikely for complex features like templates, but may reasonably happen for simpler features.

[eduucaldas]

isC -> hasBoolType

clang/unittests/Tooling/Syntax/TreeTest.cpp

1243

Generally, testing for features instead of product versions is considered better, as that leads to more future-proof code

I totally agree with all you said. My worry was more regarding the fact that the implementation of hasBoolType would merely check for the language anyways. So if, following what you said, true and false really became a feature of C35 then the code would still be broken.

But then I guess it would be easier to fix hasBoolType than finding all the places where we used just isC because booleans are not in C.

Specially if all these predicates were put in clang/Testing/CommandLineArgs.h or something similar where more people would be seeing it.

Which brings the question shouldn't we move these predicates up?

[gribozavr2]

clang/unittests/Tooling/Syntax/TreeTest.cpp

1243

Sorry for miscommunication -- in this specific we could go either way, since the C bool feature is already implemented to have a very different AST than in C++, and given C's stability principles, it is unlikely to change.

I am trying to move this testing infra to a common place in https://reviews.llvm.org/D82179.

clang/include/clang/Tooling/Syntax/Nodes.h

enum class NodeKind : uint16_t {
 
   // Expressions.
   UnknownExpression,
   PrefixUnaryOperatorExpression,
   PostfixUnaryOperatorExpression,
   BinaryOperatorExpression,
   CxxNullPtrExpression,
   IntegerLiteralExpression,
+  BoolLiteralExpression,
   IdExpression,
 
   // Statements.
   UnknownStatement,
   DeclarationStatement,
   EmptyStatement,
   SwitchStatement,
   CaseStatement,

 public:
   IntegerLiteralExpression() : Expression(NodeKind::IntegerLiteralExpression) {}
   static bool classof(const Node *N) {
     return N->kind() == NodeKind::IntegerLiteralExpression;
   }
   syntax::Leaf *literalToken();
 };
 
+/// Expression for boolean literals. C++ [lex.bool]
+class BoolLiteralExpression final : public Expression {
+public:
+  BoolLiteralExpression() : Expression(NodeKind::BoolLiteralExpression) {}
+  static bool classof(const Node *N) {
+    return N->kind() == NodeKind::BoolLiteralExpression;
+  }
+  syntax::Leaf *literalToken();
+};
+
 /// An abstract class for prefix and postfix unary operators.
 class UnaryOperatorExpression : public Expression {
 public:
   UnaryOperatorExpression(NodeKind K) : Expression(K) {}
   static bool classof(const Node *N) {
     return N->kind() == NodeKind::PrefixUnaryOperatorExpression ||
            N->kind() == NodeKind::PostfixUnaryOperatorExpression;
   }

clang/lib/Tooling/Syntax/BuildTree.cpp

public:
 
   bool WalkUpFromIntegerLiteral(IntegerLiteral *S) {
     Builder.markChildToken(S->getLocation(), syntax::NodeRole::LiteralToken);
     Builder.foldNode(Builder.getExprRange(S),
                      new (allocator()) syntax::IntegerLiteralExpression, S);
     return true;
   }
 
+  bool WalkUpFromCXXBoolLiteralExpr(CXXBoolLiteralExpr *S) {
+    Builder.markChildToken(S->getLocation(), syntax::NodeRole::LiteralToken);
+    Builder.foldNode(Builder.getExprRange(S),
+                     new (allocator()) syntax::BoolLiteralExpression, S);
+    return true;
+  }
+
   bool WalkUpFromCXXNullPtrLiteralExpr(CXXNullPtrLiteralExpr *S) {
     Builder.markChildToken(S->getLocation(), syntax::NodeRole::LiteralToken);
     Builder.foldNode(Builder.getExprRange(S),
                      new (allocator()) syntax::CxxNullPtrExpression, S);
     return true;
   }
 
   bool WalkUpFromUnaryOperator(UnaryOperator *S) {

clang/lib/Tooling/Syntax/Nodes.cpp

llvm::raw_ostream &syntax::operator<<(llvm::raw_ostream &OS, NodeKind K) {
   case NodeKind::TranslationUnit:
     return OS << "TranslationUnit";
   case NodeKind::UnknownExpression:
     return OS << "UnknownExpression";
   case NodeKind::CxxNullPtrExpression:
     return OS << "CxxNullPtrExpression";
   case NodeKind::IntegerLiteralExpression:
     return OS << "IntegerLiteralExpression";
+  case NodeKind::BoolLiteralExpression:
+    return OS << "BoolLiteralExpression";
   case NodeKind::PrefixUnaryOperatorExpression:
     return OS << "PrefixUnaryOperatorExpression";
   case NodeKind::PostfixUnaryOperatorExpression:
     return OS << "PostfixUnaryOperatorExpression";
   case NodeKind::BinaryOperatorExpression:
     return OS << "BinaryOperatorExpression";
   case NodeKind::UnqualifiedId:
     return OS << "UnqualifiedId";
return llvm::cast_or_null<syntax::UnqualifiedId>(
       findChild(syntax::NodeRole::IdExpression_id));
 }
 
 syntax::Leaf *syntax::IntegerLiteralExpression::literalToken() {
   return llvm::cast_or_null<syntax::Leaf>(
       findChild(syntax::NodeRole::LiteralToken));
 }
 
+syntax::Leaf *syntax::BoolLiteralExpression::literalToken() {
+  return llvm::cast_or_null<syntax::Leaf>(
+      findChild(syntax::NodeRole::LiteralToken));
+}
+
 syntax::Leaf *syntax::CxxNullPtrExpression::nullPtrKeyword() {
   return llvm::cast_or_null<syntax::Leaf>(
       findChild(syntax::NodeRole::LiteralToken));
 }
 
 syntax::Expression *syntax::BinaryOperatorExpression::lhs() {
   return llvm::cast_or_null<syntax::Expression>(
       findChild(syntax::NodeRole::BinaryOperatorExpression_leftHandSide));

clang/unittests/Tooling/Syntax/TreeTest.cpp

static llvm::ArrayRef<syntax::Token> tokens(syntax::Node *N) {
   return llvm::makeArrayRef(T->firstLeaf()->token(),
                             T->lastLeaf()->token() + 1);
 }
 
 struct TestClangConfig {
   TestLanguage Language;
   std::string Target;
 
   bool isC99OrLater() const { return Language == Lang_C99; }
 
   bool isCXX() const {
     return Language == Lang_CXX03 || Language == Lang_CXX11 ||
            Language == Lang_CXX14 || Language == Lang_CXX17 ||
            Language == Lang_CXX20;
   }
 
   bool isCXX11OrLater() const {
     return Language == Lang_CXX11 || Language == Lang_CXX14 ||
            Language == Lang_CXX17 || Language == Lang_CXX20;
   }
 
   bool isCXX14OrLater() const {
     return Language == Lang_CXX14 || Language == Lang_CXX17 ||
            Language == Lang_CXX20;
   }
 
+  bool hasBoolType() const {
+    return Language == Lang_C89 || Language == Lang_C99;
+  }
+
   bool supportsCXXDynamicExceptionSpecification() const {
     return Language == Lang_CXX03 || Language == Lang_CXX11 ||
            Language == Lang_CXX14;
   }
 
   bool hasDelayedTemplateParsing() const {
     return Target == "x86_64-pc-win32-msvc";
   }
`-CompoundStatement
     |-ExpressionStatement
     | |-CxxNullPtrExpression
     | | `-nullptr
     | `-;
     `-}
 )txt"));
 }
 
+TEST_P(SyntaxTreeTest, BoolLiteral) {
+  if (GetParam().hasBoolType()) {
+    return;
+  }
+  EXPECT_TRUE(treeDumpEqual(
+      R"cpp(
+void test() {
+  true;
+  false;
+}
+)cpp",
+      R"txt(
+*: TranslationUnit
+`-SimpleDeclaration
+  |-void
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
+  `-CompoundStatement
+    |-{
+    |-ExpressionStatement
+    | |-BoolLiteralExpression
+    | | `-true
+    | `-;
+    |-ExpressionStatement
+    | |-BoolLiteralExpression
+    | | `-false
+    | `-;
+    `-}
+)txt"));
+}
+
 TEST_P(SyntaxTreeTest, IntegerLiteral) {
   EXPECT_TRUE(treeDumpEqual(
       R"cpp(
 void test() {
   12;
   12u;
   12l;
   12ul;
`-SimpleDeclaration
   |   | |-int
   |   | `-SimpleDeclarator
   |   |   `-a
   |   `-)
   `-CompoundStatement
     |-{
     |-ExpressionStatement
     | |-BinaryOperatorExpression
-    | | |-UnknownExpression
+    | | |-BoolLiteralExpression
     | | | `-true
     | | |-||
-    | | `-UnknownExpression
+    | | `-BoolLiteralExpression
     | |   `-false
     | `-;
     |-ExpressionStatement
     | |-BinaryOperatorExpression
-    | | |-UnknownExpression
+    | | |-BoolLiteralExpression
     | | | `-true
     | | |-or
-    | | `-UnknownExpression
+    | | `-BoolLiteralExpression
     | |   `-false
     | `-;
     |-ExpressionStatement
     | |-BinaryOperatorExpression
     | | |-IntegerLiteralExpression
     | | | `-1
     | | |-bitand
     | | `-IntegerLiteralExpression

 static_assert(true, "message");
 static_assert(true);
 )cpp",
       R"txt(
 *: TranslationUnit
 |-StaticAssertDeclaration
 | |-static_assert
 | |-(
-| |-UnknownExpression
+| |-BoolLiteralExpression
 | | `-true
 | |-,
 | |-UnknownExpression
 | | `-"message"
 | |-)
 | `-;
 `-StaticAssertDeclaration
   |-static_assert
   |-(
-  |-UnknownExpression
+  |-BoolLiteralExpression
   | `-true
   |-)
   `-;
 )txt"));
 }
 
 TEST_P(SyntaxTreeTest, ExternC) {
   if (!GetParam().isCXX()) {
`-SimpleDeclaration
   |-int
   |-SimpleDeclarator
   | |-b
   | `-ParametersAndQualifiers
   |   |-(
   |   |-)
   |   |-noexcept
   |   |-(
-  |   |-UnknownExpression
+  |   |-BoolLiteralExpression
   |   | `-true
   |   `-)
   `-;
 )txt"));
 }
 
 TEST_P(SyntaxTreeTest, DeclaratorsInParentheses) {
   EXPECT_TRUE(treeDumpEqual(