hokein added inline comments.
================
Comment at: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp:235
+
+ int main() {
+ #ifdef Y
----------------
nit: we can get rid of the main function, it is not needed.
================
Comment at: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp:238
+ #elifdef ^X
+ #else
+ #endif
----------------
nit: this `#else` line can be removed.
================
Comment at: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp:243
+ #elifndef ^X
+ #else
+ #endif
----------------
The same, can be removed.
================
Comment at: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp:249
+
+ Inputs.Code = MainFile.code();
+ auto AST = build();
----------------
The `elifndef` and `elifdef` is a C++2b extension feature, so
`Inputs.ExtraArgs.push_back("-std=c++2b");` to get rid of the
`[-Wc++2b-extensions]` warning in the testcase.
================
Comment at: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp:248
+ SourceManager &SM = AST.sourceManager();
+ ASSERT_THAT(Recorded.MacroReferences, Not(IsEmpty()));
+
----------------
VitaNuo wrote:
> hokein wrote:
> > nit: this can be removed, as the EXPECT_THAT on line 258 covers this.
> Sorry, not sure what this comment refers to. Can it be that the line
> numbering changed due to my newer patch, and this comment does not show up in
> the correct place anymore?
yeah, the comment was attached to the old snapshot. it is the line 254 now (`
ASSERT_THAT(Recorded.MacroReferences, Not(IsEmpty()));`).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D138559/new/
https://reviews.llvm.org/D138559
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
