[PATCH] D82310: Add `BoolLiteralExpression` to SyntaxTree

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 accepted this revision.
gribozavr2 marked an inline comment as done.
gribozavr2 added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:1240
+  true;
+  false;
+}
----------------
eduucaldas wrote:
> gribozavr2 wrote:
> > 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.
> [[ https://clang.llvm.org/doxygen/stdbool_8h_source.html | 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 
> 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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82310/new/

https://reviews.llvm.org/D82310