[PATCH] D25363: Store a SourceRange for multi-token builtin types

[Piotr Padlewski via cfe-commits]

Prazek added a comment.

Thanks for the patch! I think some unit test should be added.

Do you also handle cases like

  unsigned long long
  unsigned volatile long const long static

etc.
The problem here is that the whole type like "unsigned long long" could be in other tokens. 
I talked with Richard Smith while ago and one of the solutions proposed was to have "isValid()" for source range
returning false for cases like this

  unsigned const long



================
Comment at: include/clang/AST/TypeLoc.h:529
+  void setBuiltinLocStart(SourceLocation Loc) {
+    if (getLocalData()->BuiltinRange.getEnd().isValid()) {
+      getLocalData()->BuiltinRange.setBegin(Loc);
----------------
Can you add a comment here? It might be because I don't know the AST API, but it is not clear for me what does it do.


================
Comment at: lib/Sema/DeclSpec.cpp:621
   TypeSpecWidth = W;
+  // Remember location of the last 'long'
+  TSTLoc = Loc;
----------------
What about cases like 
  unsigned int
  unsigned long

This is not only about long.


https://reviews.llvm.org/D25363