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

[Malcolm Parsons via cfe-commits]

malcolm.parsons added a comment.

In https://reviews.llvm.org/D25363#565371, @Prazek wrote:

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


Are there any existing unit tests for TypeLoc that I can add to?

> Do you also handle cases like
> 
>   unsigned long long

Yes - see tests in https://reviews.llvm.org/D25316.

> 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

I see.  I hope that's a rare case that I can ignore for now.



================
Comment at: lib/Sema/DeclSpec.cpp:621
   TypeSpecWidth = W;
+  // Remember location of the last 'long'
+  TSTLoc = Loc;
----------------
Prazek wrote:
> What about cases like 
>   unsigned int
>   unsigned long
> 
> This is not only about long.
for `unsigned int`, `DeclSpec::SetTypeSpecSign()` sets `TSSLoc` to the location of `unsigned` and `DeclSpec::SetTypeSpecType()` sets `TSTLoc` to the location of `int` so `VisitBuiltinTypeLoc()` already has the whole range available.

for `unsigned long`, `DeclSpec::SetTypeSpecSign()` sets `TSSLoc` to the location of `unsigned` and `DeclSpec::SetTypeSpecWidth()` sets `TSWLoc` to the location of `long` so `VisitBuiltinTypeLoc()` already has the whole range available.

The sign and type parts can't appear more than once, but the width part can.
I'm treating the final width part as being the type part.
This works for `long long` and `long long int`, but it looks like it would have issues with `int long long`.
Maybe TSWLoc should be replaced by a range.


https://reviews.llvm.org/D25363