[PATCH] D33135: [ASTMatchers] Add support for floatLiterals

[Peter Wu via Phabricator via cfe-commits]

Lekensteyn added a comment.

By the way, I think that `long double` is less common than long unsigned literals, so changing unsigned to uint64_t might be something more important?



================
Comment at: include/clang/ASTMatchers/Dynamic/Parser.h:25
 /// <Boolean>           := true | false
+/// <Double>            := 1.0 | 2e-3 | 3.45e67
 /// <Unsigned>          := [0-9]+
----------------
aaron.ballman wrote:
> It'd be good to list the actual grammar rather than a few examples.
I am concerned that listing a very precise grammar actually makes this less readable (see also the StringLiteral example).

If a grammar is used instead, how about this:

    <Double> := [0-9]+.[0-9]* | [0-9]+.[0-9]*[eE][-+]?[0-9]+



================
Comment at: include/clang/ASTMatchers/Dynamic/VariantValue.h:335
     unsigned Unsigned;
+    double Double;
     bool Boolean;
----------------
aaron.ballman wrote:
> This may or may not be a good idea, but do we want to put the values into an APFloat rather than a double? My concern with double is that (0) it may be subtly different if the user wants a 16- or 32-bit float explicitly, (1) it won't be able to represent long double values, or quad double.
> 
> I'm thinking this value could be passed directly from the C++ API as an APFloat, float, or double, or provided using a StringRef for the dynamic API.
(32-bit) double values are a superset of (16-bit) float values, that should be OK.
Long doubles are possible in the AST (e.g. for `0.1L`), but neither C11 nor C++14 seem to define a quad double literal type (so that should be of a lesser concern).

Reasons why I chose for double instead of APFloat:
- `strtod` is readily available and does not abort the program. By contrast, `APFloat(StringRef)` trips on assertions if the input is invalid.
- I was not sure if the APFloat class can be used in an union.


================
Comment at: lib/ASTMatchers/Dynamic/Parser.cpp:180
   /// \brief Consume an unsigned literal.
   void consumeUnsignedLiteral(TokenInfo *Result) {
+    bool isFloatingLiteral = false;
----------------
aaron.ballman wrote:
> This function should be renamed and the comment updated.
How does "consumeNumberLiteral" sound?


https://reviews.llvm.org/D33135