sammccall marked 6 inline comments as done.
sammccall added inline comments.
================
Comment at: clang-tools-extra/clangd/XRefs.cpp:1883
+ QualType VisitIfStmt(const IfStmt *S) { return type(S->getCond()); }
+ QualType VisitCaseStmt(const CaseStmt *S) { return type(S->getLHS()); }
+ QualType VisitCXXForRangeStmt(const CXXForRangeStmt *S) {
----------------
usaxena95 wrote:
> I think this would be useful for enumerations primarily. Would it work as of
> now (would we return the enum definition loc) ?
Yes, it does work (tested manually).
================
Comment at: clang-tools-extra/clangd/XRefs.cpp:1925-1927
+ // If we targeted something function-like, prefer its return type.
+ if (auto FT = Type->getAs<FunctionType>())
+ Type = FT->getReturnType();
----------------
usaxena95 wrote:
> Can this be accommodated in `typeForNode` ?
> It would be nicer if we keep this method only for plumbing.
Agree. It's not convenient to do it exactly in typeForNode though, as we have
to wrap every return with the unwrapping logic.
I added `unwrapFindType` to do this, and that's a natural place to fix
array/pointer cases too while here.
================
Comment at: clang-tools-extra/clangd/XRefs.h:110
+///
+/// For example, given `foo(b^ar())` where bar() returns unique_ptr<Foo>,
+/// this should return the definition of class Foo.
----------------
usaxena95 wrote:
> This is sounds confusing to me about the current behaviour.
> We would return `unique_ptr<T>` here atm. Maybe just have simple `returns
> Foo` here as of now.
Whoops, you're right. I'd love to have this behavior but decided not to bite it
off in this patch.
================
Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:1805
+ "namespace m { Target tgt; } auto x = m^::tgt;",
+ "Target funcCall(); auto x = ^funcCall();",
+ "Aggregate a = { {}, ^{} };",
----------------
usaxena95 wrote:
> Can you add a lambda as well
> `"au^to x = []() -> Target {return Target{};}"`
Done. This isn't handled yet (lambdas are CXXRecordType, not FunctionType) so
added it
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D116443/new/
https://reviews.llvm.org/D116443
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
