hokein added inline comments.
================
Comment at: clang-tools-extra/include-cleaner/lib/AnalysisInternal.h:33
+/// Indicates the relation between the reference and the target.
+enum class RefType {
+ /// Target is explicit from the reference, e.g. function call.
----------------
I'm wondering what's our plan of supporting policy (different coding-style may
have different decisions about which includes are used)?
IIUC, the RefType is part of the picture, providing fine-grained information
about each reference, and the caller can make their decisions based on it?
Thinking about the `Implicit` type, I think cases like non-spelled constructor
call, implicit conversion calls (maybe more?) fall into this type, if we
support `Policy.Constructor`, and `Policy.Conversion`, how do we distinguish
with these cases? We can probably do some ad-hoc checks on the `TargetDecl`,
but I'm not sure that the tuple `<SourceLocation, TargetDecl, Ref>` will
provide enough information to implement different policy .
================
Comment at: clang-tools-extra/include-cleaner/lib/AnalysisInternal.h:36
+ Explicit,
+ /// Target isn't spelled, e.g. default constructor call.
+ Implicit,
----------------
nit: `default constructor call` is vague -- `S s = S();` it is a default
constructor call, but it is not implicit, maybe more specific by giving a
simple example, like default constructor call in `S s;`?
================
Comment at: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp:12
#include "gtest/gtest.h"
+#include <map>
+#include <unordered_map>
----------------
nit: it seems unused.
================
Comment at: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp:177
TEST(WalkAST, ConstructExprs) {
- testWalk("struct ^S {};", "S ^t;");
- testWalk("struct S { ^S(int); };", "S ^t(42);");
+ testWalk("struct $implicit^S {};", "S ^t;");
+ testWalk("struct S { $explicit^S(int); };", "S ^t(42);");
----------------
maybe add this following case, I think it will mark the the constructor `S()`
implicit.
```
struct S { S(); };
S ^t;
```
================
Comment at: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp:178
+ testWalk("struct $implicit^S {};", "S ^t;");
+ testWalk("struct S { $explicit^S(int); };", "S ^t(42);");
}
----------------
nit: also add an implicit case
```
S t = 42;
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135859/new/
https://reviews.llvm.org/D135859
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
