public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Better error messages for merge-conflict markers (v3)
@ 2015-10-30 15:02 David Malcolm
  2015-11-02 22:52 ` Jeff Law
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: David Malcolm @ 2015-10-30 15:02 UTC (permalink / raw)
  To: gcc-patches; +Cc: David Malcolm

This is a rebased version of this patch from back in April:
  v2: https://gcc.gnu.org/ml/gcc-patches/2015-04/msg00944.html
which in turn is a rewrite of this one:
  v1: https://gcc.gnu.org/ml/gcc-patches/2015-03/msg01087.html

The idea is to more gracefully handle merger conflict markers
in the source code being compiled.  Specifically, in the C and
C++ frontends, if we're about to emit an error, see if the
source code is at a merger conflict marker, and if so, emit
a more specific message, so that the user gets this:

foo.c:1:1: error: source file contains patch conflict marker
 <<<<<<< HEAD
 ^

rather than this gobbledegook:

foo.c:1:1: error: expected identifier or '(' before '<<' token
 <<<<<<< HEAD
 ^

It's something of a "fit and finish" cosmetic item, but these
things add up.

Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu;
adds 82 new PASSes to g++.sum and 27 new PASSes to gcc.sum.

OK for trunk?

This implementation works by looking at the token stream, which
is slightly clunky; an alternative way of doing it would be to directly
look at the line the error occurs in and to see if it begins with a
conflict marker.  Would that be preferable?

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.
---
 gcc/c-family/c-common.h                            |  4 ++
 gcc/c-family/c-lex.c                               | 26 +++++++++++
 gcc/c/c-parser.c                                   | 53 +++++++++++++++++++++-
 gcc/cp/parser.c                                    | 34 ++++++++++++++
 .../c-c++-common/patch-conflict-markers-1.c        |  9 ++++
 .../c-c++-common/patch-conflict-markers-2.c        |  2 +
 .../c-c++-common/patch-conflict-markers-3.c        | 11 +++++
 .../c-c++-common/patch-conflict-markers-4.c        | 11 +++++
 .../c-c++-common/patch-conflict-markers-5.c        | 11 +++++
 .../c-c++-common/patch-conflict-markers-6.c        | 38 ++++++++++++++++
 .../c-c++-common/patch-conflict-markers-7.c        |  6 +++
 .../c-c++-common/patch-conflict-markers-8.c        |  4 ++
 .../c-c++-common/patch-conflict-markers-9.c        |  8 ++++
 gcc/testsuite/g++.dg/patch-conflict-markers-1.C    | 13 ++++++
 14 files changed, 228 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/patch-conflict-markers-1.c
 create mode 100644 gcc/testsuite/c-c++-common/patch-conflict-markers-2.c
 create mode 100644 gcc/testsuite/c-c++-common/patch-conflict-markers-3.c
 create mode 100644 gcc/testsuite/c-c++-common/patch-conflict-markers-4.c
 create mode 100644 gcc/testsuite/c-c++-common/patch-conflict-markers-5.c
 create mode 100644 gcc/testsuite/c-c++-common/patch-conflict-markers-6.c
 create mode 100644 gcc/testsuite/c-c++-common/patch-conflict-markers-7.c
 create mode 100644 gcc/testsuite/c-c++-common/patch-conflict-markers-8.c
 create mode 100644 gcc/testsuite/c-c++-common/patch-conflict-markers-9.c
 create mode 100644 gcc/testsuite/g++.dg/patch-conflict-markers-1.C

diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 4b5cac8..7ca9497 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1090,6 +1090,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 c69f4a6..6fc63eb 100644
--- a/gcc/c-family/c-lex.c
+++ b/gcc/c-family/c-lex.c
@@ -1269,3 +1269,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 c8c6a2d..a2d7ef6 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -181,8 +181,8 @@ 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.
@@ -471,6 +471,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
@@ -808,6 +822,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.
@@ -829,6 +867,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 7555bf3..6b6bd29 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -2632,6 +2632,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
@@ -2656,6 +2680,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
-- 
1.8.5.3

^ permalink raw reply	[flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-12-16 18:33 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-30 15:02 [PATCH] Better error messages for merge-conflict markers (v3) David Malcolm
2015-11-02 22:52 ` Jeff Law
2015-11-03  4:05   ` Trevor Saunders
2015-11-04 13:56 ` Bernd Schmidt
2015-12-09 16:39   ` [PATCH] Better error recovery for merge-conflict markers (v4) David Malcolm
2015-12-09 17:44     ` Bernd Schmidt
2015-12-09 20:18       ` Jeff Law
2015-12-16 18:23       ` David Malcolm
2015-12-15 19:11   ` [PATCH] Better error recovery for merge-conflict markers (v5) David Malcolm
2015-12-15 23:52     ` Bernd Schmidt
2015-12-16 18:33       ` David Malcolm
2015-12-09 23:50 ` [PATCH] Better error messages for merge-conflict markers (v3) Martin Sebor

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).