From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9353 invoked by alias); 13 Dec 2010 15:38:36 -0000 Received: (qmail 9058 invoked by uid 22791); 13 Dec 2010 15:38:35 -0000 X-SWARE-Spam-Status: No, hits=-2.1 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW X-Spam-Check-By: sourceware.org Received: from mail-bw0-f44.google.com (HELO mail-bw0-f44.google.com) (209.85.214.44) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 13 Dec 2010 15:38:29 +0000 Received: by bwz12 with SMTP id 12so6626705bwz.17 for ; Mon, 13 Dec 2010 07:38:27 -0800 (PST) Received: by 10.204.126.26 with SMTP id a26mr3685983bks.186.1292254706521; Mon, 13 Dec 2010 07:38:26 -0800 (PST) MIME-Version: 1.0 Received: by 10.204.100.130 with HTTP; Mon, 13 Dec 2010 07:38:06 -0800 (PST) In-Reply-To: <4D063157.3050901@gnu.org> References: <1291979498-1604-1-git-send-email-dodji@redhat.com> <1291979498-1604-5-git-send-email-dodji@redhat.com> <4D063157.3050901@gnu.org> From: =?ISO-8859-1?Q?Manuel_L=F3pez=2DIb=E1=F1ez?= Date: Mon, 13 Dec 2010 16:30:00 -0000 Message-ID: Subject: Re: [PATCH 3/6] Emit macro expansion related diagnostics To: Paolo Bonzini Cc: Dodji Seketeli , gcc-patches@gcc.gnu.org, tromey@redhat.com, joseph@codesourcery.com, gdr@integrable-solutions.net Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2010-12/txt/msg00995.txt.bz2 On 13 December 2010 15:44, Paolo Bonzini wrote: > On 12/10/2010 12:11 PM, Dodji Seketeli wrote: >> [dodji@adjoa gcc]$ ./cc1 -quiet -ftrack-macro-expansion test.c >> test.c: In function =91g=92: >> test.c:5:14: error: invalid operands to binary<< =A0(have =91double=92 a= nd =91int=92) >> In macro 'OPERATE' at test.c:2:9 >> =A0 =A0 =A0Expanded at test.c:5:3 >> In macro 'SHIFTL' at test.c:5:14 >> =A0 =A0 =A0Expanded at test.c:8:3 >> In macro 'MULT' at test.c:8:3 >> =A0 =A0 =A0Expanded at test.c:13:3 >> > > > or this shorter example: > > test.c:5:14: error: invalid operands to binary<< =A0(have =91double=92 an= d =91int=92) > test.c:2:9: note: while expanding macro 'OPERATE' > test.c:5:14: note: while expanding macro 'SHIFTL' > test.c:8:3: note: while expanding macro 'MULT' > test.c:13:3: note: expanded from here > This format is closer to what GCC currently prints for templates instantiations, however, in template instantiations, the context goes first and the error/warning goes last. The context is also not marked with "note:". See http://people.redhat.com/bkoz/diagnostics/diagnostics.html#template-instant= iate-1 and http://people.redhat.com/bkoz/diagnostics/diagnostics.html#9335 and other examples therein. See gcc/cp/error.c:print_instantiation_partial_context, which also handles eliding excessive number of instantiation contexts (too deep macro expansion). I am not saying that this has to be done in this patch series, but it should eventually be implemented, so why not copy what is already available? > or this that does not change the location compared to current GCC: > > test.c:13:3: error: invalid operands to binary<< =A0(have =91double=92 an= d =91int=92) > test.c:2:9: note: while expanding macro 'OPERATE' > test.c:5:14: note: while expanding macro 'SHIFTL' > test.c:8:3: note: while expanding macro 'MULT' Interestingly, this is closer to the format adopted by Clang (http://clang.llvm.org/diagnostics.html), which I guess just followed GCC. However, I don't see how changing the locations to the ones proposed by Dodji causes any kind of "havoc" for users. In fact, looking at the caret information of clang, the diagnostic would be clearer if the locations were exchanged, which is what Dodji proposes. I don't think gcc diagnostics can be used to do any kind of automatic rewriting. At most, it is parsed in order to present the location of errors/messages in a nicer way, and the new locations proposed by Dodji do not break that. The question should be what locations are clearer/more informative to the user. I think the new locations proposed by Dodji are more informative and they follow what g++ already does for template errors. In any case, I guess that if there is a flag to disable the tracking of macro expansions for memory concerns, the same flag disables the enhanced locations and reverts to the current status, no? No need for yet another separate flag to control the output. Cheers, Manuel.