dexonsmith added a comment.
Thanks for looking at this — I'm just reviewing the test changes in the
minimizer (assuming the code change is the right thing to do).
================
Comment at:
clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp:170-173
ASSERT_FALSE(
- minimizeSourceToDependencyDirectives("#define MACRO(a \\\n"
+ minimizeSourceToDependencyDirectives("#define MACRO(a
\\\n\\\v\\\f"
" )",
Out));
----------------
Please don't modify this assertion to test `\v` and `\f`. Instead add new
assertions. I'm not sure if they belong in `DefineMultilineArgs`, or if
`DefineSpacing` might be a better spot, or maybe a new test
`DefineVerticalWhitespace`?
================
Comment at: clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp:186
Out));
- EXPECT_STREQ("#define MACRO(a)\n", Out.data());
-
----------------
Please don't remove this test coverage.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108742/new/
https://reviews.llvm.org/D108742
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
