sammccall added inline comments.
================
Comment at: clang-tools-extra/clangd/XRefs.cpp:722
+ // Fill in value with initializer. Puts evaluated version if possible.
+ if (const auto *Var = dyn_cast<VarDecl>(D)) {
----------------
do we want the initializer both here and in Definition?
I suspect we'd like to turn off one or the other when both are present.
Looking at the examples, I think it's clearer to keep a non-evaluated
initializer as part of the Definition (which is mostly as-written), and only
include Value if it's evaluated.
================
Comment at: clang-tools-extra/clangd/XRefs.cpp:733
+ else
+ Init->printPretty(ValueOS, nullptr, Policy);
+ }
----------------
why not print the non-evaluated init if it's dependent?
================
Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:920
+ {R"cpp(
+ constexpr int add(int a, int b) { return a + b; }
+ int [[b^ar]] = add(1, 2);
----------------
constexpr may not be required here, I think?
At least the docs suggest evaluation ignores language semantics like this.
================
Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:956
+ }},
+ // FIXME: We should use TypeLoc instead of Decl to extract the concrete
+ // value.
----------------
Hmm, actually I'm not sure whether this is TypeLoc vs Decl here. Rather this
should be a DeclRefExpr to the VarDecl (result) in the *expanded* CXXRecordDecl.
This suggests that maybe the isValueDependent check isn't quite right - the
initializer expression is value dependent as spelled, but in the instantiation
it's resolved. Not sure if this is fixable. What happens if you comment out the
check?
================
Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:981
+ HI.NamespaceScope = "";
+ HI.Value = "&\"1234\"[0]";
+ }},
----------------
whoa, that's... weird. But fine, I think.
(It would be nice to elide the `&[]` if the index is zero, or maybe print as
`"1234" + 2` etc)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63330/new/
https://reviews.llvm.org/D63330
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
