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

Meinersbur added a comment. In D104601#2831746 , @dblaikie wrote: > but for my money this seems pretty problematic - will make quoted text in > compiler diagnostics weird/difficult to read, etc. FWIW, clang has a line length limit of 4096

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

Meinersbur updated this revision to Diff 360278. Meinersbur added a comment. - Rename -felide-unnecessary-whitespace to -fminimize-whitespace - Add -fminimize-whitespace only vlid to be used to docs - Adjust some code comments - Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

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

aaron.ballman added a comment. In D104601#2886412 , @Meinersbur wrote: > In D104601#2877855 , @aaron.ballman > wrote: > >> I don't think they'd *add* it, I worry it's already used by their build >> system for

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

Meinersbur updated this revision to Diff 359666. Meinersbur marked an inline comment as done. Meinersbur added a comment. - Rename -fnormalize-whitespace to -felide-unnecessary-whitespace - error when used without -E - Reabse Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

Meinersbur marked an inline comment as done. Meinersbur added a comment. In D104601#2877855 , @aaron.ballman wrote: > In D104601#2848366 , @Meinersbur > wrote: > >> In D104601#2847400

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

aaron.ballman added a comment. In D104601#2848366 , @Meinersbur wrote: > In D104601#2847400 , @aaron.ballman > wrote: > >> ... would add that it's very common for implementers to ask developers to >> run their

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

Meinersbur added a comment. ping It would be nice if we could get this into clang 13 since the ccache part detects support for `-fnormalize-whitespace` when detecting clang 13+. Probing the compiler for whether it supports `-fnormalize-whitespace` on every run would be prohibitively

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

Meinersbur added a comment. ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104601/new/ https://reviews.llvm.org/D104601 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

Meinersbur added a comment. In D104601#2847400 , @aaron.ballman wrote: > ... 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

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

Meinersbur updated this revision to Diff 355350. Meinersbur marked 6 inline comments as done. Meinersbur added a comment. - Address review by @aaron.ballman Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104601/new/ https://reviews.llvm.org/D104601

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

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

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

Meinersbur added a comment. In D104601#2842686 , @dblaikie wrote: > One other thing: This wouldn't be usable when using debug info, presumably, > because it'd refer to the wrong lines, etc. This is considered in the ccache patch

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

dblaikie added a comment. > Admittedly, I focused on changes (of Clang and Polly) like refactoring, > improving comments, minimize difference to upstream (clang-)formatting etc. > during the testing. Yeah, I was more curious about general purpose/average changes, but anyway. > In

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

Meinersbur added a comment. During a trial phase while compiling everything twice with ccache I got the following results. Only unify_mode:: $ ccache -d . -s cache directory . primary config ./ccache.conf secondary config (readonly)

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

dblaikie added a comment. One of the concerns I'd have, for instance (have you done some broad testing of these patches on sizable code bases?) is that it wouldn't surprise me if clang had some scalability bugs/issues with very long source lines - so it might be necessary to introduce some

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

Meinersbur added a comment. Some bikeshedding: Calls to `AvoidConcat` could be avoided by always inserting a space between tokens at the cost of making the output larger. In the current form, the flag could also be named `-fminimize-whitespace`. Repository: rG LLVM Github Monorepo CHANGES

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

Meinersbur added a comment. In 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

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

dblaikie added a comment. 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. Do you have a particular use case that has exceptionally frequent whitespace-only changes?

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

Meinersbur updated this revision to Diff 353220. Meinersbur edited the summary of this revision. Meinersbur added a comment. - Undo "typechange" changes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104601/new/ https://reviews.llvm.org/D104601

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

Meinersbur created this revision. Herald added subscribers: dang, jvesely. Meinersbur requested review of this revision. Herald added a reviewer: jdoerfert. Herald added subscribers: openmp-commits, libcxx-commits, cfe-commits, sstefan1. Herald added projects: clang, libc++, OpenMP. Herald added a