On Fri, 2015-03-20 at 17:50 +0000, Joseph Myers wrote:
> On Fri, 20 Mar 2015, David Malcolm wrote:
>
> > I believe that the presense of these markers in source code is almost
> > always a bug (are there any GCC frontends in which the markers are
> > parsable as something valid?)
>
> Well, obviously they are valid inside #if 0, strings (where you have a
> test, though not one at start of line "\
> <<<<<<<") and comments (where you don't have a test). They are also valid
> when stringized:
>
> #define str(s) #s
> const char *s = str(
> <<<<<<<
> );
>
> must be accepted. They are also valid in the expansion of a macro that
> doesn't get expanded.
>
> #define foo \
> <<<<<<<
>
> That is, in general, the invalidity only occurs when preprocessing tokens
> are converted to tokens.
>
> In C++ (C++11 and later), >>>>>>> can also close a sequence of nested
> template argument lists, thanks to the rule about replacing >> by > > in
> that context. And of course it's OK, if odd, to put that at the start of
> a line. So in that case the preprocessing tokens do get converted to
> tokens, and that token sequence (interpreted as >> >> >> > and then
> contextually adjusted to > > > > > > >) is valid.
Thanks.
It seems that the libcpp approach from my original attempt is untenable,
so I've rewritten the patch.
Attached is v2 of the patch, which instead does things in the C and C++
frontends.
Specifically, it adds special-case detection of patch conflict markers
to c_parser_error and cp_parser_error. It only affects places where we
were going to emit an error anyway, and checks to see if we have a patch
conflict marker. If we do, it simply changes the error message to
better describe the problem, eliminating the:
expected identifier or ‘(’ before ‘<<’ token
gobbledydook in favor of:
source file contains patch conflict marker
Hence this should not break any existing code, it should merely give
better error messages.
I added testcases to cover the situations you described in your mail.
Successfully bootstrapped®rtested on x86_64-unknown-linux-gnu
(Fedora 20), with:
27 new "PASS" results in gcc.sum
82 new "PASS" results in g++.sum
for the new test cases (relative to a control build of r221492).
OK for trunk? (for GCC 6)
gcc/c-family/ChangeLog:
* c-common.h (conflict_marker_get_final_tok_kind): New prototype.
* c-lex.c (conflict_marker_get_final_tok_kind): New function.
gcc/c/ChangeLog:
* c-parser.c (struct c_parser): Expand array "tokens_buf" from 2
to 4.
(c_parser_peek_nth_token): New function.
(c_parser_peek_conflict_marker): New function.
(c_parser_error): Detect patch conflict markers and report them as
such.
gcc/cp/ChangeLog:
* parser.c (cp_lexer_peek_conflict_marker): New function.
(cp_parser_error): Detect patch conflict markers and report them
as such.
gcc/testsuite/ChangeLog:
* c-c++-common/patch-conflict-markers-1.c: New testcase.
* c-c++-common/patch-conflict-markers-2.c: Likewise.
* c-c++-common/patch-conflict-markers-3.c: Likewise.
* c-c++-common/patch-conflict-markers-4.c: Likewise.
* c-c++-common/patch-conflict-markers-5.c: Likewise.
* c-c++-common/patch-conflict-markers-6.c: Likewise.
* c-c++-common/patch-conflict-markers-7.c: Likewise.
* c-c++-common/patch-conflict-markers-8.c: Likewise.
* c-c++-common/patch-conflict-markers-9.c: Likewise.
* g++.dg/patch-conflict-markers-1.C: Likewise.
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 5b2c5ab..383a4c7 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1064,6 +1064,10 @@ extern void c_genericize (tree);
extern int c_gimplify_expr (tree *, gimple_seq *, gimple_seq *);
extern tree c_build_bind_expr (location_t, tree, tree);
+/* In c-lex.c. */
+extern enum cpp_ttype
+conflict_marker_get_final_tok_kind (enum cpp_ttype tok1_kind);
+
/* In c-pch.c */
extern void pch_init (void);
extern void pch_cpp_save_state (void);
diff --git a/gcc/c-family/c-lex.c b/gcc/c-family/c-lex.c
index bb55be8..387f20e 100644
--- a/gcc/c-family/c-lex.c
+++ b/gcc/c-family/c-lex.c
@@ -1275,3 +1275,29 @@ lex_charconst (const cpp_token *token)
return value;
}
+
+/* Helper function for c_parser_peek_conflict_marker
+ and cp_lexer_peek_conflict_marker.
+ Given a possible patch conflict marker token of kind TOK1_KIND
+ consisting of a pair of characters, get the token kind for the
+ standalone final character. */
+
+enum cpp_ttype
+conflict_marker_get_final_tok_kind (enum cpp_ttype tok1_kind)
+{
+ switch (tok1_kind)
+ {
+ default: gcc_unreachable ();
+ case CPP_LSHIFT:
+ /* "<<" and '<' */
+ return CPP_LESS;
+
+ case CPP_EQ_EQ:
+ /* "==" and '=' */
+ return CPP_EQ;
+
+ case CPP_RSHIFT:
+ /* ">>" and '>' */
+ return CPP_GREATER;
+ }
+}
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 5cc3892..e510a99 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -197,8 +197,8 @@ typedef struct GTY(()) c_parser {
/* The look-ahead tokens. */
c_token * GTY((skip)) tokens;
/* Buffer for look-ahead tokens. */
- c_token tokens_buf[2];
- /* How many look-ahead tokens are available (0, 1 or 2, or
+ c_token tokens_buf[4];
+ /* How many look-ahead tokens are available (0 - 4, or
more if parsing from pre-lexed tokens). */
unsigned int tokens_avail;
/* True if a syntax error is being recovered from; false otherwise.
@@ -486,6 +486,20 @@ c_parser_peek_2nd_token (c_parser *parser)
return &parser->tokens[1];
}
+/* Return a pointer to the Nth token from PARSER, reading it
+ in if necessary. The N-1th token is already read in. */
+
+static c_token *
+c_parser_peek_nth_token (c_parser *parser, unsigned int n)
+{
+ if (parser->tokens_avail >= n)
+ return &parser->tokens[n - 1];
+ gcc_assert (parser->tokens_avail == n - 1);
+ c_lex_one_token (parser, &parser->tokens[n - 1]);
+ parser->tokens_avail = n;
+ return &parser->tokens[n - 1];
+}
+
/* Return true if TOKEN can start a type name,
false otherwise. */
static bool
@@ -823,6 +837,30 @@ c_parser_set_source_position_from_token (c_token *token)
}
}
+/* Helper function for c_parser_error.
+ Having peeked a token of kind TOK1_KIND that might signify
+ a patch conflict marker, peek successor tokens to determine
+ if we actually do have a patch conflict marker.
+ Specifically, we consider a run of 7 '<', '=' or '>' characters
+ as a patch conflict marker.
+ These come through the lexer as three pairs and a single,
+ e.g. three CPP_LSHIFT ("<<") and a CPP_LESS ('<'). */
+
+static bool
+c_parser_peek_conflict_marker (c_parser *parser, enum cpp_ttype tok1_kind)
+{
+ c_token *token2 = c_parser_peek_2nd_token (parser);
+ if (token2->type != tok1_kind)
+ return false;
+ c_token *token3 = c_parser_peek_nth_token (parser, 3);
+ if (token3->type != tok1_kind)
+ return false;
+ c_token *token4 = c_parser_peek_nth_token (parser, 4);
+ if (token4->type != conflict_marker_get_final_tok_kind (tok1_kind))
+ return false;
+ return true;
+}
+
/* Issue a diagnostic of the form
FILE:LINE: MESSAGE before TOKEN
where TOKEN is the next token in the input stream of PARSER.
@@ -844,6 +882,17 @@ c_parser_error (c_parser *parser, const char *gmsgid)
parser->error = true;
if (!gmsgid)
return;
+
+ /* If this is actually a patch conflict marker, report it as such. */
+ if (token->type == CPP_LSHIFT
+ || token->type == CPP_RSHIFT
+ || token->type == CPP_EQ_EQ)
+ if (c_parser_peek_conflict_marker (parser, token->type))
+ {
+ error_at (token->location, "source file contains patch conflict marker");
+ return;
+ }
+
/* This diagnostic makes more sense if it is tagged to the line of
the token we just peeked at. */
c_parser_set_source_position_from_token (token);
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index a18f38c..8bbab6c 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -2583,6 +2583,30 @@ cp_parser_is_keyword (cp_token* token, enum rid keyword)
return token->keyword == keyword;
}
+/* Helper function for cp_parser_error.
+ Having peeked a token of kind TOK1_KIND that might signify
+ a patch conflict marker, peek successor tokens to determine
+ if we actually do have a patch conflict marker.
+ Specifically, we consider a run of 7 '<', '=' or '>' characters
+ as a patch conflict marker.
+ These come through the lexer as three pairs and a single,
+ e.g. three CPP_LSHIFT tokens ("<<") and a CPP_LESS token ('<'). */
+
+static bool
+cp_lexer_peek_conflict_marker (cp_lexer *lexer, enum cpp_ttype tok1_kind)
+{
+ cp_token *token2 = cp_lexer_peek_nth_token (lexer, 2);
+ if (token2->type != tok1_kind)
+ return false;
+ cp_token *token3 = cp_lexer_peek_nth_token (lexer, 3);
+ if (token3->type != tok1_kind)
+ return false;
+ cp_token *token4 = cp_lexer_peek_nth_token (lexer, 4);
+ if (token4->type != conflict_marker_get_final_tok_kind (tok1_kind))
+ return false;
+ return true;
+}
+
/* If not parsing tentatively, issue a diagnostic of the form
FILE:LINE: MESSAGE before TOKEN
where TOKEN is the next token in the input stream. MESSAGE
@@ -2607,6 +2631,16 @@ cp_parser_error (cp_parser* parser, const char* gmsgid)
return;
}
+ /* If this is actually a patch conflict marker, report it as such. */
+ if (token->type == CPP_LSHIFT
+ || token->type == CPP_RSHIFT
+ || token->type == CPP_EQ_EQ)
+ if (cp_lexer_peek_conflict_marker (parser->lexer, token->type))
+ {
+ error_at (token->location, "source file contains patch conflict marker");
+ return;
+ }
+
c_parse_error (gmsgid,
/* Because c_parser_error does not understand
CPP_KEYWORD, keywords are treated like
diff --git a/gcc/testsuite/c-c++-common/patch-conflict-markers-1.c b/gcc/testsuite/c-c++-common/patch-conflict-markers-1.c
new file mode 100644
index 0000000..71e9fa7
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/patch-conflict-markers-1.c
@@ -0,0 +1,9 @@
+int p;
+
+<<<<<<< HEAD /* { dg-error "patch conflict marker" } */
+extern int some_var;
+======= /* { dg-error "patch conflict marker" } */
+extern short some_var; /* this line would lead to a warning */
+>>>>>>> Some commit message /* { dg-error "patch conflict marker" } */
+
+int q;
diff --git a/gcc/testsuite/c-c++-common/patch-conflict-markers-2.c b/gcc/testsuite/c-c++-common/patch-conflict-markers-2.c
new file mode 100644
index 0000000..79030ee
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/patch-conflict-markers-2.c
@@ -0,0 +1,2 @@
+/* This should not be flagged as a patch conflict marker. */
+const char *msg = "<<<<<<< ";
diff --git a/gcc/testsuite/c-c++-common/patch-conflict-markers-3.c b/gcc/testsuite/c-c++-common/patch-conflict-markers-3.c
new file mode 100644
index 0000000..be956b2
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/patch-conflict-markers-3.c
@@ -0,0 +1,11 @@
+/* Ensure we can handle unterminated conflict markers. */
+
+int p;
+
+<<<<<<< HEAD /* { dg-error "patch conflict marker" } */
+
+int q;
+
+<<<<<<< HEAD /* { dg-error "patch conflict marker" } */
+
+int r;
diff --git a/gcc/testsuite/c-c++-common/patch-conflict-markers-4.c b/gcc/testsuite/c-c++-common/patch-conflict-markers-4.c
new file mode 100644
index 0000000..ec3730c
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/patch-conflict-markers-4.c
@@ -0,0 +1,11 @@
+/* Ensure we can handle mismatched conflict markers. */
+
+int p;
+
+>>>>>>> Some commit message /* { dg-error "patch conflict marker" } */
+
+int q;
+
+>>>>>>> Some other commit message /* { dg-error "patch conflict marker" } */
+
+int r;
diff --git a/gcc/testsuite/c-c++-common/patch-conflict-markers-5.c b/gcc/testsuite/c-c++-common/patch-conflict-markers-5.c
new file mode 100644
index 0000000..816a97e
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/patch-conflict-markers-5.c
@@ -0,0 +1,11 @@
+/* Ensure we can handle mismatched conflict markers. */
+
+int p;
+
+======= /* { dg-error "patch conflict marker" } */
+
+int q;
+
+======= /* { dg-error "patch conflict marker" } */
+
+int r;
diff --git a/gcc/testsuite/c-c++-common/patch-conflict-markers-6.c b/gcc/testsuite/c-c++-common/patch-conflict-markers-6.c
new file mode 100644
index 0000000..74ea2d5
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/patch-conflict-markers-6.c
@@ -0,0 +1,38 @@
+/* Branch coverage of patch conflict marker detection:
+ none of these should be reported as patch conflict markers. */
+
+int a0;
+
+<< HEAD /* { dg-error "expected" } */
+
+int a1;
+
+<<<< HEAD /* { dg-error "expected" } */
+
+int a2;
+
+<<<<<< HEAD /* { dg-error "expected" } */
+
+int b0;
+
+== HEAD /* { dg-error "expected" } */
+
+int b1;
+
+==== HEAD /* { dg-error "expected" } */
+
+int b2;
+
+====== HEAD /* { dg-error "expected" } */
+
+int c0;
+
+>> HEAD /* { dg-error "expected" } */
+
+int c1;
+
+>>>> HEAD /* { dg-error "expected" } */
+
+int c2;
+
+>>>>>> HEAD /* { dg-error "expected" } */
diff --git a/gcc/testsuite/c-c++-common/patch-conflict-markers-7.c b/gcc/testsuite/c-c++-common/patch-conflict-markers-7.c
new file mode 100644
index 0000000..2b5d4e6
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/patch-conflict-markers-7.c
@@ -0,0 +1,6 @@
+/* It's valid to stringize the "<<<<<<<"; don't
+ report it as a patch conflict marker. */
+#define str(s) #s
+const char *s = str(
+<<<<<<<
+);
diff --git a/gcc/testsuite/c-c++-common/patch-conflict-markers-8.c b/gcc/testsuite/c-c++-common/patch-conflict-markers-8.c
new file mode 100644
index 0000000..90d75b0
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/patch-conflict-markers-8.c
@@ -0,0 +1,4 @@
+/* A macro that's never expanded shouldn't be reported as a patch
+ conflict marker. */
+#define foo \
+<<<<<<<
diff --git a/gcc/testsuite/c-c++-common/patch-conflict-markers-9.c b/gcc/testsuite/c-c++-common/patch-conflict-markers-9.c
new file mode 100644
index 0000000..5c1e663
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/patch-conflict-markers-9.c
@@ -0,0 +1,8 @@
+/* It's valid to have
+<<<<<<<
+ inside both
+ comments (as above), and within string literals. */
+const char *s = "\
+<<<<<<<";
+
+/* The above shouldn't be reported as errors. */
diff --git a/gcc/testsuite/g++.dg/patch-conflict-markers-1.C b/gcc/testsuite/g++.dg/patch-conflict-markers-1.C
new file mode 100644
index 0000000..ae19193
--- /dev/null
+++ b/gcc/testsuite/g++.dg/patch-conflict-markers-1.C
@@ -0,0 +1,13 @@
+/* Ensure that we don't complain about patch conflict markers on
+ valid template argument lists, valid in C++11 onwards. */
+// { dg-options "-std=c++11" }
+
+template <typename T>
+struct foo
+{
+ T t;
+};
+
+foo <foo <foo <foo <foo <foo <foo <int
+>>>>>>> f;
+// The above line is valid C++11, and isn't a patch conflict marker
