[PATCH] D104462: [clang][lex] Add minimizer option to pad the output to the input size

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith requested changes to this revision.
dexonsmith added a comment.
This revision now requires changes to proceed.

I'm not sure D104465 <https://reviews.llvm.org/D104465> (the motivation for this patch) is the right approach, but we can have that discussion over there.



================
Comment at: clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp:24
+                                              SourceLocation(),
+                                              /*PadOutputToInputSize=*/true);
+}
----------------
Why turn this on all the time? That adds a lot of noise below. Can you instead just turn it on for specific targeted tests?

Also, maybe I just missed it, but I don't see any tests that check the behaviour of this flag.


================
Comment at: clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp:113-115
   ASSERT_FALSE(
       minimizeSourceToDependencyDirectives("#define MACRO\n\n\n", Out));
+  EXPECT_EQ("#define MACRO", rtrim(Out));
----------------
I'm uncomfortable dropping the check that `#define MACRO\n\n\n` turns into precisely `#define MACRO\n`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104462/new/

https://reviews.llvm.org/D104462