diff mbox series

[C++] structure tag lookup.

Message ID 8cd2bcd7-3c2c-32d6-e3df-ead95b2403a5@acm.org
State New
Headers show
Series [C++] structure tag lookup. | expand

Commit Message

Nathan Sidwell June 4, 2019, 3:13 p.m. UTC 
lookup of a structure tag, via xref tag, is rather confused, and I found 
myself wandering into that code.  We lookup a tag on the binding scopes, 
then if that fails we try the innermost enclosing namespace.  /Then/ we 
see if we were allowed to look at the scope we found something in.

I've reimplemented this so we stop looking when we reach a scope we're 
not to look in.  It also neatly separates the local binding-scope chain 
walking from the namespace lookup, which is the goal I had.

applying to trunk.

nathan

Comments

Marek Polacek June 4, 2019, 3:20 p.m. UTC | #1 
On Tue, Jun 04, 2019 at 11:13:14AM -0400, Nathan Sidwell wrote:
> -  /* Type found, check if it is in the allowed scopes, ignoring cleanup
> -     and template parameter scopes.  */
> -  if (val)
> +  if (b->kind != sk_namespace)
> +    /* Look in non-namespace scopes.  */
> +    for (cxx_binding *iter = NULL;
> +	 (iter = outer_binding (name, iter, /*class_p=*/ true)); )
> +      {
> +	/* First check we're supposed to be looking in this scope --
> +	   if we're not, we're done.  */
> +	for (; b != iter->scope; b = b->level_chain)
> +	  if (!(b->kind == sk_cleanup
> +		|| b->kind == sk_template_parms
> +		|| b->kind == sk_function_parms
> +		|| (b->kind == sk_class
> +		    && scope == ts_within_enclosing_non_class)))
> +	    return NULL_TREE;

[...]

> +  /* Now check if we can look in namespace scope.  */
> +  for (; b->kind != sk_namespace; b = b->level_chain)
> +    if (!(b->kind == sk_cleanup
> +	  || b->kind == sk_template_parms
> +	  || b->kind == sk_function_parms
> +	  || (b->kind == sk_class
> +	      && scope == ts_within_enclosing_non_class)))
> +      return NULL_TREE;

Looks like we could break that out into a new predicate function?
Something like allowed_scope_p?

Marek
Nathan Sidwell June 5, 2019, 10:30 a.m. UTC | #2 
On 6/4/19 11:20 AM, Marek Polacek wrote:
> On Tue, Jun 04, 2019 at 11:13:14AM -0400, Nathan Sidwell wrote:

> [...]
> 
>> +  /* Now check if we can look in namespace scope.  */
>> +  for (; b->kind != sk_namespace; b = b->level_chain)
>> +    if (!(b->kind == sk_cleanup
>> +	  || b->kind == sk_template_parms
>> +	  || b->kind == sk_function_parms
>> +	  || (b->kind == sk_class
>> +	      && scope == ts_within_enclosing_non_class)))
>> +      return NULL_TREE;
> 
> Looks like we could break that out into a new predicate function?
> Something like allowed_scope_p?

Yeah, I thought about that, but didn't think it worth the effort.

nathan
diff mbox series

Patch

2019-06-04  Nathan Sidwell  <nathan@acm.org>

	* name-lookup.c (lookup_type_scope_1): Reimplement, handle local
	and namespace scopes separately.

Index: gcc/cp/name-lookup.c
===================================================================
--- gcc/cp/name-lookup.c	(revision 271906)
+++ gcc/cp/name-lookup.c	(working copy)
@@ -6461,79 +6461,64 @@  static tree
 lookup_type_scope_1 (tree name, tag_scope scope)
 {
-  cxx_binding *iter = NULL;
-  tree val = NULL_TREE;
-  cp_binding_level *level = NULL;
-
-  /* Look in non-namespace scope first.  */
-  if (current_binding_level->kind != sk_namespace)
-    iter = outer_binding (name, NULL, /*class_p=*/ true);
-  for (; iter; iter = outer_binding (name, iter, /*class_p=*/ true))
-    {
-      /* Check if this is the kind of thing we're looking for.
-	 If SCOPE is TS_CURRENT, also make sure it doesn't come from
-	 base class.  For ITER->VALUE, we can simply use
-	 INHERITED_VALUE_BINDING_P.  For ITER->TYPE, we have to use
-	 our own check.
-
-	 We check ITER->TYPE before ITER->VALUE in order to handle
-	   typedef struct C {} C;
-	 correctly.  */
-
-      if (qualify_lookup (iter->type, LOOKUP_PREFER_TYPES)
-	  && (scope != ts_current
-	      || LOCAL_BINDING_P (iter)
-	      || DECL_CONTEXT (iter->type) == iter->scope->this_entity))
-	val = iter->type;
-      else if ((scope != ts_current
-		|| !INHERITED_VALUE_BINDING_P (iter))
-	       && qualify_lookup (iter->value, LOOKUP_PREFER_TYPES))
-	val = iter->value;
-
-      if (val)
-	break;
-    }
-
-  /* Look in namespace scope.  */
-  if (val)
-    level = iter->scope;
-  else
-    {
-      tree ns = current_decl_namespace ();
-
-      if (tree *slot = find_namespace_slot (ns, name))
-	{
-	  /* If this is the kind of thing we're looking for, we're done.  */
-	  if (tree type = MAYBE_STAT_TYPE (*slot))
-	    if (qualify_lookup (type, LOOKUP_PREFER_TYPES))
-	      val = type;
-	  if (!val)
-	    {
-	      if (tree decl = MAYBE_STAT_DECL (*slot))
-		if (qualify_lookup (decl, LOOKUP_PREFER_TYPES))
-		  val = decl;
-	    }
-	  level = NAMESPACE_LEVEL (ns);
-	}
-    }
+  cp_binding_level *b = current_binding_level;
 
-  /* Type found, check if it is in the allowed scopes, ignoring cleanup
-     and template parameter scopes.  */
-  if (val)
+  if (b->kind != sk_namespace)
+    /* Look in non-namespace scopes.  */
+    for (cxx_binding *iter = NULL;
+	 (iter = outer_binding (name, iter, /*class_p=*/ true)); )
+      {
+	/* First check we're supposed to be looking in this scope --
+	   if we're not, we're done.  */
+	for (; b != iter->scope; b = b->level_chain)
+	  if (!(b->kind == sk_cleanup
+		|| b->kind == sk_template_parms
+		|| b->kind == sk_function_parms
+		|| (b->kind == sk_class
+		    && scope == ts_within_enclosing_non_class)))
+	    return NULL_TREE;
+
+	/* Check if this is the kind of thing we're looking for.  If
+	   SCOPE is TS_CURRENT, also make sure it doesn't come from
+	   base class.  For ITER->VALUE, we can simply use
+	   INHERITED_VALUE_BINDING_P.  For ITER->TYPE, we have to
+	   use our own check.
+
+	   We check ITER->TYPE before ITER->VALUE in order to handle
+	     typedef struct C {} C;
+	   correctly.  */
+	if (tree type = iter->type)
+	  if ((scope != ts_current
+	       || LOCAL_BINDING_P (iter)
+	       || DECL_CONTEXT (type) == iter->scope->this_entity)
+	      && qualify_lookup (iter->type, LOOKUP_PREFER_TYPES))
+	    return iter->type;
+
+	if ((scope != ts_current
+	     || !INHERITED_VALUE_BINDING_P (iter))
+	    && qualify_lookup (iter->value, LOOKUP_PREFER_TYPES))
+	  return iter->value;
+      }
+
+  /* Now check if we can look in namespace scope.  */
+  for (; b->kind != sk_namespace; b = b->level_chain)
+    if (!(b->kind == sk_cleanup
+	  || b->kind == sk_template_parms
+	  || b->kind == sk_function_parms
+	  || (b->kind == sk_class
+	      && scope == ts_within_enclosing_non_class)))
+      return NULL_TREE;
+
+  /* Look in the innermost namespace.  */
+  tree ns = b->this_entity;
+  if (tree *slot = find_namespace_slot (ns, name))
     {
-      cp_binding_level *b = current_binding_level;
-      while (b)
-	{
-	  if (level == b)
-	    return val;
-
-	  if (b->kind == sk_cleanup || b->kind == sk_template_parms
-	      || b->kind == sk_function_parms)
-	    b = b->level_chain;
-	  else if (b->kind == sk_class
-		   && scope == ts_within_enclosing_non_class)
-	    b = b->level_chain;
-	  else
-	    break;
-	}
+      /* If this is the kind of thing we're looking for, we're done.  */
+      if (tree type = MAYBE_STAT_TYPE (*slot))
+	if (qualify_lookup (type, LOOKUP_PREFER_TYPES))
+	  return type;
+
+      if (tree decl = MAYBE_STAT_DECL (*slot))
+	if (qualify_lookup (decl, LOOKUP_PREFER_TYPES))
+	  return decl;
     }