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

[Michael Kruse via Phabricator via cfe-commits]

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 

 for printing diagnostics.


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://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Michael Kruse via Phabricator via cfe-commits]

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 ACTION
  https://reviews.llvm.org/D104601/new/

https://reviews.llvm.org/D104601

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/PreprocessorOutputOptions.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/PrintPreprocessedOutput.cpp
  clang/test/CXX/lex/lex.pptoken/p3-2a.cpp
  clang/test/Preprocessor/c99-6_10_3_4_p5.c
  clang/test/Preprocessor/c99-6_10_3_4_p6.c
  clang/test/Preprocessor/comment_save.c
  clang/test/Preprocessor/first-line-indent.c
  clang/test/Preprocessor/hash_line.c
  clang/test/Preprocessor/line-directive-output-mincol.c
  clang/test/Preprocessor/line-directive-output.c
  clang/test/Preprocessor/macro_space.c
  clang/test/Preprocessor/minimize-whitespace-messages.c
  clang/test/Preprocessor/minimize-whitespace.c
  clang/test/Preprocessor/print_line_include.c
  clang/test/Preprocessor/stringize_space.c

Index: clang/test/Preprocessor/stringize_space.c
===
--- clang/test/Preprocessor/stringize_space.c
+++ clang/test/Preprocessor/stringize_space.c
@@ -1,16 +1,18 @@
 // RUN: %clang_cc1 -E %s | FileCheck --strict-whitespace %s
+// RUN: %clang_cc1 -E -P -fminimize-whitespace %s | FileCheck --strict-whitespace %s --check-prefix=MINWS
 
 #define A(b) -#b  ,  - #b  ,  -# b  ,  - # b
 A()
 
 // CHECK: {{^}}-"" , - "" , -"" , - ""{{$}}
-
+// MINWS: {{^}}-"",-"",-"",-""
 
 #define t(x) #x
 t(a
 c)
 
 // CHECK: {{^}}"a c"{{$}}
+// MINWS-SAME: "a c"
 
 #define str(x) #x
 #define f(x) str(-x)
@@ -18,6 +20,7 @@
 1)
 
 // CHECK: {{^}}"-1"
+// MINWS-SAME: "-1"
 
 #define paste(a,b) str(a&1 | FileCheck %s --strict-whitespace --check-prefix=MINCOL
+// RUN: %clang_cc1 -fminimize-whitespace -E -C %s 2>&1 | FileCheck %s --strict-whitespace --check-prefix=MINCCOL
+// RUN: %clang_cc1 -fminimize-whitespace -E -P %s 2>&1 | FileCheck %s --strict-whitespace --check-prefix=MINWS
+// RUN: %clang_cc1 -fminimize-whitespace -E -C -P %s 2>&1 | FileCheck %s --strict-whitespace --check-prefix=MINCWS
+
+#define NOT_OMP  omp  something
+#define HASH #
+
+  int  a; /*  span-comment  */
+  int  b  ;   //  line-comment
+  _Pragma  (  "omp  barrier"  ) x //  more line-comments
+  #pragma  omp  nothing  //  another comment
+HASH  pragma  NOT_OMP
+  int  e;// again a line
+  int  f  ;
+
+
+// MINCOL:  {{^}}# 9 "{{.*}}minimize-whitespace.c"{{$}}
+// MINCOL:  {{^}}int a;{{$}}
+// MINCOL-NEXT: {{^}}int b;{{$}}
+// MINCOL-NEXT: {{^}}#pragma omp barrier{{$}}
+// MINCOL-NEXT: # 11 "{{.*}}minimize-whitespace.c"
+// MINCOL-NEXT: {{^}}x{{$}}
+// MINCOL-NEXT: {{^}}#pragma omp nothing{{$}}
+// MINCOL-NEXT: {{^ }}#pragma omp something{{$}}
+// MINCOL-NEXT: {{^}}int e;{{$}}
+// MINCOL-NEXT: {{^}}int f;{{$}}
+
+// FIXME: Comments after pragmas disappear, even without -fminimize-whitespace
+// MINCCOL:  {{^}}# 9 "{{.*}}minimize-whitespace.c"{{$}}
+// MINCCOL:  {{^}}int a;/*  span-comment  */{{$}}
+// MINCCOL-NEXT: {{^}}int b;//  line-comment{{$}}
+// MINCCOL-NEXT: {{^}}#pragma omp barrier{{$}}
+// MINCCOL-NEXT: # 11 "{{.*}}minimize-whitespace.c"
+// MINCCOL-NEXT: {{^}}x//  more line-comments{{$}}
+// MINCCOL-NEXT: {{^}}#pragma omp nothing{{$}}
+// MINCCOL-NEXT: {{^ }}#pragma omp something{{$}}
+// MINCCOL-NEXT: {{^}}int e;// again a line{{$}}
+// MINCCOL-NEXT: {{^}}int f;{{$}}
+
+// MINWS:  {{^}}int a;int b;{{$}}
+// MINWS-NEXT: {{^}}#pragma omp barrier{{$}}
+// MINWS-NEXT: {{^}}x{{$}}
+// MINWS-NEXT: {{^}}#pragma omp nothing{{$}}
+// MINWS-NEXT: {{^ }}#pragma omp something int e;int f;{{$}}
+
+// FIXME: Comments after pragmas disappear, even without -fminimize-whitespace
+// MINCWS:  {{^}}int a;/*  span-comment  */int b;//  line-comment{{$}}
+// MINCWS-NEXT: {{^}}#pragma omp barrier{{$}}
+// MINCWS-NEXT: {{^}}x//  more line-comments{{$}}
+// MINCWS-NEXT: {{^}}#pragma omp nothing{{$}}
+// MINCWS-NEXT: {{^ }}#pragma omp something int e;// again a line{{$}}
+// MINCWS-NEXT: {{^}}int f;
+
Index: clang/test/Preprocessor/minimize-whitespace-messages.c
===
--- /dev/null
+++ clang/test/Preprocessor/minimize-whitespace-messages.c
@@ -0,0 +1,5 @@
+// RUN: not %clang -c -fminimize-whitespace %s 2>&1 | FileCheck %s --check-prefix=ON
+// ON: error: invalid argument '-fminimize-whitespace' only allowed with '-E'
+
+// RUN: not %clang -c -fno-minimize-whitespace %s 2>&1 | FileCheck %s  --check-prefix=OFF
+// OFF: error: invalid argument '-fno-minimize-whitespace' only allowed with '-E'
Index: clang/test/Preprocessor/macro_space.c
===
--- 

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

[Aaron Ballman via Phabricator via cfe-commits]

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 reasons unknown and the developer replicates it when reporting 
>> the bug because they're not looking at what flags can be removed.
>
> I made the flag an error if used without `-E`, so the build system cannot add 
> it.
>
> Flags such as `-P`, `-fuse-line-directives`, `-frewrite-imports` and 
> `-frewrite-includes` are also silently ignored without `-E`. This requirement 
> seems exclusive for `-felide-unnecessary-whitespace`. It also suggests that 
> it is not a problematic scenario.

Perhaps it's not a problematic scenario, but I'd consider those cases to also 
be bugs. Silently ignoring things the user wrote (either in code or on the 
command line) is typically user-hostile.

 The "very long line" example mentioned by @dblaikie is sort of along these 
 lines (sorry for the bad pun).
>>>
>>> I can add forced line breaks (after/before 80 cols? configurable?) if 
>>> requested.
>>
>> I don't think that's necessary just yet. If there's an issue in practice, it 
>> seems like we could address it once we have the real world use in front of 
>> us. However, It'd help my confidence if you were able to run this 
>> functionality over a large corpus of code to see if any issues do pop out, 
>> if that's plausible for you.
>
> I ran it (using ccache) over llvm-project and the llvm-test-suite. Is there 
> another large corpus do you have in mind?

A Linux or BSD distro worth of code would be really nice, but I don't insist. 
The trouble with llvm-project is that it has a somewhat uniform style for code, 
and I've found that tools in distro packages tend to exercise more interesting 
cases sometimes.

>> I also think we should rename it because "normalize whitespace" can be 
>> ambiguous depending on what context you're reading the words in.
>
> Changed to your suggested `-felide-unnecessary-whitespace`, although I think 
> name will not allow use to insert whitespace e.g. for keeping the line length 
> down. If that's not required, I would prefer `-fminimize-whitespace` over 
> -felide-unnecessary-whitespace`.

I'd be perfectly happy with `-fminimize-whitespace` as well if that's your 
preference.




Comment at: clang/docs/ClangCommandLineReference.rst:2484
+-P option to normlize whitespace such that two files with only formatted
+changes are equal.
+

Meinersbur wrote:
> aaron.ballman wrote:
> > Should this option be ignored when not performing a preprocessing action 
> > (and documented as such)?
> I changed it to an error if used without `-E`.
Thanks! Can you add that info to the documentation here?


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://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Michael Kruse via Phabricator via cfe-commits]

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
  https://reviews.llvm.org/D104601/new/

https://reviews.llvm.org/D104601

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/PreprocessorOutputOptions.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/PrintPreprocessedOutput.cpp
  clang/test/CXX/lex/lex.pptoken/p3-2a.cpp
  clang/test/Preprocessor/c99-6_10_3_4_p5.c
  clang/test/Preprocessor/c99-6_10_3_4_p6.c
  clang/test/Preprocessor/comment_save.c
  clang/test/Preprocessor/elide-unnecessary-whitespace-messages.c
  clang/test/Preprocessor/elide-unnecessary-whitespace.c
  clang/test/Preprocessor/first-line-indent.c
  clang/test/Preprocessor/hash_line.c
  clang/test/Preprocessor/line-directive-output-normcol.c
  clang/test/Preprocessor/line-directive-output.c
  clang/test/Preprocessor/macro_space.c
  clang/test/Preprocessor/print_line_include.c
  clang/test/Preprocessor/stringize_space.c

Index: clang/test/Preprocessor/stringize_space.c
===
--- clang/test/Preprocessor/stringize_space.c
+++ clang/test/Preprocessor/stringize_space.c
@@ -1,16 +1,18 @@
 // RUN: %clang_cc1 -E %s | FileCheck --strict-whitespace %s
+// RUN: %clang_cc1 -E -P -felide-unnecessary-whitespace %s | FileCheck --strict-whitespace %s --check-prefix=NORMWS
 
 #define A(b) -#b  ,  - #b  ,  -# b  ,  - # b
 A()
 
 // CHECK: {{^}}-"" , - "" , -"" , - ""{{$}}
-
+// NORMWS: {{^}}-"",-"",-"",-""
 
 #define t(x) #x
 t(a
 c)
 
 // CHECK: {{^}}"a c"{{$}}
+// NORMWS-SAME: "a c"
 
 #define str(x) #x
 #define f(x) str(-x)
@@ -18,6 +20,7 @@
 1)
 
 // CHECK: {{^}}"-1"
+// NORMWS-SAME: "-1"
 
 #define paste(a,b) str(a < > <> <> < > <> < > < >
+// NORMWS: FOO1<><><><><><><><>
 
 TEST(FOO2,)
 // CHECK: FOO2 <> < > <> <> < > <> < > < >
+// NORMWS-SAME: FOO2<><><><><><><><>
 
 TEST(FOO3,)
 // CHECK: FOO3 <> < > <> <> < > <> < > < >
+// NORMWS-SAME: FOO3<><><><><><><><>
 
 TEST(FOO4,)
 // CHECK: FOO4 < > < > < > < > < > < > < > < >
+// NORMWS-SAME: FOO4<><><><><><><><>
 
 TEST(FOO5,)
 // CHECK: FOO5 < > < > < > < > < > < > < > < >
+// NORMWS-SAME: FOO5<><><><><><><><>
 
 TEST(FOO6,)
 // CHECK: FOO6 <[]> < []> <[]> <[]> <[] > <[]> <[] > < []>
+// NORMWS-SAME: FOO6<[]><[]><[]><[]><[]><[]><[]><[]>
 
 TEST(FOO7,)
 // CHECK: FOO7 <[ ]> < [ ]> <[ ]> <[ ]> <[ ] > <[ ]> <[ ] > < [ ]>
+// NORMWS-SAME: FOO7<[]><[]><[]><[]><[]><[]><[]><[]>
 
 TEST(FOO8,)
 // CHECK: FOO8 <[ ]> < [ ]> <[ ]> <[ ]> <[ ] > <[ ]> <[ ] > < [ ]>
+// NORMWS-SAME: FOO8<[]><[]><[]><[]><[]><[]><[]><[]>
Index: clang/test/Preprocessor/line-directive-output.c
===
--- clang/test/Preprocessor/line-directive-output.c
+++ clang/test/Preprocessor/line-directive-output.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -E %s 2>&1 | FileCheck %s -strict-whitespace
+// RUN: %clang_cc1 -E -felide-unnecessary-whitespace %s 2>&1 | FileCheck %s -strict-whitespace
 // PR6101
 int a;
 // CHECK: # 1 "{{.*}}line-directive-output.c"
Index: clang/test/Preprocessor/line-directive-output-normcol.c
===
--- /dev/null
+++ clang/test/Preprocessor/line-directive-output-normcol.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -E -felide-unnecessary-whitespace %s 2>&1 | FileCheck %s -strict-whitespace
+
+// CHECK:  # 6 "{{.*}}line-directive-output-normcol.c"
+// CHECK-NEXT: int x;
+// CHECK-NEXT: int y;
+int x;
+int y;
+// CHECK-NEXT: # 10 "{{.*}}line-directive-output-normcol.c"
+// CHECK-NEXT: int z;
+int z;
Index: clang/test/Preprocessor/hash_line.c
===
--- clang/test/Preprocessor/hash_line.c
+++ clang/test/Preprocessor/hash_line.c
@@ -4,6 +4,10 @@
 // CHECK-NEXT: {{^  #$}}
 // CHECK-NEXT: {{^2$}}
 // CHECK-NEXT: {{^   #$}}
+
+// RUN: %clang_cc1 -E -P -felide-unnecessary-whitespace %s | FileCheck --strict-whitespace %s --check-prefix=NORMWS
+// NORMWS:  {{^}}1#2#{{$}}
+
 #define EMPTY
 #define IDENTITY(X) X
 1
Index: clang/test/Preprocessor/first-line-indent.c
===
--- clang/test/Preprocessor/first-line-indent.c
+++ clang/test/Preprocessor/first-line-indent.c
@@ -1,7 +1,14 @@
foo
 // RUN: %clang_cc1 -E %s | FileCheck -strict-whitespace %s
+// RUN: %clang_cc1 -E -felide-unnecessary-whitespace %s | FileCheck -strict-whitespace %s --check-prefix=NORMCOL
+// RUN: %clang_cc1 -E -felide-unnecessary-whitespace -P %s | FileCheck -strict-whitespace %s --check-prefix=NORMWS
bar
 
 // CHECK: {{^   }}foo
 // CHECK: {{^   }}bar
 
+// NORMCOL: {{^}}foo
+// NORMCOL: {{^}}bar
+
+// NORMWS: 

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

[Michael Kruse via Phabricator via cfe-commits]

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 , 
>> @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 issue with the compiler, and I worry that uses of this 
>>> flag will have unintended consequences in that scenario.
>>
>> Why would one add `-fnormalize-whitespace` for this use case?
>
> I don't think they'd *add* it, I worry it's already used by their build 
> system for reasons unknown and the developer replicates it when reporting the 
> bug because they're not looking at what flags can be removed.

I made the flag an error if used without `-E`, so the build system cannot add 
it.

Flags such as `-P`, `-fuse-line-directives`, `-frewrite-imports` and 
`-frewrite-includes` are also silently ignored without `-E`. This requirement 
seems exclusive for `-felide-unnecessary-whitespace`. It also suggests that it 
is not a problematic scenario.

>>> The "very long line" example mentioned by @dblaikie is sort of along these 
>>> lines (sorry for the bad pun).
>>
>> I can add forced line breaks (after/before 80 cols? configurable?) if 
>> requested.
>
> I don't think that's necessary just yet. If there's an issue in practice, it 
> seems like we could address it once we have the real world use in front of 
> us. However, It'd help my confidence if you were able to run this 
> functionality over a large corpus of code to see if any issues do pop out, if 
> that's plausible for you.

I ran it (using ccache) over llvm-project and the llvm-test-suite. Is there 
another large corpus do you have in mind?

> I'd like to see it diagnosed when used outside of a preprocessor invocation 
> because it really serves no purpose there.

I made the flag an error if not used without `-E`.

> I also think we should rename it because "normalize whitespace" can be 
> ambiguous depending on what context you're reading the words in.

Changed to your suggested `-felide-unnecessary-whitespace`, although I think 
name will not allow use to insert whitespace e.g. for keeping the line length 
down. If that's not required, I would prefer `-fminimize-whitespace` over 
-felide-unnecessary-whitespace`.




Comment at: clang/docs/ClangCommandLineReference.rst:2484
+-P option to normlize whitespace such that two files with only formatted
+changes are equal.
+

aaron.ballman wrote:
> Should this option be ignored when not performing a preprocessing action (and 
> documented as such)?
I changed it to an error if used without `-E`.



Comment at: clang/include/clang/Driver/Options.td:1789
   PosFlag, 
NegFlag>;
+defm normalize_whitespace : BoolFOption<"normalize-whitespace",
+  PreprocessorOutputOpts<"NormalizeWhitespace">, DefaultFalse,

aaron.ballman wrote:
> I think I'd feel most comfortable if this didn't use "normalize" in the name. 
> When I hear that, I think it's going to be doing something like converting 
> all (perhaps funky Unicode) notions of whitespace into ASCII space 
> characters. But this option doesn't actually do that -- it's removing as much 
> whitespace from the preprocessed output as possible. Would it be better named 
> as `-felide-unnecessary-whitespace` or something along those lines?
I don't think it is ambiguous what whitespace means for the preprocessor. 
Currently, `-E` already converts into `'\n'` and `'\n'` only.

I don't like the suggested name but still applied it to this patch. I'd prefer 
`-fminimize-whitespace`. However, the name would be wrong if we add additional 
whitespace such as line-breaks to avoid the long line problem, of we 
unconditionally always emit a space between tokens to not having to call 
`AvoidConcat` which would still serve the same goal.


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://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Aaron Ballman via Phabricator via cfe-commits]

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 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.
>
> Why would one add `-fnormalize-whitespace` for this use case?

I don't think they'd *add* it, I worry it's already used by their build system 
for reasons unknown and the developer replicates it when reporting the bug 
because they're not looking at what flags can be removed.

>> The "very long line" example mentioned by @dblaikie is sort of along these 
>> lines (sorry for the bad pun).
>
> I can add forced line breaks (after/before 80 cols? configurable?) if 
> requested.

I don't think that's necessary just yet. If there's an issue in practice, it 
seems like we could address it once we have the real world use in front of us. 
However, It'd help my confidence if you were able to run this functionality 
over a large corpus of code to see if any issues do pop out, if that's 
plausible for you.

That said, I think I've convinced myself that this functionality is reasonable 
(if a bit novel), but I'd like to see it diagnosed when used outside of a 
preprocessor invocation because it really serves no purpose there. I also think 
we should rename it because "normalize whitespace" can be ambiguous depending 
on what context you're reading the words in.




Comment at: clang/docs/ClangCommandLineReference.rst:2484
+-P option to normlize whitespace such that two files with only formatted
+changes are equal.
+

Should this option be ignored when not performing a preprocessing action (and 
documented as such)?



Comment at: clang/include/clang/Driver/Options.td:1789
   PosFlag, 
NegFlag>;
+defm normalize_whitespace : BoolFOption<"normalize-whitespace",
+  PreprocessorOutputOpts<"NormalizeWhitespace">, DefaultFalse,

I think I'd feel most comfortable if this didn't use "normalize" in the name. 
When I hear that, I think it's going to be doing something like converting all 
(perhaps funky Unicode) notions of whitespace into ASCII space characters. But 
this option doesn't actually do that -- it's removing as much whitespace from 
the preprocessed output as possible. Would it be better named as 
`-felide-unnecessary-whitespace` or something along those lines?


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://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Michael Kruse via Phabricator via cfe-commits]

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 expensive.


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://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Michael Kruse via Phabricator via cfe-commits]

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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Michael Kruse via Phabricator via cfe-commits]

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 issue with the compiler, and I worry that uses of this 
> flag will have unintended consequences in that scenario.

Why would one add `-fnormalize-whitespace` for this use case?

> The "very long line" example mentioned by @dblaikie is sort of along these 
> lines (sorry for the bad pun).

I can add a forced line breaks (after/before 80 cols? configurable?) if 
requested.




Comment at: clang/lib/Frontend/PrintPreprocessedOutput.cpp:174
+  ///on being on the same line, such as directives.
+  void HandleWhitespaceBeforeTok(Token , bool RequireSpace,
+ bool RequireSameLine);

aaron.ballman wrote:
> Can `Tok` be `const Token &` instead?
Note that this is a renamed `bool HandleFirstTokOnLine(Token )`. Renamed 
because it is now called for every token.


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://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Michael Kruse via Phabricator via cfe-commits]

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

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/PreprocessorOutputOptions.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/PrintPreprocessedOutput.cpp
  clang/test/CXX/lex/lex.pptoken/p3-2a.cpp
  clang/test/Preprocessor/c99-6_10_3_4_p5.c
  clang/test/Preprocessor/c99-6_10_3_4_p6.c
  clang/test/Preprocessor/comment_save.c
  clang/test/Preprocessor/first-line-indent.c
  clang/test/Preprocessor/hash_line.c
  clang/test/Preprocessor/line-directive-output-normcol.c
  clang/test/Preprocessor/line-directive-output.c
  clang/test/Preprocessor/macro_space.c
  clang/test/Preprocessor/normalize-whitespace.c
  clang/test/Preprocessor/print_line_include.c
  clang/test/Preprocessor/stringize_space.c

Index: clang/test/Preprocessor/stringize_space.c
===
--- clang/test/Preprocessor/stringize_space.c
+++ clang/test/Preprocessor/stringize_space.c
@@ -1,16 +1,18 @@
 // RUN: %clang_cc1 -E %s | FileCheck --strict-whitespace %s
+// RUN: %clang_cc1 -E -P -fnormalize-whitespace %s | FileCheck --strict-whitespace %s --check-prefix=NORMWS
 
 #define A(b) -#b  ,  - #b  ,  -# b  ,  - # b
 A()
 
 // CHECK: {{^}}-"" , - "" , -"" , - ""{{$}}
-
+// NORMWS: {{^}}-"",-"",-"",-""
 
 #define t(x) #x
 t(a
 c)
 
 // CHECK: {{^}}"a c"{{$}}
+// NORMWS-SAME: "a c"
 
 #define str(x) #x
 #define f(x) str(-x)
@@ -18,6 +20,7 @@
 1)
 
 // CHECK: {{^}}"-1"
+// NORMWS-SAME: "-1"
 
 #define paste(a,b) str(a&1 | FileCheck %s --strict-whitespace --check-prefix=NORMCOL
+// RUN: %clang_cc1 -E -C -fnormalize-whitespace %s 2>&1 | FileCheck %s --strict-whitespace --check-prefix=NORMCCOL
+// RUN: %clang_cc1 -E -P -fnormalize-whitespace %s 2>&1 | FileCheck %s --strict-whitespace --check-prefix=NORMWS
+// RUN: %clang_cc1 -E -C -P -fnormalize-whitespace %s 2>&1 | FileCheck %s --strict-whitespace --check-prefix=NORMCWS
+
+#define NOT_OMP  omp  something
+#define HASH #
+
+  int  a; /*  span-comment  */
+  int  b  ;   //  line-comment
+  _Pragma  (  "omp  barrier"  ) x //  more line-comments
+  #pragma  omp  nothing  //  another comment
+HASH  pragma  NOT_OMP
+  int  e;// again a line
+  int  f  ;
+
+
+// NORMCOL:  {{^}}# 9 "{{.*}}normalize-whitespace.c"{{$}}
+// NORMCOL:  {{^}}int a;{{$}}
+// NORMCOL-NEXT: {{^}}int b;{{$}}
+// NORMCOL-NEXT: {{^}}#pragma omp barrier{{$}}
+// NORMCOL-NEXT: # 11 "{{.*}}normalize-whitespace.c"
+// NORMCOL-NEXT: {{^}}x{{$}}
+// NORMCOL-NEXT: {{^}}#pragma omp nothing{{$}}
+// NORMCOL-NEXT: {{^ }}#pragma omp something{{$}}
+// NORMCOL-NEXT: {{^}}int e;{{$}}
+// NORMCOL-NEXT: {{^}}int f;{{$}}
+
+// FIXME: Comments after pragmas disappear, even without -fnormalize-whitespace
+// NORMCCOL:  {{^}}# 9 "{{.*}}normalize-whitespace.c"{{$}}
+// NORMCCOL:  {{^}}int a;/*  span-comment  */{{$}}
+// NORMCCOL-NEXT: {{^}}int b;//  line-comment{{$}}
+// NORMCCOL-NEXT: {{^}}#pragma omp barrier{{$}}
+// NORMCCOL-NEXT: # 11 "{{.*}}normalize-whitespace.c"
+// NORMCCOL-NEXT: {{^}}x//  more line-comments{{$}}
+// NORMCCOL-NEXT: {{^}}#pragma omp nothing{{$}}
+// NORMCCOL-NEXT: {{^ }}#pragma omp something{{$}}
+// NORMCCOL-NEXT: {{^}}int e;// again a line{{$}}
+// NORMCCOL-NEXT: {{^}}int f;{{$}}
+
+// NORMWS:  {{^}}int a;int b;{{$}}
+// NORMWS-NEXT: {{^}}#pragma omp barrier{{$}}
+// NORMWS-NEXT: {{^}}x{{$}}
+// NORMWS-NEXT: {{^}}#pragma omp nothing{{$}}
+// NORMWS-NEXT: {{^ }}#pragma omp something int e;int f;{{$}}
+
+// FIXME: Comments after pragmas disappear, even without -fnormalize-whitespace
+// NORMCWS:  {{^}}int a;/*  span-comment  */int b;//  line-comment{{$}}
+// NORMCWS-NEXT: {{^}}#pragma omp barrier{{$}}
+// NORMCWS-NEXT: {{^}}x//  more line-comments{{$}}
+// NORMCWS-NEXT: {{^}}#pragma omp nothing{{$}}
+// NORMCWS-NEXT: {{^ }}#pragma omp something int e;// again a line{{$}}
+// NORMCWS-NEXT: {{^}}int f;
+
Index: clang/test/Preprocessor/macro_space.c
===
--- clang/test/Preprocessor/macro_space.c
+++ clang/test/Preprocessor/macro_space.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -E %s | FileCheck --strict-whitespace %s
+// RUN: %clang_cc1 -E -P -fnormalize-whitespace %s | FileCheck --strict-whitespace %s --check-prefix=NORMWS
 
 #define FOO1()
 #define FOO2(x)x
@@ -13,24 +14,32 @@
 
 TEST(FOO1,)
 // CHECK: FOO1 <> < > <> <> < > <> < > < >
+// NORMWS: FOO1<><><><><><><><>
 
 TEST(FOO2,)
 // CHECK: FOO2 <> < > <> <> < > <> < > < >
+// NORMWS-SAME: FOO2<><><><><><><><>
 
 TEST(FOO3,)
 // CHECK: FOO3 <> < > <> <> < > <> < > < >
+// NORMWS-SAME: FOO3<><><><><><><><>
 
 TEST(FOO4,)
 // CHECK: FOO4 < > < > < > < > < > < > < > 

[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 , 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

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Michael Kruse via Phabricator via cfe-commits]

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 
. By default (unless a corresponding 
"sloppiness" option is used), only direct hits (by binary file comparison) are 
used if: There is compiler output, debug info is generated, or `.incbin` 
assembly instruction is used.

However, the is orthogonal to `-fnormalize-whitespace`. It is a mitigation of 
using `-E -P` as preprocessor intermediate.


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://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[David Blaikie via Phabricator via cfe-commits]

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 D104601#2831951 , @dblaikie 
> wrote:
>
>> 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 (arbitrary?) newlines to break up the 
>> code. Though I'm not sure - no need to do that pre-emptively, but might be 
>> good to have some data that indicates whether this might be a problem or not.
>
> I found no such issues during my trials. However, I think the request is 
> understandable an I would implement it on request. It introduces a new 
> problem having to determine where no newlines mat be introduced (e.g. within 
> a #pragma).

Yeah, no need to get ahead of that - just something to be aware of/on the look 
out if this feature ends up in use.

One other thing: This wouldn't be usable when using debug info, presumably, 
because it'd refer to the wrong lines, etc.


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://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Michael Kruse via Phabricator via cfe-commits]

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) 
/home/meinersbur/install/ccache/release/etc/ccache.conf
  stats updated   Fri Jun 25 02:00:22 2021
  stats zeroedWed Jun 23 03:22:36 2021
  cache hit (direct)  1001
  cache hit (preprocessed)5904
  cache miss 10717
  cache hit rate 39.18 %
  compile failed 5
  cleanups performed 0
  files in cache 38182
  cache size   1.9 GB
  max cache size   5.5 GB

unify_mode with `-fnormalize-whitespace`:

  $ ccache -d . -s
  cache directory .
  primary config  ./ccache.conf
  secondary config (readonly) 
/home/meinersbur/install/ccache/release/etc/ccache.conf
  stats updated   Fri Jun 25 02:04:19 2021
  stats zeroedWed Jun 23 03:22:31 2021
  cache hit (direct)  1001
  cache hit (preprocessed)2663
  cache miss 13957
  cache hit rate 20.79 %
  compile failed 5
  cleanups performed 0
  files in cache 44661
  cache size   2.4 GB
  max cache size   5.5 GB

Admittedly, I focused on changes (of Clang and Polly) like refactoring, 
improving comments, minimize difference to upstream (clang-)formatting etc. 
during the testing.

The difference is most pronounced with changes such as 
rG3e8d1e8b12ba9017b861fff94afdd4a29b39de17 
:

  cache hit (direct) 0
  cache hit (preprocessed)   0
  cache miss  2245
  cache hit rate  0.00 %

vs

  cache hit (direct) 0
  cache hit (preprocessed)2235
  cache miss10
  cache hit rate 99.55 %

(the misses come from compilations with warning diagnostics)

In D104601#2831951 , @dblaikie wrote:

> 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 (arbitrary?) newlines to break up the 
> code. Though I'm not sure - no need to do that pre-emptively, but might be 
> good to have some data that indicates whether this might be a problem or not.

I found no such issues during my trials. However, I think the request is 
understandable an I would implement it on request. It introduces a new problem 
having to determine where no newlines mat be introduced (e.g. within a #pragma).


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://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[David Blaikie via Phabricator via cfe-commits]

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 (arbitrary?) newlines to break up the code. Though 
I'm not sure - no need to do that pre-emptively, but might be good to have some 
data that indicates whether this might be a problem or not.


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://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Michael Kruse via Phabricator via cfe-commits]

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 SINCE LAST ACTION
  https://reviews.llvm.org/D104601/new/

https://reviews.llvm.org/D104601

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Michael Kruse via Phabricator via cfe-commits]

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 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

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[David Blaikie via Phabricator via cfe-commits]

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?
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?


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://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Michael Kruse via Phabricator via cfe-commits]

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

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/PreprocessorOutputOptions.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/PrintPreprocessedOutput.cpp
  clang/test/CXX/lex/lex.pptoken/p3-2a.cpp
  clang/test/Preprocessor/c99-6_10_3_4_p5.c
  clang/test/Preprocessor/c99-6_10_3_4_p6.c
  clang/test/Preprocessor/comment_save.c
  clang/test/Preprocessor/first-line-indent.c
  clang/test/Preprocessor/hash_line.c
  clang/test/Preprocessor/line-directive-output-normcol.c
  clang/test/Preprocessor/line-directive-output.c
  clang/test/Preprocessor/macro_space.c
  clang/test/Preprocessor/normalize-whitespace.c
  clang/test/Preprocessor/print_line_include.c
  clang/test/Preprocessor/stringize_space.c

Index: clang/test/Preprocessor/stringize_space.c
===
--- clang/test/Preprocessor/stringize_space.c
+++ clang/test/Preprocessor/stringize_space.c
@@ -1,16 +1,18 @@
 // RUN: %clang_cc1 -E %s | FileCheck --strict-whitespace %s
+// RUN: %clang_cc1 -E -P -fnormalize-whitespace %s | FileCheck --strict-whitespace %s --check-prefix=NORMWS
 
 #define A(b) -#b  ,  - #b  ,  -# b  ,  - # b
 A()
 
 // CHECK: {{^}}-"" , - "" , -"" , - ""{{$}}
-
+// NORMWS: {{^}}-"",-"",-"",-""
 
 #define t(x) #x
 t(a
 c)
 
 // CHECK: {{^}}"a c"{{$}}
+// NORMWS-SAME: "a c"
 
 #define str(x) #x
 #define f(x) str(-x)
@@ -18,6 +20,7 @@
 1)
 
 // CHECK: {{^}}"-1"
+// NORMWS-SAME: "-1"
 
 #define paste(a,b) str(a&1 | FileCheck %s --strict-whitespace --check-prefix=NORMCOL
+// RUN: %clang_cc1 -E -C -fnormalize-whitespace %s 2>&1 | FileCheck %s --strict-whitespace --check-prefix=NORMCCOL
+// RUN: %clang_cc1 -E -P -fnormalize-whitespace %s 2>&1 | FileCheck %s --strict-whitespace --check-prefix=NORMWS
+// RUN: %clang_cc1 -E -C -P -fnormalize-whitespace %s 2>&1 | FileCheck %s --strict-whitespace --check-prefix=NORMCWS
+
+#define NOT_OMP  omp  something
+#define HASH #
+
+  int  a; /*  span-comment  */
+  int  b  ;   //  line-comment
+  _Pragma  (  "omp  barrier"  ) x //  more line-comments
+  #pragma  omp  nothing  //  another comment
+HASH  pragma  NOT_OMP
+  int  e;// again a line
+  int  f  ;
+
+
+// NORMCOL:  {{^}}# 9 "{{.*}}normalize-whitespace.c"{{$}}
+// NORMCOL:  {{^}}int a;{{$}}
+// NORMCOL-NEXT: {{^}}int b;{{$}}
+// NORMCOL-NEXT: {{^}}#pragma omp barrier{{$}}
+// NORMCOL-NEXT: # 11 "{{.*}}normalize-whitespace.c"
+// NORMCOL-NEXT: {{^}}x{{$}}
+// NORMCOL-NEXT: {{^}}#pragma omp nothing{{$}}
+// NORMCOL-NEXT: {{^ }}#pragma omp something{{$}}
+// NORMCOL-NEXT: {{^}}int e;{{$}}
+// NORMCOL-NEXT: {{^}}int f;{{$}}
+
+// FIXME: Comments after pragmas disappear, even without -fnormalize-whitespace
+// NORMCCOL:  {{^}}# 9 "{{.*}}normalize-whitespace.c"{{$}}
+// NORMCCOL:  {{^}}int a;/*  span-comment  */{{$}}
+// NORMCCOL-NEXT: {{^}}int b;//  line-comment{{$}}
+// NORMCCOL-NEXT: {{^}}#pragma omp barrier{{$}}
+// NORMCCOL-NEXT: # 11 "{{.*}}normalize-whitespace.c"
+// NORMCCOL-NEXT: {{^}}x//  more line-comments{{$}}
+// NORMCCOL-NEXT: {{^}}#pragma omp nothing{{$}}
+// NORMCCOL-NEXT: {{^ }}#pragma omp something{{$}}
+// NORMCCOL-NEXT: {{^}}int e;// again a line{{$}}
+// NORMCCOL-NEXT: {{^}}int f;{{$}}
+
+// NORMWS:  {{^}}int a;int b;{{$}}
+// NORMWS-NEXT: {{^}}#pragma omp barrier{{$}}
+// NORMWS-NEXT: {{^}}x{{$}}
+// NORMWS-NEXT: {{^}}#pragma omp nothing{{$}}
+// NORMWS-NEXT: {{^ }}#pragma omp something int e;int f;{{$}}
+
+// FIXME: Comments after pragmas disappear, even without -fnormalize-whitespace
+// NORMCWS:  {{^}}int a;/*  span-comment  */int b;//  line-comment{{$}}
+// NORMCWS-NEXT: {{^}}#pragma omp barrier{{$}}
+// NORMCWS-NEXT: {{^}}x//  more line-comments{{$}}
+// NORMCWS-NEXT: {{^}}#pragma omp nothing{{$}}
+// NORMCWS-NEXT: {{^ }}#pragma omp something int e;// again a line{{$}}
+// NORMCWS-NEXT: {{^}}int f;
+
Index: clang/test/Preprocessor/macro_space.c
===
--- clang/test/Preprocessor/macro_space.c
+++ clang/test/Preprocessor/macro_space.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -E %s | FileCheck --strict-whitespace %s
+// RUN: %clang_cc1 -E -P -fnormalize-whitespace %s | FileCheck --strict-whitespace %s --check-prefix=NORMWS
 
 #define FOO1()
 #define FOO2(x)x
@@ -13,24 +14,32 @@
 
 TEST(FOO1,)
 // CHECK: FOO1 <> < > <> <> < > <> < > < >
+// NORMWS: FOO1<><><><><><><><>
 
 TEST(FOO2,)
 // CHECK: FOO2 <> < > <> <> < > <> < > < >
+// NORMWS-SAME: FOO2<><><><><><><><>
 
 TEST(FOO3,)
 // CHECK: FOO3 <> < > <> <> < > <> < > < >
+// NORMWS-SAME: FOO3<><><><><><><><>
 
 TEST(FOO4,)
 // CHECK: FOO4 < > < > < > < > < > < > < > < >

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

[Michael Kruse via Phabricator via cfe-commits]

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 reviewer: libc++.

This patch adds the `-fnormalize-whitespace` with the following effects:

- If combined with `-E`, remove as much non-line-breaking whitespace as possible
- If combined with `-E -P`, removes as much whitespace as possible, including 
line-breaks.

The motivation is to reduce


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104601

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/PreprocessorOutputOptions.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/PrintPreprocessedOutput.cpp
  clang/test/CXX/lex/lex.pptoken/p3-2a.cpp
  
clang/test/Driver/Inputs/basic_cross_linux_tree/usr/bin/i386-unknown-linux-gnu-ld
  
clang/test/Driver/Inputs/basic_cross_linux_tree/usr/bin/x86_64-unknown-linux-gnu-ld
  
clang/test/Driver/Inputs/basic_cross_linux_tree/usr/i386-unknown-linux-gnu/bin/ld
  
clang/test/Driver/Inputs/basic_cross_linux_tree/usr/x86_64-unknown-linux-gnu/bin/ld
  clang/test/Driver/Inputs/multilib_32bit_linux_tree/usr/bin/as
  clang/test/Driver/Inputs/multilib_32bit_linux_tree/usr/bin/ld
  
clang/test/Driver/Inputs/multilib_32bit_linux_tree/usr/i386-unknown-linux/bin/as
  
clang/test/Driver/Inputs/multilib_32bit_linux_tree/usr/i386-unknown-linux/bin/ld
  clang/test/Driver/Inputs/multilib_64bit_linux_tree/usr/bin/as
  clang/test/Driver/Inputs/multilib_64bit_linux_tree/usr/bin/ld
  
clang/test/Driver/Inputs/multilib_64bit_linux_tree/usr/x86_64-unknown-linux/bin/as
  
clang/test/Driver/Inputs/multilib_64bit_linux_tree/usr/x86_64-unknown-linux/bin/ld
  clang/test/Preprocessor/c99-6_10_3_4_p5.c
  clang/test/Preprocessor/c99-6_10_3_4_p6.c
  clang/test/Preprocessor/comment_save.c
  clang/test/Preprocessor/first-line-indent.c
  clang/test/Preprocessor/hash_line.c
  clang/test/Preprocessor/line-directive-output-normcol.c
  clang/test/Preprocessor/line-directive-output.c
  clang/test/Preprocessor/macro_space.c
  clang/test/Preprocessor/normalize-whitespace.c
  clang/test/Preprocessor/print_line_include.c
  clang/test/Preprocessor/stringize_space.c
  libclc/amdgcn-mesa3d
  libcxx/test/std/pstl
  openmp/tools/analyzer/llvm-openmp-analyzer++

Index: openmp/tools/analyzer/llvm-openmp-analyzer++
===
--- /dev/null
+++ openmp/tools/analyzer/llvm-openmp-analyzer++
@@ -0,0 +1 @@
+llvm-openmp-analyzer
\ No newline at end of file
Index: libcxx/test/std/pstl
===
--- /dev/null
+++ libcxx/test/std/pstl
@@ -0,0 +1 @@
+../../../pstl/test/std
\ No newline at end of file
Index: libclc/amdgcn-mesa3d
===
--- /dev/null
+++ libclc/amdgcn-mesa3d
@@ -0,0 +1 @@
+amdgcn-amdhsa
\ No newline at end of file
Index: clang/test/Preprocessor/stringize_space.c
===
--- clang/test/Preprocessor/stringize_space.c
+++ clang/test/Preprocessor/stringize_space.c
@@ -1,16 +1,18 @@
 // RUN: %clang_cc1 -E %s | FileCheck --strict-whitespace %s
+// RUN: %clang_cc1 -E -P -fnormalize-whitespace %s | FileCheck --strict-whitespace %s --check-prefix=NORMWS
 
 #define A(b) -#b  ,  - #b  ,  -# b  ,  - # b
 A()
 
 // CHECK: {{^}}-"" , - "" , -"" , - ""{{$}}
-
+// NORMWS: {{^}}-"",-"",-"",-""
 
 #define t(x) #x
 t(a
 c)
 
 // CHECK: {{^}}"a c"{{$}}
+// NORMWS-SAME: "a c"
 
 #define str(x) #x
 #define f(x) str(-x)
@@ -18,6 +20,7 @@
 1)
 
 // CHECK: {{^}}"-1"
+// NORMWS-SAME: "-1"
 
 #define paste(a,b) str(a&1 | FileCheck %s --strict-whitespace --check-prefix=NORMCOL
+// RUN: %clang_cc1 -E -C -fnormalize-whitespace %s 2>&1 | FileCheck %s --strict-whitespace --check-prefix=NORMCCOL
+// RUN: %clang_cc1 -E -P -fnormalize-whitespace %s 2>&1 | FileCheck %s --strict-whitespace --check-prefix=NORMWS
+// RUN: %clang_cc1 -E -C -P -fnormalize-whitespace %s 2>&1 | FileCheck %s --strict-whitespace --check-prefix=NORMCWS
+
+#define NOT_OMP  omp  something
+#define HASH #
+
+  int  a; /*  span-comment  */
+  int  b  ;   //  line-comment
+  _Pragma  (  "omp  barrier"  ) x //  more line-comments
+  #pragma  omp  nothing  //  another comment
+HASH  pragma  NOT_OMP
+  int  e;// again a line
+  int  f  ;
+
+
+// NORMCOL:  {{^}}# 9 "{{.*}}normalize-whitespace.c"{{$}}
+// NORMCOL:  {{^}}int a;{{$}}
+// NORMCOL-NEXT: {{^}}int b;{{$}}
+// NORMCOL-NEXT: {{^}}#pragma omp barrier{{$}}
+// NORMCOL-NEXT: # 11 "{{.*}}normalize-whitespace.c"
+// NORMCOL-NEXT: {{^}}x{{$}}
+// NORMCOL-NEXT: {{^}}#pragma omp nothing{{$}}
+// NORMCOL-NEXT: {{^ }}#pragma omp something{{$}}
+// NORMCOL-NEXT: {{^}}int e;{{$}}
+// NORMCOL-NEXT: