VitaNuo marked an inline comment as done.
VitaNuo added inline comments.
================
Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:79
+
+ if (isa<TemplateSpecializationType>(UnqualifiedType)) {
+ const TemplateSpecializationType *TST =
----------------
hokein wrote:
> if we just aim to support the `vector<T>().size()` case, we only need this
> part of code right?
Sure.
================
Comment at: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp:198
+ "template<typename T> void k(typename Base<T>::Nested d) { "
+ "d.^method(); }");
+ testWalk("template<typename T> struct $explicit^Base { struct Nested { void "
----------------
hokein wrote:
> Thanks for coming up these cases. IMO a more reasonable behavior would be
> that the target symbol of the reported reference is `Nested` rather than the
> outer class `Base`, but implementing it would require some dependent name
> lookup in the `Base` (in clangd we have a similar thing `HeuristicResolver`,
> but it is sophisticated).
>
> I think it is find to ignore these cases (these cases are more tricky, and
> less interesting to us) right now, this would simplify the implementation.
>
> Note that the existing behavior of accessing a member expr from a dependent
> type is that we don't report any reference on `d.^method()`, so the original
> "include base header via a base-member-access from a derived-class object"
> issue doesn't exist. We aim to improve this behavior for some critical cases.
>
> The critical case we want to support is the `vector<T>().size()`, if other
> cases come up in the furture, we could revisit it.
Thank you for the extended explanation!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D139409/new/
https://reviews.llvm.org/D139409
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
