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

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added a comment.

In D104601#2831746 <https://reviews.llvm.org/D104601#2831746>, @dblaikie wrote:

> This is probably more @aaron.ballman 's wheelhouse, but for my money this seems pretty problematic - will make quoted text in compiler diagnostics weird/difficult to read, etc.

It is not meant for human readability but automatic processing. The linked ccache patch either re-runs the compiler with on the original source file if there is any diagnostic output (run_second_cpp=false) or(inclusive) uses the preprocessor output only to determine whether there was a significant change in the first place (run_second_cpp=true, default).

To discourage use by humans, we could make the flag available only behind `-Xclang`, but I'd expect people to only use the flag if they have a need for it anyway. Compiler diagnostics invoked on `-E -P` output is already not very useful; `-fnormalize-whitespace` would potentially cause the entire source be on a single line. If this is a show-stopper, I could try making the diagnostic's quoted text print only an excerpt of a line if it becomes too long.

> Do you have a particular use case that has exceptionally frequent whitespace-only changes?

Every edit of a (multi-line) comment and running of clang-format. The former makes me hesitant to improve comments in central header files because I know it means that I have to rebuild the entire code base; it should not be a reason to not improve documentation. The latter is best-practice before any commit. Again, if it includes a central header file, it means rebuilding significant portions.

ccache uses preprocessor mode by default while it also supports strict binary comparison ("direct mode") and I assume there is a reason for this. However, every non-intra-line change causes the preprocessor output to change due to the line number in the #line directive changing, making the preprocessor mode much less effective than it could be. Very few NF changes take place within a single line only.

Unlike gcc, clang's `-E -P` output still includes empty lines (unless there are more than 8 consecutive empty lines). That is, there is hardly a benefit to use `-P` output in ccache with clang.

> Or is this something you think would be generally applicable? How applicable, do you think? Do you have some data showing a % of rebuilds that would be avoided in practice/based on real code bases, etc?

If it supports my case, I can work with builds using ccache for a while: one configured using -fnormalize-whitespace, the other without, and compare ccache statistics. I have used it already, but never comparatively over the same time frame. However, I think the above argument is already pretty good.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104601