Re: [PATCH] Better error messages for merge-conflict markers (v3)

[Trevor Saunders <tbsaunde at tbsaunde dot org>]

On Mon, Nov 02, 2015 at 03:52:13PM -0700, Jeff Law wrote:
> On 10/30/2015 09:16 AM, David Malcolm wrote:
> >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.
> I'm still having a hard time seeing this one as all that useful.  It's not
> like it's terribly hard to see the <<<<<< HEAD and realize that there's a
> conflict marker mucking things up.

Its probably not that hard, but I do know people who really like this
sort of thing.  I would think it also becomes more useful when you have
fix it hints, or even allow the compiler to try to fix and continue.
Apparently clang does that?  That does actually sound useful if I can
avoid thinking about having left the conflict marker at all and have
tools deal with it for me.  So unless this is a lot of code which I
doubt it seems worth while to me.

Trev

> 
> I'm willing to step aside if other folks thing this is really useful and
> want to review the changes.
> 
> jeff
>