[PATCH] D116793: [AST] Add more source information for DecltypeTypeLoc.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Thank you for digging into this!

(BTW the parsing part of this is probably adjacent to a fix for https://github.com/clangd/clangd/issues/121, AutoType also doesn't have enough locations for `decltype(auto)`. But definitely not something to touch in this patch)



================
Comment at: clang/lib/Parse/ParseDeclCXX.cpp:1012
+    // token doesn't have it.
+    DS.setTypeofParensRange(SourceRange(SourceLocation(), EndLoc));
     ConsumeAnnotationToken();
----------------
hokein wrote:
> This is unfortunate for a common case `decltype(expression) a;`, the `ParseDecltypeSpecifier` is called twice to parse the decltype specifier: 
> 
> - first time, we parse the raw decltype token, and we annotate that (the `DS` was created locally, and then was thrown away)
> - second time, we parsed the annotated decltype, and set another `DS`, at this point, we lost the LParen information :(
> 
> One way to fix that is to add a new field `SourceLocation` in `Token` class (there is 4-bytes padding size left, so it won't increase its size), but I'm a little worried, it seems like an overkill to just fix this issue. 
> 
> 
What do you think about only storing the kw loc + endloc then?
This is enough to compute the correct sourcerange, which was the original bug we hit.

Our options are:
 - do the work to always compute it: nice to have but it does seem hard/intrusive
 - don't store/expose it
 - expose it but have it be unreliable

I don't know if 3 is better than 2 (without a use case it's hard to judge), maybe it just encourages writing buggy code. And if nothing else, 2 is smaller :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116793