From: Nathan Sidwell <nathan@acm.org>
To: GCC Patches <gcc-patches@gcc.gnu.org>
Subject: [C++ PATCH] Remove namespace field
Date: Fri, 02 Jun 2017 11:13:00 -0000 [thread overview]
Message-ID: <1200bcba-e010-2b1a-40be-65f02153e614@acm.org> (raw)
[-- Attachment #1: Type: text/plain, Size: 1505 bytes --]
We currently chain namespace decls on a special chain in a binding
level. This patch drops that field and chains namespaces on the regular
names list.
There are 3 places we walk child namespaces.
1) spelling correction. Ironically this is the one search where the old
IDENTIFIER_GLOBAL_VALUES representation was perfect -- given an
identifier, tell me all the namespaces that have a binding. We didn't
take advantage of that though, but recursively walked the namespaces and
then doing a full-blown qualified-name lookup at each one. This patch
makes a few changes:
a) just look in the namespace of interest for a binding.
b) iterate over the name list to find namespace children. This is less
optimal, but we're in the error path anyway
c) implement a breadth-first walk of the children in declaration order.
Previously we were doing a depth-first walk. The difference is when the
search is limited. Breadth-first seemed more appropriate?
2 & 3) two ada-related walks. One of them we have to iterate the names
list, so we can just look for namespace children at that time. The
other case the worker function (in c-family), knows to skip namespace
decls already, but doesn't know how to walk them. So we have to insert
another walk here. If this is a pain point, the right solution is a
lang hook called by the worker function when it meets a namespace decl.
I bootstrapped including an ada compiler because of change #2 & #3.
Applied to trunk.
nathan
--
Nathan Sidwell
[-- Attachment #2: nspc-ect.diff --]
[-- Type: text/x-patch, Size: 10603 bytes --]
2017-06-02 Nathan Sidwell <nathan@acm.org>
* name-lookup.h (cp_binding_level): Lose namespaces field.
* name-lookup.c (add_decl_to_level): Chain namespaces on the names
list.
(suggest_alternatives_for): Adjust for namespace list. Do
breadth-first search.
* decl2.c (collect_source_refs): Namespaces are on the regulr
list.
(collect_ada_namespace): Likewise.
* g++.dg/pr45330.C: Adjust. Check breadth-firstness.
Index: cp/decl2.c
===================================================================
--- cp/decl2.c (revision 248820)
+++ cp/decl2.c (working copy)
@@ -4052,21 +4052,14 @@ cpp_check (tree t, cpp_operation op)
static void
collect_source_refs (tree namespc)
{
- tree t;
-
- if (!namespc)
- return;
-
/* Iterate over names in this name space. */
- for (t = NAMESPACE_LEVEL (namespc)->names; t; t = TREE_CHAIN (t))
- if (!DECL_IS_BUILTIN (t) )
+ for (tree t = NAMESPACE_LEVEL (namespc)->names; t; t = TREE_CHAIN (t))
+ if (DECL_IS_BUILTIN (t))
+ ;
+ else if (TREE_CODE (t) == NAMESPACE_DECL && !DECL_NAMESPACE_ALIAS (t))
+ collect_source_refs (t);
+ else
collect_source_ref (DECL_SOURCE_FILE (t));
-
- /* Dump siblings, if any */
- collect_source_refs (TREE_CHAIN (namespc));
-
- /* Dump children, if any */
- collect_source_refs (NAMESPACE_LEVEL (namespc)->namespaces);
}
/* Collect decls relevant to SOURCE_FILE from all namespaces recursively,
@@ -4075,17 +4068,16 @@ collect_source_refs (tree namespc)
static void
collect_ada_namespace (tree namespc, const char *source_file)
{
- if (!namespc)
- return;
-
- /* Collect decls from this namespace */
- collect_ada_nodes (NAMESPACE_LEVEL (namespc)->names, source_file);
-
- /* Collect siblings, if any */
- collect_ada_namespace (TREE_CHAIN (namespc), source_file);
+ tree decl = NAMESPACE_LEVEL (namespc)->names;
- /* Collect children, if any */
- collect_ada_namespace (NAMESPACE_LEVEL (namespc)->namespaces, source_file);
+ /* Collect decls from this namespace. This will skip
+ NAMESPACE_DECLs (both aliases and regular, it cannot tell). */
+ collect_ada_nodes (decl, source_file);
+
+ /* Now scan for namespace children, and dump them. */
+ for (; decl; decl = TREE_CHAIN (decl))
+ if (TREE_CODE (decl) == NAMESPACE_DECL && !DECL_NAMESPACE_ALIAS (decl))
+ collect_ada_namespace (decl, source_file);
}
/* Returns true iff there is a definition available for variable or
Index: cp/name-lookup.c
===================================================================
--- cp/name-lookup.c (revision 248820)
+++ cp/name-lookup.c (working copy)
@@ -115,38 +115,28 @@ add_decl_to_level (cp_binding_level *b,
{
gcc_assert (b->kind != sk_class);
- if (TREE_CODE (decl) == NAMESPACE_DECL && !DECL_NAMESPACE_ALIAS (decl))
- {
- /* Inner namespaces get their own chain, to make walking
- simpler. */
- DECL_CHAIN (decl) = b->namespaces;
- b->namespaces = decl;
- }
- else
- {
- /* Make sure we don't create a circular list. xref_tag can end
- up pushing the same artificial decl more than once. We
- should have already detected that in update_binding. */
- gcc_assert (b->names != decl);
-
- /* We build up the list in reverse order, and reverse it later if
- necessary. */
- TREE_CHAIN (decl) = b->names;
- b->names = decl;
-
- /* If appropriate, add decl to separate list of statics. We
- include extern variables because they might turn out to be
- static later. It's OK for this list to contain a few false
- positives. */
- if (b->kind == sk_namespace)
- if ((VAR_P (decl)
- && (TREE_STATIC (decl) || DECL_EXTERNAL (decl)))
- || (TREE_CODE (decl) == FUNCTION_DECL
- && (!TREE_PUBLIC (decl)
- || decl_anon_ns_mem_p (decl)
- || DECL_DECLARED_INLINE_P (decl))))
- vec_safe_push (static_decls, decl);
- }
+ /* Make sure we don't create a circular list. xref_tag can end
+ up pushing the same artificial decl more than once. We
+ should have already detected that in update_binding. */
+ gcc_assert (b->names != decl);
+
+ /* We build up the list in reverse order, and reverse it later if
+ necessary. */
+ TREE_CHAIN (decl) = b->names;
+ b->names = decl;
+
+ /* If appropriate, add decl to separate list of statics. We
+ include extern variables because they might turn out to be
+ static later. It's OK for this list to contain a few false
+ positives. */
+ if (b->kind == sk_namespace
+ && ((VAR_P (decl)
+ && (TREE_STATIC (decl) || DECL_EXTERNAL (decl)))
+ || (TREE_CODE (decl) == FUNCTION_DECL
+ && (!TREE_PUBLIC (decl)
+ || decl_anon_ns_mem_p (decl)
+ || DECL_DECLARED_INLINE_P (decl)))))
+ vec_safe_push (static_decls, decl);
}
/* Find the binding for NAME in the local binding level B. */
@@ -4708,70 +4698,74 @@ suggest_alternatives_for (location_t loc
bool suggest_misspellings)
{
vec<tree> candidates = vNULL;
- vec<tree> namespaces_to_search = vNULL;
- int max_to_search = PARAM_VALUE (CXX_MAX_NAMESPACES_FOR_DIAGNOSTIC_HELP);
- int n_searched = 0;
- tree t;
- unsigned ix;
-
- namespaces_to_search.safe_push (global_namespace);
-
- while (!namespaces_to_search.is_empty ()
- && n_searched < max_to_search)
- {
- tree scope = namespaces_to_search.pop ();
- name_lookup lookup (name, 0);
- cp_binding_level *level = NAMESPACE_LEVEL (scope);
-
- n_searched++;
-
- /* Look in this namespace. */
- if (qualified_namespace_lookup (scope, &lookup))
- candidates.safe_push (lookup.value);
-
- /* Add child namespaces. */
- for (t = level->namespaces; t; t = DECL_CHAIN (t))
- namespaces_to_search.safe_push (t);
- }
-
- /* If we stopped before we could examine all namespaces, inform the
- user. Do this even if we don't have any candidates, since there
- might be more candidates further down that we weren't able to
- find. */
- if (n_searched >= max_to_search
- && !namespaces_to_search.is_empty ())
- inform (location,
- "maximum limit of %d namespaces searched for %qE",
- max_to_search, name);
+ vec<tree> worklist = vNULL;
+ unsigned limit = PARAM_VALUE (CXX_MAX_NAMESPACES_FOR_DIAGNOSTIC_HELP);
+ bool limited = false;
+
+ /* Breadth-first search of namespaces. Up to limit namespaces
+ searched (limit zero == unlimited). */
+ worklist.safe_push (global_namespace);
+ for (unsigned ix = 0; ix != worklist.length (); ix++)
+ {
+ tree ns = worklist[ix];
+
+ if (tree value = ovl_skip_hidden (find_namespace_value (ns, name)))
+ candidates.safe_push (value);
+
+ if (!limited)
+ {
+ /* Look for child namespaces. We have to do this
+ indirectly because they are chained in reverse order,
+ which is confusing to the user. */
+ vec<tree> children = vNULL;
+
+ for (tree decl = NAMESPACE_LEVEL (ns)->names;
+ decl; decl = TREE_CHAIN (decl))
+ if (TREE_CODE (decl) == NAMESPACE_DECL
+ && !DECL_NAMESPACE_ALIAS (decl))
+ children.safe_push (decl);
- namespaces_to_search.release ();
-
- /* Nothing useful to report for NAME. Report on likely misspellings,
- or do nothing. */
- if (candidates.is_empty ())
- {
- if (suggest_misspellings)
- {
- const char *fuzzy_name = lookup_name_fuzzy (name, FUZZY_LOOKUP_NAME);
- if (fuzzy_name)
+ while (!limited && !children.is_empty ())
{
- gcc_rich_location richloc (location);
- richloc.add_fixit_replace (fuzzy_name);
- inform_at_rich_loc (&richloc, "suggested alternative: %qs",
- fuzzy_name);
+ if (worklist.length () == limit)
+ {
+ /* Unconditionally warn that the search was truncated. */
+ inform (location,
+ "maximum limit of %d namespaces searched for %qE",
+ limit, name);
+ limited = true;
+ }
+ else
+ worklist.safe_push (children.pop ());
}
+ children.release ();
}
- return;
}
+ worklist.release ();
- inform_n (location, candidates.length (),
- "suggested alternative:",
- "suggested alternatives:");
+ if (candidates.length ())
+ {
+ inform_n (location, candidates.length (),
+ "suggested alternative:",
+ "suggested alternatives:");
+ for (unsigned ix = 0; ix != candidates.length (); ix++)
+ {
+ tree val = candidates[ix];
- FOR_EACH_VEC_ELT (candidates, ix, t)
- inform (location_of (t), " %qE", t);
+ inform (location_of (val), " %qE", val);
+ }
+ candidates.release ();
+ }
+ else if (!suggest_misspellings)
+ ;
+ else if (const char *fuzzy = lookup_name_fuzzy (name, FUZZY_LOOKUP_NAME))
+ {
+ /* Show a spelling correction. */
+ gcc_rich_location richloc (location);
- candidates.release ();
+ richloc.add_fixit_replace (fuzzy);
+ inform_at_rich_loc (&richloc, "suggested alternative: %qs", fuzzy);
+ }
}
/* Subroutine of maybe_suggest_missing_header for handling unrecognized names
Index: cp/name-lookup.h
===================================================================
--- cp/name-lookup.h (revision 248820)
+++ cp/name-lookup.h (working copy)
@@ -188,9 +188,6 @@ struct GTY(()) cp_binding_level {
are wrapped in TREE_LISTs; the TREE_VALUE is the OVERLOAD. */
tree names;
- /* A chain of NAMESPACE_DECL nodes. */
- tree namespaces;
-
/* A list of USING_DECL nodes. */
tree usings;
Index: testsuite/g++.dg/pr45330.C
===================================================================
--- testsuite/g++.dg/pr45330.C (revision 248820)
+++ testsuite/g++.dg/pr45330.C (working copy)
@@ -1,17 +1,23 @@
// { dg-do compile }
-// Search std, __cxxabiv1, and global namespaces, plus one more.
-// { dg-options "--param cxx-max-namespaces-for-diagnostic-help=4" }
+// Search std, __cxxabiv1, and global namespaces, plus two more,
+// breadth first
-#define NSPACE(NAME) namespace NAME { int foo; }
+// { dg-options "--param cxx-max-namespaces-for-diagnostic-help=5" }
+
+// ::, std and __cxxabiv1
namespace A
{
int foo; // { dg-message "A::foo" "suggested alternative" }
+ namespace A0
+ {
+ int foo; // not me
+ }
}
namespace B
{
- int foo;
+ int foo; // { dg-message "B::foo" "suggested alternative" }
}
namespace C
@@ -32,6 +38,6 @@ namespace E
int bar()
{
return foo; // { dg-error "was not declared" }
- // { dg-message "maximum limit of 4 namespaces" "maximum limit" { target *-*-* } .-1 }
+ // { dg-message "maximum limit of 5 namespaces" "maximum limit" { target *-*-* } .-1 }
// { dg-message "suggested alternative" "suggested alternative" { target *-*-* } .-2 }
}
reply other threads:[~2017-06-02 11:13 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1200bcba-e010-2b1a-40be-65f02153e614@acm.org \
--to=nathan@acm.org \
--cc=gcc-patches@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).