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