[PATCH] D104601: [Preprocessor] Implement -fnormalize-whitespace.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I'm still mulling over the feature, but I have some nits with the patch while I was exploring it. I share the concerns raised by @dblaikie and would add that it's very common for implementers to ask developers to run their code through `-E` mode to submit preprocessed output in order to reproduce a customer issue with the compiler, and I worry that uses of this flag will have unintended consequences in that scenario. However, I don't yet have a concrete "this code demonstrates the problem I'm worried about" example to discuss, so this worry may be unfounded. The "very long line" example mentioned by @dblaikie is sort of along these lines (sorry for the bad pun).



================
Comment at: clang/docs/ClangCommandLineReference.rst:2480
+
+Ignore the whitespace from the input file when emitting preprocessor output. It will only contain whitespace when necessary, e.g. to keep two minus signs from merging into to an increment operator. Useful with the -P option to normlize whitespace such that two files with only formatted changes are equal.
+
----------------
You might want to wrap this to 80-col limits.


================
Comment at: clang/lib/Frontend/PrintPreprocessedOutput.cpp:135
+  /// Ensure that the output stream position is at the beginning of a new line
+  /// and inserts one if it does not.It is intended to ensure that directives
+  /// inserted by the directives not from the input source (such as #line) are
----------------



================
Comment at: clang/lib/Frontend/PrintPreprocessedOutput.cpp:137
+  /// inserted by the directives not from the input source (such as #line) are
+  /// in the first column. To insert newlines the represent the input, use
+  /// MoveToLine(/*...*/, /*RequireStartOfLine=*/true).
----------------



================
Comment at: clang/lib/Frontend/PrintPreprocessedOutput.cpp:174
+  ///                        on being on the same line, such as directives.
+  void HandleWhitespaceBeforeTok(Token &Tok, bool RequireSpace,
+                                 bool RequireSameLine);
----------------
Can `Tok` be `const Token &` instead?


================
Comment at: clang/lib/Frontend/PrintPreprocessedOutput.cpp:181
+  /// In these cases the next output will be the first column on the line and
+  /// make it possible to insert indention. The newline is was inserted
+  /// implicitly when at the beginning of the file.
----------------



================
Comment at: clang/lib/Frontend/PrintPreprocessedOutput.cpp:635
+                                                         bool RequireSameLine) {
+  // These tokens are not expand to anything and don't need whitespace before
+  // them.
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104601