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

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

> (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)

Yeah, that is my next step, this is an extending change of `AutoTypeLoc`.



================
Comment at: clang/lib/Parse/ParseDeclCXX.cpp:1012
+    // token doesn't have it.
+    DS.setTypeofParensRange(SourceRange(SourceLocation(), EndLoc));
     ConsumeAnnotationToken();
----------------
sammccall wrote:
> 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 :-)
Yeah, having the decltype loc + rparen loc should be enough to fix our bug, and it might be a better idea than having a partial working `getLParenLoc`.

> 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 :-)

We don't have a concrete use case of the paren range of `decltype`, so 2 is probably the best (though having a getRParenLoc API without a paired getLParenLoc feels incomplete).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116793