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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
