From patchwork Wed Feb 28 10:26:59 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 879170 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-474019-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="bt678sXf"; dkim-atps=neutral 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 3zrz485zBHz9s26 for ; Thu, 1 Mar 2018 01:49:41 +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:date :from:to:cc:subject:message-id:reply-to:references:mime-version :content-type:in-reply-to; q=dns; s=default; b=u5HhUlyL6WyHJjkD9 GaPUyFtoLoDZA7GSZWS0Ycy+wVfVz4zfA/+93Fyg3Zits+BQaDnoojDj8YmwWXWc DEaLrXNRB7TyFXnCddCzbT3GEzCZFcKIHxpqKno/dVsEdBhz09mOIPBzgvpDfYhz GpcxSrSircHtK9k9ljrm3antYc= 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:date :from:to:cc:subject:message-id:reply-to:references:mime-version :content-type:in-reply-to; s=default; bh=1pfm4IDr9gH+Wocc7WTaOk8 j/yg=; b=bt678sXfxUztLnPH6PkogZ2OTlI8cMm7oBr/dZFgPG9E2vo/kiNCLdo kDcIABfN+ol+gUvnyJAAJUmftQLFM+gNrBCJXrvIcxXTjVU2C72MesUY+tamyD5Y ODx/VHTnCJZaHGSPQKy01uv3BHjdsQXG/hz3B1eG2staZuWQpCVw= Received: (qmail 70344 invoked by alias); 28 Feb 2018 14:49:34 -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 70328 invoked by uid 89); 28 Feb 2018 14:49:33 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-11.6 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_2, GIT_PATCH_3, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx3-rdu2.redhat.com (HELO mx1.redhat.com) (66.187.233.73) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 28 Feb 2018 14:49:32 +0000 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 6811640FB647 for ; Wed, 28 Feb 2018 14:49:30 +0000 (UTC) Received: from tucnak.zalov.cz (ovpn-204-85.brq.redhat.com [10.40.204.85]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 94687AFD5A for ; Wed, 28 Feb 2018 14:49:29 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.15.2/8.15.2) with ESMTP id w1SAR1wH018574; Wed, 28 Feb 2018 11:27:01 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.15.2/8.15.2/Submit) id w1SAQxfQ018573; Wed, 28 Feb 2018 11:26:59 +0100 Date: Wed, 28 Feb 2018 11:26:59 +0100 From: Jakub Jelinek To: "Joseph S. Myers" , Jason Merrill , Martin Sebor Cc: Gcc Patch List Subject: [PATCH] Fix i18n in warn_spec_missing_attributes (PR 83871) Message-ID: <20180228102659.GL5867@tucnak> Reply-To: Jakub Jelinek References: <669cf511-5912-b862-c67d-2abe7a903c42@gmail.com> <2529778f-fae8-6d1c-b5b6-bce4feebf5b5@gmail.com> <7a48d97a-9fc7-82cc-2d5b-e2f88ddbda44@redhat.com> <20180227234411.GF5867@tucnak> <3781b346-fe6e-a391-aa1b-205b42e46001@gmail.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <3781b346-fe6e-a391-aa1b-205b42e46001@gmail.com> User-Agent: Mutt/1.9.1 (2017-09-22) X-IsSubscribed: yes On Tue, Feb 27, 2018 at 05:52:03PM -0700, Martin Sebor wrote: > > This is broken for multiple reasons: > > 1) it should be inform_n rather than inform > > 2) you really can't do what you're doing for translations; > > G_(...) marks the string for translations, but what actually is > > translated is not that string, but rather what is passed to inform, > > i.e. str.c_str (), so it will be likely never translated > > 3) as others have mentioned, the #include you are doing is > > wrong > > 4) I don't see justification to use std::string here > > > > What you IMHO should use instead is use > > pretty_printer str; > > instead, and the pp_* APIs to add stuff in there, including > > pp_begin_quote (&str, pp_show_color (global_dc->printer)) > > and > > pp_end_quote (&str, pp_show_color (global_dc->printer)) > > when you want to add what %< or %> expand to, > > and finally > > inform_n (DECL_SOURCE_LOCATION (tmpl), nattrs, > > "missing primary template attribute %s", > > "missing primary template attributes %s", > > pp_formatted_text (&str)); > > That way it should be properly translatable. > > Using inform_n() would not be correct here. What's being > translated is one of exactly two forms: singular and plural. > It doesn't matter how many things the plural form refers to > because the number doesn't appear in the message. Let's ask > Google to translate the message above to a language with more > than two plural forms, such as Czech: > > there are missing attributes: > https://translate.google.com/?tl=cs#auto/cs/there%20are%20missing%20attributes > > vs there are 5 missing attributes: > https://translate.google.com/?tl=cs#auto/cs/there%20are%205%20missing%20attributes > > Only the first form is correct when the exact number isn't > mentioned. I stand corrected on 1) (though wonder if there are locales which have different plurals based on the count in the list). > There are many places in the C++ front-end where a string > enclosed in G_() is assigned to a pointer and later used > in a diagnostic call. Is there something different about > the usage I introduced that makes it unsuitable for > translation? G_() does what it is designed for, collects a string for exgettext extraction into *.pot file. For most of the diagnostic calls we have arranged automatic extractions of the string literals passed directly to the functions, so we don't need to type it in most places, just where there is a conditional or the string is elsewhere. The problem in your code is that you mark using G_() for translation one string literal, e.g. G_("missing primary template attributes ") but then actually call inform with something different, like: "missing primary template attributes %, %" Functions inform will call will call gettext to translate that, but as this second string is dynamic and never in the *.pot file, nobody will translate it and so the inform will be always in English, even when some translator translates everything he is provided. And another advantage of using the %s appart from translatability is that it is -Wformat-security compatible. > std::string is used in a number of places in GCC. Why does > using it here need any special justification? Outside of config/*/* (aarch64, arm, nvptx, sh backends only) where the maintainers chose to use it, it is used just in a single file, ipa-chkp.c which is going away soon, all the diagnostics uses the pretty printers infrastructure instead. More importantly, with the latter you can do the begin/end quotes easily, with std::string you can't, unless you just use pretty printers and convert that to std::string, at which point it is certainly less efficient. > Using the pretty printer as you suggest also sounds > complicated to me and so prone to error but I will defer > to Jason's opinion to decide if any changes are necessary. Here it is in patch form, tested so far with: make check-c++-all RUNTESTFLAGS="--target_board=unix\{-m32,-m64\} dg.exp='attr*.C Wmissing-attrib*'" and waiting for bootstrap/regtest, ok for trunk if it passes? 2018-02-28 Jakub Jelinek PR c++/83871 PR c++/83503 * pt.c (INCLUDE_STRING): Remove define. (warn_spec_missing_attributes): Use pretty_printer instead of std::string. Fix up inform call so that the list of attributes is in %s argument. Jakub --- gcc/cp/pt.c.jj 2018-02-28 09:55:57.830897618 +0100 +++ gcc/cp/pt.c 2018-02-28 11:00:52.185543847 +0100 @@ -25,7 +25,6 @@ along with GCC; see the file COPYING3. file that contains only the method templates and "just win". */ #include "config.h" -#define INCLUDE_STRING #include "system.h" #include "coretypes.h" #include "cp-tree.h" @@ -2681,7 +2680,7 @@ warn_spec_missing_attributes (tree tmpl, template is declared with that the specialization is not, in case it's not apparent from the most recent declaration of the primary. */ unsigned nattrs = 0; - std::string str; + pretty_printer str; for (unsigned i = 0; i != sizeof blacklist / sizeof *blacklist; ++i) { @@ -2695,11 +2694,11 @@ warn_spec_missing_attributes (tree tmpl, if (lookup_attribute (blacklist[i], spec_attrs[k])) break; - if (str.size ()) - str += ", "; - str += "%<"; - str += blacklist[i]; - str += "%>"; + if (nattrs) + pp_string (&str, ", "); + pp_begin_quote (&str, pp_show_color (global_dc->printer)); + pp_string (&str, blacklist[i]); + pp_end_quote (&str, pp_show_color (global_dc->printer)); ++nattrs; } } @@ -2711,15 +2710,11 @@ warn_spec_missing_attributes (tree tmpl, if (warning_at (DECL_SOURCE_LOCATION (spec), OPT_Wmissing_attributes, "explicit specialization %q#D may be missing attributes", spec)) - { - if (nattrs > 1) - str = G_("missing primary template attributes ") + str; - else - str = G_("missing primary template attribute ") + str; - - inform (DECL_SOURCE_LOCATION (tmpl), str.c_str ()); - } - + inform (DECL_SOURCE_LOCATION (tmpl), + nattrs > 1 + ? G_("missing primary template attributes %s") + : G_("missing primary template attribute %s"), + pp_formatted_text (&str)); } /* Check to see if the function just declared, as indicated in