hokein added inline comments.
================
Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:67
+ bool VisitCXXDependentScopeMemberExpr(CXXDependentScopeMemberExpr *E) {
+ QualType QT = E->getBaseType().getCanonicalType();
----------------
Rebase the current patch. Move the implementation in the `resolveType` method,
then this method will just pass the `BaseType` to `resolveType`.
================
Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:79
+
+ if (isa<TemplateSpecializationType>(UnqualifiedType)) {
+ const TemplateSpecializationType *TST =
----------------
if we just aim to support the `vector<T>().size()` case, we only need this part
of code right?
================
Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:81
+ const TemplateSpecializationType *TST =
+ cast<TemplateSpecializationType>(UnqualifiedType);
+ TemplateDecl *TD = TST->getTemplateName().getAsTemplateDecl();
----------------
You can simplify it in a single if statement:
`if (const TemplateSpecializationType *TS =
Type->getAs<TemplateSpecializationType>()) `
================
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 "
----------------
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.
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
