Re: [PATCH] C++/DWARF : Add 'using' support

[Jason Merrill <jason at redhat dot com>]

On Thu, 11 Dec 2003 18:01:05 -0800, Devang Patel <dpatel@apple.com> wrote:

> !   debug_true_tree,		/* ignore_block */
> --- 382,406 ----
> !   debug_true_tree,		         /* ignore_block */

Don't reformat these lines.

> ! /* Returns the DIE for decl or aborts.  */
>   
>   static dw_die_ref
> ! force_type_die (tree type)
>   {
> !   dw_die_ref type_die;
>   
> !   dwarf2out_decl (type);
> !   type_die = lookup_decl_die (type);
> !   if (!type_die)
>       abort();
>   
> !   return type_die;
>   }

This is obviously wrong.

>         /* Otherwise we're emitting the primary DIE for this decl.  */
> !       else if (debug_info_level > DINFO_LEVEL_TERSE)
> --- 11976,12005 ----
>         /* Otherwise we're emitting the primary DIE for this decl.  */
> !       else 
> ! 	{ 
> ! 	  if (debug_info_level > DINFO_LEVEL_TERSE)

More gratuitous reformatting.

> +       /* Setup namespace for the variable.  */
> +       context_die = setup_namespace_context (decl, context_die);

This is probably left over from when you were working with Dan's patch.  It
doesn't belong.

> +   if (tag == DBG_IMPORTED_GLOBAL_DECL
> +       || tag == DBG_IMPORTED_DECL)
> +     {
> +       /* Check DECL_INITIAL for inlines. For example,
> +            extern void foo():
> +            ...
> +            using ::foo;  
> +          More informative comment about DECL_INITIAL check for extern is 
> + 	 in dwarf2out_decl() in this source file.  */
> +       if (DECL_INITIAL (decl) == NULL_TREE)
> +         return;
> +     }

So you're throwing away a using-declaration which refers to a function
which doesn't happen to have been defined yet?  That's wrong.  We discussed
this--yes, dwarf2out_decl wouldn't normally emit a DIE for this function,
but if we want to use it we need to force it out.

I suppose you put this here because force_decl_die uses dwarf2out_decl, so
it would end up aborting rather than emit the die you're looking for.  The
right thing to do is to fix force_decl_die to actually force out the die.

> +   /* To emit DW_TAG_Imported_module or DW_TAG_Imported_decl, we need two DIEs.

No capital 'I' in these TAG names.

> +   /* Use original type for TYPE_DECL.  */

That seems wrong.  If we're using a typedef, presumably we want the
imported declaration to have the same name.

> +       at_import_die = lookup_type_die (type);
> +       if (!at_import_die)
> + 	  at_import_die = force_type_die (type);

This is redundant.  Just call force_type_die.

> +       at_import_die = lookup_decl_die (decl);
> +       if (!at_import_die)

Likewise.

> + 	  if (TREE_CODE (decl) == FUNCTION_DECL && DECL_INLINE (decl))
> + 	    /* Inline function is still in defferend functions linst.
> + 	       We do not have any DIE to reference here. Ignore this
> + 	       using declaration for now.  This is FIXME.
> + 	       
> + 	       extern __inline void foo (int a)
> + 	       { ... }
> + 	       namespace A
> + 	       {
> + 	         using ::foo;   <--- We are here.
> + 	       }
> + 	    */
> + 	    return;

We can certainly force out something for foo.  Either a declaration or the
abstract instance.

> +   /* Get the scope die for decl context. Use comp_unit_die for global module
> +      or decl. If die is not found for non globals, force new die.  */
> +   if (tag == DBG_IMPORTED_GLOBAL_MODULE
> +       || tag == DBG_IMPORTED_GLOBAL_DECL)
> +     scope_die = comp_unit_die;

Lose the _GLOBAL_ variants.  Instead, pass NULL_TREE for context if the
using is at file scope.

> +       scope_die = lookup_decl_die (context);
> +       if (!scope_die)
> + 	scope_die = force_decl_die (context);

Again, just call force_decl_die.

> +   /* OK, now we have DIEs for decl as well as scope. Emit imported die.  */ 
> +   if (tag == DBG_IMPORTED_GLOBAL_MODULE 
> +       || tag == DBG_IMPORTED_MODULE)
> +     imported_die = new_die (DW_TAG_imported_module, scope_die, context);

Passing context as the "created for" argument seems odd here.  decl seems
more appropriate, but still wrong.  I'd just leave it null.

> +   else if (tag == DBG_IMPORTED_GLOBAL_DECL
> + 	   || tag == DBG_IMPORTED_DECL)
> +     imported_die = new_die (DW_TAG_imported_declaration, scope_die, context);

Ditto.

> ! 	 However we want to emit DIES to represent mere function declarations
> ! 	 if they are namespace members. While generating DIEs for 'using' decls 
> ! 	 in C++ we may need to force out DIE for functions whose definition is
> ! 	 not yet seen. At that point do not want to skip such functions.
> ! 	 We check DECL_CONTEXT here for this purpose.  */
> !       if (DECL_INITIAL (decl) == NULL_TREE && DECL_CONTEXT (decl) == NULL_TREE)
>   	return;

This is still wrong.  We still want to ignore declarations of
namespace-scope functions which are not used.  force_decl_die needs to
override this somehow.

> *************** cp_parser_qualified_namespace_specifier 
> *** 9181,9186 ****
> --- 9182,9226 ----
>     return cp_parser_namespace_name (parser);
>   }
>   
> + static void
> + cp_emit_debug_info_for_using (tree t);

The prototype goes at the top of the file.  And this function belongs in
name-lookup.c.

> + 
> + /* Emit debugging information for using declarations and directives.
> +    If input tree is overloaded fn then emit debug info for all 
> +    candidates.  */
> + 
> + static void
> + cp_emit_debug_info_for_using (tree t)
> + {
> +   enum dbg_module_or_decl tag;
> + 
> +   tag = (current_namespace == global_namespace ? 
> + 	 DBG_IMPORTED_GLOBAL_DECL : DBG_IMPORTED_DECL);
> + 
> +   if (really_overloaded_fn (t))
> +     {
> +       /* We have using Foo::bar and bar is overloaded.
> + 	 Emit debug info. for all candidates.  */
> +       tree f;
> +       for (f = t; f ; f = OVL_CHAIN (f))
> + 	(*debug_hooks->imported_module_or_decl) (OVL_FUNCTION (f), 
> + 						 current_namespace,
> + 						 tag);
> +     }
> +   else
> +     {
> +       /* Do not trust really_overloaded_fn(). For example ...
> +          void foo (int a) { ... } 
> +          namespace A { using ::foo; }
> +          namespace B { using A::foo; }
> +       */
> +       if (TREE_CODE (t) == OVERLOAD)
> + 	t = OVL_FUNCTION (t);
> + 
> +       (*debug_hooks->imported_module_or_decl) (t, current_namespace, tag);
> +     }
> + }

You can avoid this if/else by always looping using OVL_CURRENT and
OVL_NEXT.

> *************** cp_parser_using_declaration (cp_parser* 
> *** 9260,9265 ****
> --- 9300,9323 ----
>   						identifier));
>   	  /* Add it to the list of members in this class.  */
>   	  finish_member_declaration (decl);
> + 
> + 	  /* Emit debug info.  */
> + 	  if (TREE_CODE (decl) == USING_DECL)
> + 	    {
> + 	      tree r;
> + 	      r = lookup_qualified_name (DECL_INITIAL(decl) ,identifier, 
> + 					 false, false);
> + 	      if (r && TREE_CODE (r) != ERROR_MARK)
> + 		{
> + 		  r = BASELINK_FUNCTIONS (r);
> + 		  cp_emit_debug_info_for_using (r);
> + 		}
> + 	      else
> + 		{
> + 		  /* FIXME. Give up for now. Find another way to look up 
> + 		     templates.  */
> + 		}
> + 	    }

This is badly broken.  This section of code deals with class-scope usings,
and cp_emit_debug_info_for_using only handles namespace-scope usings.  As
it should--we need to handle class- and function-scope usings while
emitting debug info for the relevant class or function.

> --- 9334,9346 ----
>   	    do_local_using_decl (decl);
>   	  else
>   	    do_toplevel_using_decl (decl);
> + 
> + 	  /* Emit debug info.  */
> + 	  cp_emit_debug_info_for_using (decl);
>   	}

And here you're calling it for both function- and namespace-scope usings.
I think you want to move the call into do_toplevel_using_decl.

> *************** cp_parser_using_directive (cp_parser* pa
> *** 9315,9320 ****
> --- 9378,9390 ----
>     parse_using_directive (namespace_decl, attribs);
>     /* Look for the final `;'.  */
>     cp_parser_require (parser, CPP_SEMICOLON, "`;'");
> + 
> +   /* Emit debugging info.  */
> +   tag = (current_namespace == global_namespace ? 
> + 	 DBG_IMPORTED_GLOBAL_MODULE : DBG_IMPORTED_MODULE);
> +   (*debug_hooks->imported_module_or_decl) (namespace_decl,
> + 					   current_namespace,
> + 					   tag);

And I'd push this call down, too, perhaps into do_using_directive.
parser.c should avoid dealing with semantics as much as possible.

Jason