From patchwork Fri Dec 5 00:06:31 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?TWFudWVsIEzDs3Blei1JYsOhw7Fleg==?= X-Patchwork-Id: 417965 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 9C12F1400D5 for ; Fri, 5 Dec 2014 11:07:23 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:in-reply-to:references:from:date:message-id :subject:to:cc:content-type:content-transfer-encoding; q=dns; s= default; b=rPGU9CP2X2UUAjE367mDR07IEabnuEfKtygG0hgnHv9/oLYiCofSi 2VYhldLAfc55+L+qCHyVfUI7HNBPKAirhD+NsVdtW8OIqX3KWDiMsUlijPPU8XVp VzGRSqIccU2NURfyFUCCelgORBs7U+BUOgNsta33aEwjq5CAvi9a9w= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:in-reply-to:references:from:date:message-id :subject:to:cc:content-type:content-transfer-encoding; s= default; bh=tgzFrhFU3jzHKxjXgUscORyRY2I=; b=shgV3wAM3QFQlcHYxo8U 8lkzLEY2CqOgaFpEHYx0A5ywSw3Ey0WxAf5fzsMTBwd/ArKKD29ksK5oUyACt518 qh0HIs1iM3Jnw6+hsSNhNIwniT5iavYXPa9AAGbRKOo0LZWOyOqKtNFgijXiKZB5 LIITH++BRK4AfCXa9bo0ilE= Received: (qmail 11239 invoked by alias); 5 Dec 2014 00:07:16 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 11229 invoked by uid 89); 5 Dec 2014 00:07:16 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-wg0-f44.google.com Received: from mail-wg0-f44.google.com (HELO mail-wg0-f44.google.com) (74.125.82.44) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Fri, 05 Dec 2014 00:07:14 +0000 Received: by mail-wg0-f44.google.com with SMTP id b13so24044910wgh.3 for ; Thu, 04 Dec 2014 16:07:11 -0800 (PST) X-Received: by 10.180.83.37 with SMTP id n5mr25211958wiy.83.1417738031714; Thu, 04 Dec 2014 16:07:11 -0800 (PST) MIME-Version: 1.0 Received: by 10.217.141.72 with HTTP; Thu, 4 Dec 2014 16:06:31 -0800 (PST) In-Reply-To: <87ppc1p18n.fsf@redhat.com> References: <87ppc1p18n.fsf@redhat.com> From: =?ISO-8859-1?Q?Manuel_L=F3pez=2DIb=E1=F1ez?= Date: Fri, 5 Dec 2014 01:06:31 +0100 Message-ID: Subject: Re: [PATCH obvious/diagnostics] Honor override_column when printing the caret To: Dodji Seketeli Cc: Gcc Patch List On 3 December 2014 at 09:48, Dodji Seketeli wrote: > Manuel López-Ibáñez writes: > >> libcpp uses diagnostic->override_column to give a custom column number >> to diagnostics. This is taken into account when building the prefix, >> but it was missing when placing the caret. >> >> Before: >> >> /home/manuel/override_column.c:1:4: warning: "/*" within comment [-Wcomment] >> /* /* */ >> ^ >> >> After: >> >> /home/manuel/override_column.c:1:4: warning: "/*" within comment [-Wcomment] >> /* /* */ >> ^ >> >> >> Committed as obvious in r218295. > > Thank you for this. Thinking about it a bit more, there is no reason to not encapsulate this logic into a function. I could have used a macro, but I guess we now prefer inline functions where possible (a member function would have been even better but since I wanted this to be obvious, I'll leave it for a future C++fication of diagnostics.c). Patch below. Bootstrapped and regression tested on x86_64-linux-gnu. Committed to trunk as obvious cleanup. > Thinking about it again, it would be nice to have a regression test for > this. At some point, I guess we should do something for regression' > tests about the placement of the caret. Oh well. That would be useful. Yet, if someone has the knowledge/time for that, I would strongly suggest to spend it on adding support for testing diagnostics from LTO: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62234 All the new shiny -Wodr warnings might be broken the day before release and nobody will notice. Cheers, Manuel. Index: gcc/diagnostic.c =================================================================== --- gcc/diagnostic.c (revision 218408) +++ gcc/diagnostic.c (revision 218409) @@ -260,6 +260,8 @@ #undef DEFINE_DIAGNOSTIC_KIND NULL }; + gcc_assert (diagnostic->kind < DK_LAST_DIAGNOSTIC_KIND); + const char *text = _(diagnostic_kind_text[diagnostic->kind]); const char *text_cs = "", *text_ce = ""; const char *locus_cs, *locus_ce; @@ -274,11 +276,7 @@ locus_cs = colorize_start (pp_show_color (pp), "locus"); locus_ce = colorize_stop (pp_show_color (pp)); - expanded_location s = expand_location_to_spelling_point (diagnostic->location); - if (diagnostic->override_column) - s.column = diagnostic->override_column; - gcc_assert (diagnostic->kind < DK_LAST_DIAGNOSTIC_KIND); - + expanded_location s = diagnostic_expand_location (diagnostic); return (s.file == NULL ? build_message_string ("%s%s:%s %s%s%s", locus_cs, progname, locus_ce, @@ -289,8 +287,8 @@ : context->show_column ? build_message_string ("%s%s:%d:%d:%s %s%s%s", locus_cs, s.file, s.line, s.column, locus_ce, text_cs, text, text_ce) - : build_message_string ("%s%s:%d:%s %s%s%s", locus_cs, s.file, s.line, locus_ce, - text_cs, text, text_ce)); + : build_message_string ("%s%s:%d:%s %s%s%s", locus_cs, s.file, s.line, + locus_ce, text_cs, text, text_ce)); } /* If LINE is longer than MAX_WIDTH, and COLUMN is not smaller than @@ -337,9 +335,7 @@ return; context->last_location = diagnostic->location; - s = expand_location_to_spelling_point (diagnostic->location); - if (diagnostic->override_column) - s.column = diagnostic->override_column; + s = diagnostic_expand_location (diagnostic); line = location_get_source_line (s, &line_width); if (line == NULL || s.column > line_width) return; Index: gcc/diagnostic.h =================================================================== --- gcc/diagnostic.h (revision 218408) +++ gcc/diagnostic.h (revision 218409) @@ -297,6 +297,18 @@ void diagnostic_file_cache_fini (void); +/* Expand the location of this diagnostic. Use this function for consistency. */ + +static inline expanded_location +diagnostic_expand_location (const diagnostic_info * diagnostic) +{ + expanded_location s + = expand_location_to_spelling_point (diagnostic->location); + if (diagnostic->override_column) + s.column = diagnostic->override_column; + return s; +} + /* Pure text formatting support functions. */ extern char *file_name_as_prefix (diagnostic_context *, const char *); Index: gcc/ChangeLog =================================================================== --- gcc/ChangeLog (revision 218408) +++ gcc/ChangeLog (revision 218409) @@ -1,3 +1,9 @@ +2014-12-05 Manuel López-Ibáñez + + * diagnostic.h (diagnostic_expand_location): New inline function. + * diagnostic.c (diagnostic_build_prefix): Use it. + (diagnostic_show_locus): Likewise. + 2014-12-04 H.J. Lu PR bootstrap/64189 Index: gcc/fortran/error.c =================================================================== --- gcc/fortran/error.c (revision 218408) +++ gcc/fortran/error.c (revision 218409) @@ -1143,10 +1143,7 @@ pretty_printer *pp = context->printer; const char *locus_cs = colorize_start (pp_show_color (pp), "locus"); const char *locus_ce = colorize_stop (pp_show_color (pp)); - expanded_location s = expand_location_to_spelling_point (diagnostic->location); - if (diagnostic->override_column) - s.column = diagnostic->override_column; - + expanded_location s = diagnostic_expand_location (diagnostic); return (s.file == NULL ? build_message_string ("%s%s:%s", locus_cs, progname, locus_ce ) : !strcmp (s.file, N_("")) Index: gcc/fortran/ChangeLog =================================================================== --- gcc/fortran/ChangeLog (revision 218408) +++ gcc/fortran/ChangeLog (revision 218409) @@ -1,5 +1,10 @@ 2014-12-05 Manuel López-Ibáñez + * error.c (gfc_diagnostic_build_locus_prefix): Use + diagnostic_expand_location. + +2014-12-05 Manuel López-Ibáñez + * scanner.c (gfc_next_char_literal): Use gfc_warning_now. (load_file): Use the line length as the column hint for linemap_line_start. Reserve a location for the highest column of