usaxena95 accepted this revision.
usaxena95 added a comment.
This revision is now accepted and ready to land.
Thanks. LGTM.
================
Comment at: clang-tools-extra/clangd/XRefs.cpp:1869
+ QualType VisitDesignatedInitExpr(const DesignatedInitExpr *S) {
+ for (auto &D : llvm::reverse(S->designators()))
+ if (D.isFieldDesignator())
----------------
Reason for `reverse` isn't clear to me. Can you add a comment.
================
Comment at: clang-tools-extra/clangd/XRefs.cpp:1880-1882
+ QualType VisitWhileStmt(const WhileStmt *S) { return type(S->getCond());
}
+ QualType VisitDoStmt(const DoStmt *S) { return type(S->getCond()); }
+ QualType VisitIfStmt(const IfStmt *S) { return type(S->getCond()); }
----------------
Can you add tests for these as well.
================
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) {
----------------
I think this would be useful for enumerations primarily. Would it work as of
now (would we return the enum definition loc) ?
================
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();
----------------
Can this be accommodated in `typeForNode` ?
It would be nicer if we keep this method only for plumbing.
================
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.
----------------
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.
================
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 = { {}, ^{} };",
----------------
Can you add a lambda as well
`"au^to x = []() -> Target {return Target{};}"`
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
