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

Meinersbur added a comment. In D104601#3105044 , @manmanren wrote: > It will fail the compilation on the preprocessed output with > error: expected unqualified-id > int test();#pragma clang assume_nonnull end The handler for assume_nonull passes an inval

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

manmanren added a comment. This commit seems to cause some regression for "-save-temps" as there is no new line before the pragma. See the below test case, -E will output int test();#pragma clang assume_nonnull It will fail the compilation on the preprocessed output with error: expected unqualifi

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

RKSimon added inline comments. Comment at: clang/lib/Frontend/PrintPreprocessedOutput.cpp:680 +if (RequireSpace || (!MinimizeWhitespace && Tok.hasLeadingSpace()) || +((EmittedTokensOnThisLine || EmittedTokensOnThisLine) && + AvoidConcat(PrevPrevTok, PrevTok, T

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

Meinersbur added a comment. In D104601#2919775 , @tstellar wrote: > Do we have all the issues fixed in trunk yet or do we need to revert in the > release/13.x branch. All known issues have been fixed on trunk. However, @alexfh said they are going to do

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

tstellar added a comment. Do we have all the issues fixed in trunk yet or do we need to revert in the release/13.x branch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104601/new/ https://reviews.llvm.org/D104601

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

romanovvlad added a comment. In D104601#2917065 , @Meinersbur wrote: > Patch uploaded here: D107183 The patch resolves the issue. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

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

Meinersbur added a comment. @mstorsjo I've taken all steps required for the resolution suggested @aaron.ballman. llvm.org/PR51300 suberseedes llvm.org/PR51261 (release-13.x branch is under @tstellard's control). D107183 has been pushed to main to fix the outs

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

mstorsjo added a comment. FYI, 13.0.0 rc1 is getting tagged on Monday, so it’d be good to have the branch in a usable state by then, either with all fixes we currently have in main, or reverted. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104601

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

alexfh added a comment. In D104601#2916973 , @Meinersbur wrote: > @romanovvlad This is due to -fms-extensions. It Expands `__Pragma` to > `#pragma` instead of keeping it a `__Pragma`. This is a one-line fix. > > @alexfh This was fixed by D106924

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

Meinersbur added a comment. > Correct -- that would be unfortunate as I believe you were hoping for this to > be in Clang 13 for ccache support. Yes, that would have been the ideal outcome. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104601/new/

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

Meinersbur added a comment. Patch uploaded here: D107183 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104601/new/ https://reviews.llvm.org/D104601 ___ cfe-commits mail

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

aaron.ballman added a comment. In D104601#2916986 , @Meinersbur wrote: > In D104601#2916897 , @aaron.ballman > wrote: > >> In D104601#2916887 , @Meinersbur >> wrote: >>

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

Meinersbur added a comment. In D104601#2916897 , @aaron.ballman wrote: > In D104601#2916887 , @Meinersbur > wrote: > >> I am working on a fix, but I wouldn't mind reverting. > > I would be comfortable reverting

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

Meinersbur added a comment. @romanovvlad This is due to -fms-extensions. It Expands `__Pragma` to `#pragma` instead of keeping it a `__Pragma`. This is a one-line fix. @alexfh This was fixed by D106924 I would suggest to not revert. Will upload a patch for `-f

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

aaron.ballman added a comment. In D104601#2916887 , @Meinersbur wrote: > I am working on a fix, but I wouldn't mind reverting. I would be comfortable reverting the feature for Clang 13 and fixing forward on trunk (unless there's a need to revert from t

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

Meinersbur added a comment. I am working on a fix, but I wouldn't mind reverting. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104601/new/ https://reviews.llvm.org/D104601 ___ cfe-commits mailing list c

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

alexfh added a comment. Looking at this once again, I'm pretty sure that reverting this is much safer than trying to fix forward. Unless there's a really trivial fix to the outstanding issues, I'll revert this later today. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

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

mstorsjo added a subscriber: tstellar. mstorsjo added a comment. @tstellar, FYI there are still issues being reported with this commit (which was made before the branch). One fix is committed and a backport request was made in https://llvm.org/PR51261, but there’s still a couple other outstandin

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

alexfh added a comment. In D104601#2915102 , @alexfh wrote: > This commit changes the behavior of clang -E -P even when no > -fminimize-whitespace is used. This breaks certain use cases like using clang > to preprocess files for flex, which turns out to

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

alexfh added a comment. This commit changes the behavior of clang -E -P even when no -fminimize-whitespace is used. This breaks certain use cases like using clang to preprocess files for flex, which turns out to be sensitive to the presence of line breaks in places where C++ compilers aren't.

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

romanovvlad added a comment. Hi @Meinersbur, It seems the patch introduces one more regression. The following test doesn't pass on Windows: // RUN: %clang -E %s -o %t.ii // RUN: %clang %t.ii #include "string.h" int main() { return 0; } The following macro from vcruntime.h:

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

Meinersbur added a comment. Proposed fix here: D106924 The reproducer had another whitespace difference before and after D104601 : " typedef __builtin_va_list __gnuc_va_list;" instead of " typedef __builtin_va_list __gnuc_va_

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

Meinersbur added a comment. Looking into it... 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 https://l

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

mstorsjo added a comment. In D104601#2906588 , @mstorsjo wrote: > This broke some cases for me, where the output is missing some newlines, > breaking the output badly. I’ll try to provide a repro… https://martin.st/temp/repro.tar.xz clang -target x86_6

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

mstorsjo added a comment. This broke some cases for me, where the output is missing some newlines, breaking the output badly. I’ll try to provide a repro… Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104601/new/ https://reviews.llvm.org/D104601

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGae6b4238: [Preprocessor] Implement -fminimize-whitespace. (authored by Meinersbur). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

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

aaron.ballman accepted this revision. aaron.ballman added a comment. In D104601#2898095 , @Meinersbur wrote: > Because of how large the switch construct would have been, I created a new > function `types::isDerivedFromC` together with the other function

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

Meinersbur added a comment. Because of how large the switch construct would have been, I created a new function `types::isDerivedFromC` together with the other functions. These commonly have default-cases so compilers would not warn when `Types.def` is amended, but `isDerivedFromC` can be found

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

Meinersbur updated this revision to Diff 360977. Meinersbur added a comment. - Introduce types::isDerivedFromC - Print input type in error message Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104601/new/ https://reviews.llvm.org/D104601 Files:

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

Meinersbur added a comment. In D104601#2896091 , @aaron.ballman wrote: > I think there is, but I'm an eternal optimist. :-) This LGTM, but you should > wait a day or two before landing it in case @dblaikie or @rsmith have > concerns. Of course. ===

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

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. In D104601#2895941 , @Meinersbur wrote: > I compiled the Linux kernel (default config, excludes most drivers. Takes > ~18mins to compil

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

Meinersbur updated this revision to Diff 360753. Meinersbur added a comment. - Revert test changes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104601/new/ https://reviews.llvm.org/D104601 Files: clang/docs/ClangCommandLineReference.rst clang

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

Meinersbur added a comment. I compiled the Linux kernel (default config, excludes most drivers. Takes ~18mins to compile) and indeed found two problems: 1. Linux contains `assembler-with-cpp` files. When compiling with clang, it processes it with `-E` before passing the result to `as`. There ar

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

Meinersbur updated this revision to Diff 360745. Meinersbur marked 2 inline comments as done. Meinersbur added a comment. - Error if -fminimize-whitespace is applied to asm files - Join lines with escaped newlines - Added some FIXMEs for grandfathered bugs Repository: rG LLVM Github Monorepo