From patchwork Thu Jul 27 21:36:43 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Malcolm X-Patchwork-Id: 794572 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-459209-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="pksazHX7"; 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 3xJPYx70CMz9s78 for ; Fri, 28 Jul 2017 07:02:27 +1000 (AEST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id; q=dns; s=default; b=TpTxqfF6Is56 P3yzyD2bx4+bGFu3Bk6uDuD+n78oOzAZLfgOVw49KoV2PxBk8kkE5v0GoktKPzbA pFd+xxpzxYkqHQy4H69QnQEVQ13ryilV1rGfMANFatV9by6q8YgzuoGFhy61cqOI KzUAas08lV59aSsEv/SLXuTrlGbVGhY= 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:from :to:cc:subject:date:message-id; s=default; bh=ScN8EUvwPL/mjZapEC 87X3Ic5b4=; b=pksazHX7JcJqtP2E3PZkCG8MTS7djla2dAc7USjKOkhiG5viGH 7mZwHDXtBY3R+oTQOvZcY5M/Zxg0/Zq5aHSi0lx82c8NmH8en/YwRBeIFEFBN0RY 75wdRvEZCr/2+RGEeCII43erb74EIyLudfSQHg3f1wkufWuljF/f2qwBw= Received: (qmail 55530 invoked by alias); 27 Jul 2017 21:02: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 55502 invoked by uid 89); 27 Jul 2017 21:02:15 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=offer X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 27 Jul 2017 21:02:13 +0000 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 1AA9268E02 for ; Thu, 27 Jul 2017 21:02:12 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 1AA9268E02 Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=dmalcolm@redhat.com Received: from c64.redhat.com (ovpn-112-25.phx2.redhat.com [10.3.112.25]) by smtp.corp.redhat.com (Postfix) with ESMTP id 39A756046B; Thu, 27 Jul 2017 21:02:11 +0000 (UTC) From: David Malcolm To: gcc-patches@gcc.gnu.org Cc: David Malcolm Subject: [PATCH] C++: fix ordering of missing std #include suggestion (PR c++/81514) Date: Thu, 27 Jul 2017 17:36:43 -0400 Message-Id: <1501191403-11349-1-git-send-email-dmalcolm@redhat.com> X-IsSubscribed: yes PR c++/81514 reports a problem where g++.dg/lookup/missing-std-include-2.C fails on Solaris, offering the suggestion: error: 'string' is not a member of 'std' note: suggested alternative: 'sprintf' instead of the expected: error: 'string' is not a member of 'std' note: 'std::string' is defined in header ''; did you forget to '#include '? This is after a: #include suggest_alternative_in_explicit_scope currently works in two phases: (a) it attempts to look for misspellings within the explicitly-given namespace and suggests the best it finds (b) failing that, it then looks for well-known "std::" names and suggests a missing header This now seems the wrong way round to me; if the user has typed "std::string", a missing #include seems more helpful as a suggestion than attempting to look for misspellings. This patch reverses the ordering of (a) and (b) above, so that missing header hints for well-known std:: names are offered first, only then falling back to misspelling hints. The problem doesn't show up on my x86_64-pc-linux-gnu box, as the pertinent part of the #include appears to be equivalent to: extern int sprintf (char *dst, const char *format, ...); namespace std { using ::sprintf; } The "std::sprintf" thus examined within consider_binding_level is the same tree node as ::sprintf, and is rejected by: /* Skip anticipated decls of builtin functions. */ if (TREE_CODE (d) == FUNCTION_DECL && DECL_BUILT_IN (d) && DECL_ANTICIPATED (d)) continue; and so the name "sprintf" is never considered as a spell-correction for std::"string". Hence we're not issuing spelling corrections for aliases within a namespace for builtins from the global namespace; these are pre-created by cxx_builtin_function, which has: 4397 /* All builtins that don't begin with an '_' should additionally 4398 go in the 'std' namespace. */ 4399 if (name[0] != '_') 4400 { 4401 tree decl2 = copy_node(decl); 4402 push_namespace (std_identifier); 4403 builtin_function_1 (decl2, std_node, false); 4404 pop_namespace (); 4405 } I'm not sure why Solaris' decl of std::sprintf doesn't hit the reject path above. I was able to reproduce the behavior seen on Solaris on my Fedora box by using this: namespace std { extern int sprintf (char *dst, const char *format, ...); } which isn't rejected by the "Skip anticipated decls of builtin functions" test above, and hence sprintf is erroneously offered as a suggestion. The patch reworks the test case to work in the above way, to trigger the problem on Linux, and then fixes it by changing the order that the suggestions are tried in name-lookup.c. It introduces an "empty.h" since the testcase is also to verify that we suggest a good location for new #include directives relative to pre-existing #include directives. Successfully bootstrapped®rtested on x86_64-pc-linux-gnu. OK for trunk? gcc/cp/ChangeLog: PR c++/81514 * name-lookup.c (maybe_suggest_missing_header): Convert return type from void to bool; return true iff a suggestion was offered. (suggest_alternative_in_explicit_scope): Move call to maybe_suggest_missing_header to before use of best_match, and return true if the former offers a suggestion. gcc/testsuite/ChangeLog: PR c++/81514 * g++.dg/lookup/empty.h: New file. * g++.dg/lookup/missing-std-include-2.C: Replace include of stdio.h with empty.h and a declaration of a "std::sprintf" not based on a built-in. --- gcc/cp/name-lookup.c | 39 +++++++++++----------- gcc/testsuite/g++.dg/lookup/empty.h | 1 + .../g++.dg/lookup/missing-std-include-2.C | 11 ++++-- 3 files changed, 29 insertions(+), 22 deletions(-) create mode 100644 gcc/testsuite/g++.dg/lookup/empty.h diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c index cd7428a..49c4dea 100644 --- a/gcc/cp/name-lookup.c +++ b/gcc/cp/name-lookup.c @@ -4838,34 +4838,34 @@ get_std_name_hint (const char *name) return NULL; } -/* Subroutine of suggest_alternative_in_explicit_scope, for use when we have no - suggestions to offer. - If SCOPE is the "std" namespace, then suggest pertinent header - files for NAME. */ +/* If SCOPE is the "std" namespace, then suggest pertinent header + files for NAME at LOCATION. + Return true iff a suggestion was offered. */ -static void +static bool maybe_suggest_missing_header (location_t location, tree name, tree scope) { if (scope == NULL_TREE) - return; + return false; if (TREE_CODE (scope) != NAMESPACE_DECL) - return; + return false; /* We only offer suggestions for the "std" namespace. */ if (scope != std_node) - return; + return false; gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE); const char *name_str = IDENTIFIER_POINTER (name); const char *header_hint = get_std_name_hint (name_str); - if (header_hint) - { - gcc_rich_location richloc (location); - maybe_add_include_fixit (&richloc, header_hint); - inform_at_rich_loc (&richloc, - "% is defined in header %qs;" - " did you forget to %<#include %s%>?", - name_str, header_hint, header_hint); - } + if (!header_hint) + return false; + + gcc_rich_location richloc (location); + maybe_add_include_fixit (&richloc, header_hint); + inform_at_rich_loc (&richloc, + "% is defined in header %qs;" + " did you forget to %<#include %s%>?", + name_str, header_hint, header_hint); + return true; } /* Look for alternatives for NAME, an IDENTIFIER_NODE for which name @@ -4880,6 +4880,9 @@ suggest_alternative_in_explicit_scope (location_t location, tree name, /* Resolve any namespace aliases. */ scope = ORIGINAL_NAMESPACE (scope); + if (maybe_suggest_missing_header (location, name, scope)) + return true; + cp_binding_level *level = NAMESPACE_LEVEL (scope); best_match bm (name); @@ -4895,8 +4898,6 @@ suggest_alternative_in_explicit_scope (location_t location, tree name, fuzzy_name); return true; } - else - maybe_suggest_missing_header (location, name, scope); return false; } diff --git a/gcc/testsuite/g++.dg/lookup/empty.h b/gcc/testsuite/g++.dg/lookup/empty.h new file mode 100644 index 0000000..a057418 --- /dev/null +++ b/gcc/testsuite/g++.dg/lookup/empty.h @@ -0,0 +1 @@ +/* empty file for use by missing-std-include-2.C. */ diff --git a/gcc/testsuite/g++.dg/lookup/missing-std-include-2.C b/gcc/testsuite/g++.dg/lookup/missing-std-include-2.C index ae918f8..51c604a 100644 --- a/gcc/testsuite/g++.dg/lookup/missing-std-include-2.C +++ b/gcc/testsuite/g++.dg/lookup/missing-std-include-2.C @@ -6,7 +6,12 @@ /* This is padding (to avoid the generated patch containing DejaGnu directives). */ -#include +#include "empty.h" + +namespace std +{ + extern int sprintf (char *dst, const char *format, ...); +}; void test (void) { @@ -45,11 +50,11 @@ void test_2 (void) @@ -7,6 +7,8 @@ directives). */ - #include + #include "empty.h" +#include +#include - void test (void) + namespace std { { dg-end-multiline-output "" } #endif