On Thu, Sep 19, 2024 at 08:17:24AM +0200, Richard Biener wrote:
> On Wed, Sep 18, 2024 at 7:33 PM Jakub Jelinek <ja...@redhat.com> wrote:
> >
> > On Wed, Sep 18, 2024 at 06:17:58PM +0100, Richard Sandiford wrote:
> > > +1 I'd much rather learn about this kind of error before the code reaches
> > > a review tool :)
> > >
> > > >From a quick check, it doesn't look like Clang has this, so there is no
> > > existing name to follow.
> >
> > I was considering also -Wtrailing-whitespace, but
> > 1) git diff really warns just about trailing spaces/tabs, not form feeds or
> > vertical tabs
> > 2) gcc source contains tons of spots with form feed in it (though,
> > I think pretty much always as the sole character on a line).
> > And not really sure how people use vertical tabs in the source if at all.
> > Perhaps form feed could be not warned if at end of line if it isn't the sole
> > character on a line...
>
> Generally I like diagnosing this early. For the above I'd say
> -Wtrailing-whitespace=
> with a set of things to diagnose (and a sane default - just spaces and
> tabs - for
> -Wtrailiing-whitespace) would be nice. As for naming possibly follow the
> is{space,blank,cntrl} character classifications? If those are a good
> fit, that is.
Here is a patch which currently allows blank (' ' '\t') and space (' ' '\t'
'\f' '\v'), cntrl not yet added, not anything non-ASCII, but in theory could
be added later (though, non-ASCII would be just for inside of comments,
say non-breaking space etc. in the source is otherwise an error).
Bootstrapped/regtested on x86_64-linux and i686-linux.
2024-09-19 Jakub Jelinek <ja...@redhat.com>
libcpp/
* include/cpplib.h (struct cpp_options): Add
cpp_warn_trailing_whitespace member.
(enum cpp_warning_reason): Add CPP_W_TRAILING_WHITESPACE.
* internal.h (struct _cpp_line_note): Document 'W' line note.
* lex.cc (_cpp_clean_line): Add 'W' line note for trailing whitespace
except for trailing whitespace after backslash. Formatting fix.
(_cpp_process_line_notes): Emit -Wtrailing-whitespace diagnostics.
Formatting fixes.
(lex_raw_string): Clear type on 'W' notes.
gcc/
* doc/invoke.texi (Wtrailing-whitespace): Document.
gcc/c-family/
* c.opt (Wtrailing-whitespace=): New option.
(Wtrailing-whitespace): New alias.
gcc/testsuite/
* c-c++-common/cpp/Wtrailing-whitespace-1.c: New test.
* c-c++-common/cpp/Wtrailing-whitespace-2.c: New test.
* c-c++-common/cpp/Wtrailing-whitespace-3.c: New test.
* c-c++-common/cpp/Wtrailing-whitespace-4.c: New test.
* c-c++-common/cpp/Wtrailing-whitespace-5.c: New test.
--- libcpp/include/cpplib.h.jj 2024-09-13 16:09:32.690455174 +0200
+++ libcpp/include/cpplib.h 2024-09-19 16:59:09.674903649 +0200
@@ -594,6 +594,9 @@ struct cpp_options
/* True if -finput-charset= option has been used explicitly. */
bool cpp_input_charset_explicit;
+ /* -Wtrailing-whitespace= value. */
+ unsigned char cpp_warn_trailing_whitespace;
+
/* Dependency generation. */
struct
{
@@ -709,7 +712,8 @@ enum cpp_warning_reason {
CPP_W_EXPANSION_TO_DEFINED,
CPP_W_BIDIRECTIONAL,
CPP_W_INVALID_UTF8,
- CPP_W_UNICODE
+ CPP_W_UNICODE,
+ CPP_W_TRAILING_WHITESPACE
};
/* Callback for header lookup for HEADER, which is the name of a
--- libcpp/internal.h.jj 2024-09-18 09:45:36.832570227 +0200
+++ libcpp/internal.h 2024-09-19 16:54:56.610321817 +0200
@@ -318,8 +318,8 @@ struct _cpp_line_note
/* Type of note. The 9 'from' trigraph characters represent those
trigraphs, '\\' an escaped newline, ' ' an escaped newline with
- intervening space, 0 represents a note that has already been handled,
- and anything else is invalid. */
+ intervening space, 'W' trailing whitespace, 0 represents a note that
+ has already been handled, and anything else is invalid. */
unsigned int type;
};
--- libcpp/lex.cc.jj 2024-09-13 16:09:32.720454758 +0200
+++ libcpp/lex.cc 2024-09-19 16:58:37.434339128 +0200
@@ -928,7 +928,7 @@ _cpp_clean_line (cpp_reader *pfile)
if (p == buffer->next_line || p[-1] != '\\')
break;
- add_line_note (buffer, p - 1, p != d ? ' ': '\\');
+ add_line_note (buffer, p - 1, p != d ? ' ' : '\\');
d = p - 2;
buffer->next_line = p - 1;
}
@@ -943,6 +943,20 @@ _cpp_clean_line (cpp_reader *pfile)
}
}
}
+ done:
+ if (d > buffer->next_line
+ && CPP_OPTION (pfile, cpp_warn_trailing_whitespace))
+ switch (CPP_OPTION (pfile, cpp_warn_trailing_whitespace))
+ {
+ case 1:
+ if (ISBLANK (d[-1]))
+ add_line_note (buffer, d - 1, 'W');
+ break;
+ case 2:
+ if (IS_NVSPACE (d[-1]) && d[-1])
+ add_line_note (buffer, d - 1, 'W');
+ break;
+ }
}
else
{
@@ -955,7 +969,6 @@ _cpp_clean_line (cpp_reader *pfile)
s++;
}
- done:
*d = '\n';
/* A sentinel note that should never be processed. */
add_line_note (buffer, d + 1, '\n');
@@ -1013,13 +1026,23 @@ _cpp_process_line_notes (cpp_reader *pfi
if (note->type == '\\' || note->type == ' ')
{
- if (note->type == ' ' && !in_comment)
- cpp_error_with_line (pfile, CPP_DL_WARNING,
pfile->line_table->highest_line, col,
- "backslash and newline separated by space");
+ if (note->type == ' ')
+ {
+ if (!in_comment)
+ cpp_error_with_line (pfile, CPP_DL_WARNING,
+ pfile->line_table->highest_line, col,
+ "backslash and newline separated by "
+ "space");
+ else if (CPP_OPTION (pfile, cpp_warn_trailing_whitespace))
+ cpp_warning_with_line (pfile, CPP_W_TRAILING_WHITESPACE,
+ pfile->line_table->highest_line, col,
+ "trailing whitespace");
+ }
if (buffer->next_line > buffer->rlimit)
{
- cpp_error_with_line (pfile, CPP_DL_PEDWARN,
pfile->line_table->highest_line, col,
+ cpp_error_with_line (pfile, CPP_DL_PEDWARN,
+ pfile->line_table->highest_line, col,
"backslash-newline at end of file");
/* Prevent "no newline at end of file" warning. */
buffer->next_line = buffer->rlimit;
@@ -1040,15 +1063,16 @@ _cpp_process_line_notes (cpp_reader *pfi
note->type,
(int) _cpp_trigraph_map[note->type]);
else
- {
- cpp_warning_with_line
- (pfile, CPP_W_TRIGRAPHS,
- pfile->line_table->highest_line, col,
- "trigraph ??%c ignored, use -trigraphs to enable",
- note->type);
- }
+ cpp_warning_with_line (pfile, CPP_W_TRIGRAPHS,
+ pfile->line_table->highest_line, col,
+ "trigraph ??%c ignored, use -trigraphs "
+ "to enable", note->type);
}
}
+ else if (note->type == 'W')
+ cpp_warning_with_line (pfile, CPP_W_TRAILING_WHITESPACE,
+ pfile->line_table->highest_line, col,
+ "trailing whitespace");
else if (note->type == 0)
/* Already processed in lex_raw_string. */;
else
@@ -2539,6 +2563,12 @@ lex_raw_string (cpp_reader *pfile, cpp_t
note->type = 0;
note++;
break;
+
+ case 'W':
+ /* Don't warn about trailing whitespace in raw string literals. */
+ note->type = 0;
+ note++;
+ break;
default:
gcc_checking_assert (_cpp_trigraph_map[note->type]);
--- gcc/doc/invoke.texi.jj 2024-09-12 18:15:20.458626277 +0200
+++ gcc/doc/invoke.texi 2024-09-19 17:18:14.730458180 +0200
@@ -413,7 +413,8 @@ Objective-C and Objective-C++ Dialects}.
-Wsuggest-attribute=@r{[}pure@r{|}const@r{|}noreturn@r{|}format@r{|}malloc@r{]}
-Wswitch -Wno-switch-bool -Wswitch-default -Wswitch-enum
-Wno-switch-outside-range -Wno-switch-unreachable -Wsync-nand
--Wsystem-headers -Wtautological-compare -Wtrampolines -Wtrigraphs
+-Wsystem-headers -Wtautological-compare -Wtrailing-whitespace
+-Wtrailing-whitespace=@var{kind} -Wtrampolines -Wtrigraphs
-Wtrivial-auto-var-init -Wno-tsan -Wtype-limits -Wundef
-Wuninitialized -Wunknown-pragmas
-Wunsuffixed-float-constants
@@ -8996,6 +8997,21 @@ will always be false.
This warning is enabled by @option{-Wall}.
+@opindex Wtrailing-whitespace
+@opindex Wno-trailing-whitespace
+@opindex Wtrailing-whitespace=
+@item -Wtrailing-whitespace
+@itemx -Wtrailing-whitespace=@var{kind}
+Warn about trailing whitespace at the end of lines, including inside of
+comments, but excluding trailing whitespace in raw string literals.
+@code{-Wtrailing-whitespace} is equivalent to
+@code{-Wtrailing-whitespace=blank} and warns just about trailing space and
+horizontal tab characters. @code{-Wtrailing-whitespace=space} warns about
+those or trailing form feed or vertical tab characters.
+@code{-Wno-trailing-whitespace} or @code{-Wtrailing-whitespace=none}
+disables the warning, which is the default.
+This is a coding style warning.
+
@opindex Wtrampolines
@opindex Wno-trampolines
@item -Wtrampolines
--- gcc/c-family/c.opt.jj 2024-09-12 18:15:20.415626861 +0200
+++ gcc/c-family/c.opt 2024-09-19 17:10:50.463448288 +0200
@@ -1450,6 +1450,26 @@ Wtraditional-conversion
C ObjC Var(warn_traditional_conversion) Warning
Warn of prototypes causing type conversions different from what would happen
in the absence of prototype.
+Enum
+Name(warn_trailing_whitespace_kind) Type(int) UnknownError(argument %qs to
%<-Wtrailing-whitespace=%> not recognized)
+
+EnumValue
+Enum(warn_trailing_whitespace_kind) String(none) Value(0)
+
+EnumValue
+Enum(warn_trailing_whitespace_kind) String(blank) Value(1)
+
+EnumValue
+Enum(warn_trailing_whitespace_kind) String(space) Value(2)
+
+Wtrailing-whitespace=
+C ObjC C++ ObjC++ CPP(cpp_warn_trailing_whitespace)
CppReason(CPP_W_TRAILING_WHITESPACE) Enum(warn_trailing_whitespace_kind) Joined
RejectNegative Var(warn_trailing_whitespace) Init(0) Warning
+Warn about trailing whitespace on lines except when in raw string literals.
+
+Wtrailing-whitespace
+C ObjC C++ ObjC++ Alias(Wtrailing-whitespace=,blank,none) Warning
+Warn about trailing whitespace on lines except when in raw string literals.
Equivalent to Wtrailing-whitespace=blank when enabled or
Wtrailing-whitespace=none when disabled.
+
Wtrigraphs
C ObjC C++ ObjC++ CPP(warn_trigraphs) CppReason(CPP_W_TRIGRAPHS)
Var(cpp_warn_trigraphs) Init(2) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
Warn if trigraphs are encountered that might affect the meaning of the program.
--- gcc/testsuite/c-c++-common/cpp/Wtrailing-whitespace-1.c.jj 2024-09-18
14:44:22.712636656 +0200
+++ gcc/testsuite/c-c++-common/cpp/Wtrailing-whitespace-1.c 2024-09-19
17:40:36.972655199 +0200
@@ -0,0 +1,37 @@
+/* { dg-do compile { target { c || c++11 } } } */
+/* { dg-options "-Wtrailing-whitespace" } */
+
+int i;
+/* { dg-warning "trailing whitespace" "" { target *-*-* } .-1 } */
+int j;
+/* { dg-warning "trailing whitespace" "" { target *-*-* } .-1 } */
+int \
+ k \
+ ;
+/* { dg-warning "backslash and newline separated by space" "" { target *-*-* }
.-3 } */
+/* { dg-warning "backslash and newline separated by space" "" { target *-*-* }
.-3 } */
+/* { dg-warning "trailing whitespace" "" { target *-*-* } .-3 } */
+
+
+
+/* { dg-warning "trailing whitespace" "" { target *-*-* } .-1 } */
+
+
+
+/* { dg-warning "trailing whitespace" "" { target *-*-* } .-1 } */
+const char *p = R"*|*(
+
+
+.
+)*|*";
+/* { dg-warning "trailing whitespace" "" { target *-*-* } .-1 } */
+// This is a comment with trailing whitespace
+/* { dg-warning "trailing whitespace" "" { target *-*-* } .-1 } */
+/* This is a comment with trailing whitespace
+*/
+/* { dg-warning "trailing whitespace" "" { target *-*-* } .-2 } */
+// This is a comment with trailing whitespace
+/* This is a comment with trailing whitespace
+*/
+/* { dg-warning "trailing whitespace" "" { target *-*-* } .+1 } */
+
\ No newline at end of file
--- gcc/testsuite/c-c++-common/cpp/Wtrailing-whitespace-2.c.jj 2024-09-19
17:31:57.169567809 +0200
+++ gcc/testsuite/c-c++-common/cpp/Wtrailing-whitespace-2.c 2024-09-19
17:40:43.709563785 +0200
@@ -0,0 +1,37 @@
+/* { dg-do compile { target { c || c++11 } } } */
+/* { dg-options "-Wtrailing-whitespace=blank" } */
+
+int i;
+/* { dg-warning "trailing whitespace" "" { target *-*-* } .-1 } */
+int j;
+/* { dg-warning "trailing whitespace" "" { target *-*-* } .-1 } */
+int \
+ k \
+ ;
+/* { dg-warning "backslash and newline separated by space" "" { target *-*-* }
.-3 } */
+/* { dg-warning "backslash and newline separated by space" "" { target *-*-* }
.-3 } */
+/* { dg-warning "trailing whitespace" "" { target *-*-* } .-3 } */
+
+
+
+/* { dg-warning "trailing whitespace" "" { target *-*-* } .-1 } */
+
+
+
+/* { dg-warning "trailing whitespace" "" { target *-*-* } .-1 } */
+const char *p = R"*|*(
+
+
+.
+)*|*";
+/* { dg-warning "trailing whitespace" "" { target *-*-* } .-1 } */
+// This is a comment with trailing whitespace
+/* { dg-warning "trailing whitespace" "" { target *-*-* } .-1 } */
+/* This is a comment with trailing whitespace
+*/
+/* { dg-warning "trailing whitespace" "" { target *-*-* } .-2 } */
+// This is a comment with trailing whitespace
+/* This is a comment with trailing whitespace
+*/
+/* { dg-warning "trailing whitespace" "" { target *-*-* } .+1 } */
+
\ No newline at end of file
--- gcc/testsuite/c-c++-common/cpp/Wtrailing-whitespace-3.c.jj 2024-09-19
17:32:20.467260617 +0200
+++ gcc/testsuite/c-c++-common/cpp/Wtrailing-whitespace-3.c 2024-09-19
17:40:50.284474568 +0200
@@ -0,0 +1,43 @@
+/* { dg-do compile { target { c || c++11 } } } */
+/* { dg-options "-Wtrailing-whitespace=space" } */
+
+int i;
+/* { dg-warning "trailing whitespace" "" { target *-*-* } .-1 } */
+int j;
+/* { dg-warning "trailing whitespace" "" { target *-*-* } .-1 } */
+int \
+ k \
+ ;
+/* { dg-warning "backslash and newline separated by space" "" { target *-*-* }
.-3 } */
+/* { dg-warning "backslash and newline separated by space" "" { target *-*-* }
.-3 } */
+/* { dg-warning "trailing whitespace" "" { target *-*-* } .-3 } */
+
+/* { dg-warning "trailing whitespace" "" { target *-*-* } .-1 } */
+
+/* { dg-warning "trailing whitespace" "" { target *-*-* } .-1 } */
+
+/* { dg-warning "trailing whitespace" "" { target *-*-* } .-1 } */
+
+/* { dg-warning "trailing whitespace" "" { target *-*-* } .-1 } */
+
+/* { dg-warning "trailing whitespace" "" { target *-*-* } .-1 } */
+
+/* { dg-warning "trailing whitespace" "" { target *-*-* } .-1 } */
+const char *p = R"*|*(
+
+
+.
+)*|*";
+/* { dg-warning "trailing whitespace" "" { target *-*-* } .-1 } */
+// This is a comment with trailing whitespace
+/* { dg-warning "trailing whitespace" "" { target *-*-* } .-1 } */
+/* This is a comment with trailing whitespace
+*/
+/* { dg-warning "trailing whitespace" "" { target *-*-* } .-2 } */
+// This is a comment with trailing whitespace
+/* { dg-warning "trailing whitespace" "" { target *-*-* } .-1 } */
+/* This is a comment with trailing whitespace
+*/
+/* { dg-warning "trailing whitespace" "" { target *-*-* } .-2 } */
+/* { dg-warning "trailing whitespace" "" { target *-*-* } .+1 } */
+
\ No newline at end of file
--- gcc/testsuite/c-c++-common/cpp/Wtrailing-whitespace-4.c.jj 2024-09-19
17:33:02.531705971 +0200
+++ gcc/testsuite/c-c++-common/cpp/Wtrailing-whitespace-4.c 2024-09-19
17:41:21.131056009 +0200
@@ -0,0 +1,28 @@
+/* { dg-do compile { target { c || c++11 } } } */
+/* { dg-options "-Wtrailing-whitespace=none" } */
+
+int i;
+int j;
+int \
+ k \
+ ;
+/* { dg-warning "backslash and newline separated by space" "" { target *-*-* }
.-3 } */
+/* { dg-warning "backslash and newline separated by space" "" { target *-*-* }
.-3 } */
+
+
+
+
+
+
+const char *p = R"*|*(
+
+
+.
+)*|*";
+// This is a comment with trailing whitespace
+/* This is a comment with trailing whitespace
+*/
+// This is a comment with trailing whitespace
+/* This is a comment with trailing whitespace
+*/
+
\ No newline at end of file
--- gcc/testsuite/c-c++-common/cpp/Wtrailing-whitespace-5.c.jj 2024-09-19
17:33:45.427140375 +0200
+++ gcc/testsuite/c-c++-common/cpp/Wtrailing-whitespace-5.c 2024-09-19
17:41:28.352958013 +0200
@@ -0,0 +1,28 @@
+/* { dg-do compile { target { c || c++11 } } } */
+/* { dg-options "-Wno-trailing-whitespace" } */
+
+int i;
+int j;
+int \
+ k \
+ ;
+/* { dg-warning "backslash and newline separated by space" "" { target *-*-* }
.-3 } */
+/* { dg-warning "backslash and newline separated by space" "" { target *-*-* }
.-3 } */
+
+
+
+
+
+
+const char *p = R"*|*(
+
+
+.
+)*|*";
+// This is a comment with trailing whitespace
+/* This is a comment with trailing whitespace
+*/
+// This is a comment with trailing whitespace
+/* This is a comment with trailing whitespace
+*/
+
\ No newline at end of file
Jakub
