[PATCH 3/3] c++: Add locations to using_p OVERLOADs

Jason Merrill jason@redhat.com
Mon Jul 8 18:24:14 GMT 2024


On 7/6/24 10:13 PM, Nathaniel Shead wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> 
> I have also been working on a patch that uses the locations of
> using-decls in 'diagnose_name_conflict' and 'duplicate_decls' calls, but
> that will need a fair bit more work that I'll probaby put off for the
> meantime.  Otherwise, as far as I can tell there's no circumstance when
> modules-imported namespace-scope USING_DECLs have their locations
> printed (hence the lack of tests), so I'm happy to defer this patch
> until I've made something I can actually test.
> 
> An alternative approach here (rather than giving locations to OVERLOADs)
> would be to rewrite the modules handling to not using OVERLOADs
> internally for non-function usings, that way we can attach the location
> to those USING_DECLs.  I went this way because I felt having locations
> in OVL_USING_P overloads would be useful anyway, but happy to try that
> route instead if you're uncomfortable increasing the size of most
> OVERLOADs for something largely unused.

That sounds better to me.

> -- >8 --
> 
> This is used by module streaming to track locations of USING_DECLs (that
> are internally wrapped and unwrapped as OVERLOADs for consistency with
> function usings).  No testcases with this patch as there aren't any easy
> ways to actually cause a diagnostic that uses this information yet;
> that'll come in a later patch.
> 
> gcc/cp/ChangeLog:
> 
> 	* cp-tree.h (OVL_SOURCE_LOCATION): New.
> 	(struct tree_overload): Add loc field.
> 	(class ovl_iterator): New member function.
> 	* module.cc (depset::hash::add_binding_entity): Add parameter.
> 	Track locations of usings.
> 	(depset::hash::find_dependencies): Note locations of usings.
> 	(module_state::write_cluster): Write locations of usings.
> 	(module_state::read_cluster): Read locations of usings.
> 	* name-lookup.cc (ovl_iterator::source_location): New.
> 	(walk_module_binding): Add parameter to callback.  Provide
> 	locations of usings.
> 	* name-lookup.h (walk_module_binding): Add parameter to
> 	callback.
> 	* ptree.cc (cxx_print_xnode): Write using location.
> 	* tree.cc (ovl_insert): Initialise source location for usings.
> 	(lookup_maybe_add): Propagate source location for usings.
> 
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> ---
>   gcc/cp/cp-tree.h      |  6 ++++++
>   gcc/cp/module.cc      | 29 +++++++++++++++++++----------
>   gcc/cp/name-lookup.cc | 32 ++++++++++++++++++++++++++------
>   gcc/cp/name-lookup.h  |  3 ++-
>   gcc/cp/ptree.cc       |  6 ++++++
>   gcc/cp/tree.cc        |  2 ++
>   6 files changed, 61 insertions(+), 17 deletions(-)
> 
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index 8732b7dc71b..cc7c1947f9e 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -824,6 +824,10 @@ typedef struct ptrmem_cst * ptrmem_cst_t;
>   /* The name of the overload set.  */
>   #define OVL_NAME(NODE) DECL_NAME (OVL_FIRST (NODE))
>   
> +/* The source location of this OVL_USING_P overload.  */
> +#define OVL_SOURCE_LOCATION(NODE) \
> +  (((struct tree_overload*)OVERLOAD_CHECK (NODE))->loc)
> +
>   /* Whether this is a set of overloaded functions.  TEMPLATE_DECLS are
>      always wrapped in an OVERLOAD, so we don't need to check them
>      here.  */
> @@ -838,6 +842,7 @@ typedef struct ptrmem_cst * ptrmem_cst_t;
>   struct GTY(()) tree_overload {
>     struct tree_common common;
>     tree function;
> +  location_t loc;
>   };
>   
>   /* Iterator for a 1 dimensional overload.  Permits iterating over the
> @@ -895,6 +900,7 @@ class ovl_iterator {
>     }
>     bool purview_p () const;
>     bool exporting_p () const;
> +  location_t source_location () const;
>   
>    public:
>     tree remove_node (tree head)
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index f5966fc8c1c..5bb3e824acb 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -2581,7 +2581,7 @@ public:
>       void add_namespace_context (depset *, tree ns);
>   
>     private:
> -    static bool add_binding_entity (tree, WMB_Flags, void *);
> +    static bool add_binding_entity (tree, WMB_Flags, void *, location_t);
>   
>     public:
>       bool add_namespace_entities (tree ns, bitmap partitions);
> @@ -13122,7 +13122,8 @@ struct add_binding_data
>   /* Return true if we are, or contain something that is exported.  */
>   
>   bool
> -depset::hash::add_binding_entity (tree decl, WMB_Flags flags, void *data_)
> +depset::hash::add_binding_entity (tree decl, WMB_Flags flags, void *data_,
> +				  location_t loc)
>   {
>     auto data = static_cast <add_binding_data *> (data_);
>     decl = strip_using_decl (decl);
> @@ -13185,7 +13186,6 @@ depset::hash::add_binding_entity (tree decl, WMB_Flags flags, void *data_)
>   		{
>   		  if (!(flags & WMB_Hidden))
>   		    d->clear_hidden_binding ();
> -		  OVL_PURVIEW_P (d->get_entity ()) = true;
>   		  if (flags & WMB_Export)
>   		    OVL_EXPORT_P (d->get_entity ()) = true;
>   		  return bool (flags & WMB_Export);
> @@ -13229,6 +13229,7 @@ depset::hash::add_binding_entity (tree decl, WMB_Flags flags, void *data_)
>   	  OVL_PURVIEW_P (decl) = true;
>   	  if (flags & WMB_Export)
>   	    OVL_EXPORT_P (decl) = true;
> +	  OVL_SOURCE_LOCATION (decl) = loc;
>   	}
>   
>         depset *dep = data->hash->make_dependency
> @@ -13616,7 +13617,10 @@ depset::hash::find_dependencies (module_state *module)
>   	      dump.indent ();
>   	      walker.begin ();
>   	      if (current->get_entity_kind () == EK_USING)
> -		walker.tree_node (OVL_FUNCTION (decl));
> +		{
> +		  module->note_location (OVL_SOURCE_LOCATION (decl));
> +		  walker.tree_node (OVL_FUNCTION (decl));
> +		}
>   	      else if (TREE_VISITED (decl))
>   		/* A global tree.  */;
>   	      else if (item->get_entity_kind () == EK_NAMESPACE)
> @@ -15114,6 +15118,7 @@ module_state::write_cluster (elf_out *to, depset *scc[], unsigned size,
>   		depset *dep = b->deps[jx];
>   		tree bound = dep->get_entity ();
>   		unsigned flags = 0;
> +		location_t loc = UNKNOWN_LOCATION;
>   		if (dep->get_entity_kind () == depset::EK_USING)
>   		  {
>   		    tree ovl = bound;
> @@ -15126,6 +15131,7 @@ module_state::write_cluster (elf_out *to, depset *scc[], unsigned size,
>   		      flags |= cbf_using;
>   		    if (OVL_EXPORT_P (ovl))
>   		      flags |= cbf_export;
> +		    loc = OVL_SOURCE_LOCATION (ovl);
>   		  }
>   		else
>   		  {
> @@ -15140,6 +15146,8 @@ module_state::write_cluster (elf_out *to, depset *scc[], unsigned size,
>   		gcc_checking_assert (DECL_P (bound));
>   
>   		sec.i (flags);
> +		if (flags & cbf_using)
> +		  write_location (sec, loc);
>   		sec.tree_node (bound);
>   	      }
>   
> @@ -15281,6 +15289,10 @@ module_state::read_cluster (unsigned snum)
>   		    && (flags & (cbf_using | cbf_export)))
>   		  sec.set_overrun ();
>   
> +		location_t loc = UNKNOWN_LOCATION;
> +		if (flags & cbf_using)
> +		  loc = read_location (sec);
> +
>   		tree decl = sec.tree_node ();
>   		if (sec.get_overrun ())
>   		  break;
> @@ -15291,8 +15303,7 @@ module_state::read_cluster (unsigned snum)
>   		    if (type || !DECL_IMPLICIT_TYPEDEF_P (decl))
>   		      sec.set_overrun ();
>   
> -		    type = build_lang_decl_loc (UNKNOWN_LOCATION,
> -						USING_DECL,
> +		    type = build_lang_decl_loc (loc, USING_DECL,
>   						DECL_NAME (decl),
>   						NULL_TREE);
>   		    USING_DECL_DECLS (type) = decl;
> @@ -15313,10 +15324,7 @@ module_state::read_cluster (unsigned snum)
>   			if (decls)
>   			  sec.set_overrun ();
>   
> -			/* FIXME: Propagate the location of the using-decl
> -			   for use in diagnostics.  */
> -			decls = build_lang_decl_loc (UNKNOWN_LOCATION,
> -						     USING_DECL,
> +			decls = build_lang_decl_loc (loc, USING_DECL,
>   						     DECL_NAME (decl),
>   						     NULL_TREE);
>   			USING_DECL_DECLS (decls) = decl;
> @@ -15339,6 +15347,7 @@ module_state::read_cluster (unsigned snum)
>   			    OVL_PURVIEW_P (decls) = true;
>   			    if (flags & cbf_export)
>   			      OVL_EXPORT_P (decls) = true;
> +			    OVL_SOURCE_LOCATION (decls) = loc;
>   			  }
>   
>   			if (flags & cbf_hidden)
> diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
> index 96d7f938162..52c07e46d4f 100644
> --- a/gcc/cp/name-lookup.cc
> +++ b/gcc/cp/name-lookup.cc
> @@ -4229,6 +4229,17 @@ ovl_iterator::exporting_p () const
>     return OVL_EXPORT_P (ovl);
>   }
>   
> +/* The declared source location of this using.  */
> +
> +location_t
> +ovl_iterator::source_location () const
> +{
> +  gcc_checking_assert (using_p ());
> +  if (TREE_CODE (ovl) == USING_DECL)
> +    return DECL_SOURCE_LOCATION (ovl);
> +  return OVL_SOURCE_LOCATION (ovl);
> +}
> +
>   /* Given a namespace-level binding BINDING, walk it, calling CALLBACK
>      for all decls of the current module.  When partitions are involved,
>      decls might be mentioned more than once.   Return the accumulation of
> @@ -4236,7 +4247,8 @@ ovl_iterator::exporting_p () const
>   
>   unsigned
>   walk_module_binding (tree binding, bitmap partitions,
> -		     bool (*callback) (tree decl, WMB_Flags, void *data),
> +		     bool (*callback) (tree decl, WMB_Flags, void *data,
> +				       location_t loc),
>   		     void *data)
>   {
>     tree current = binding;
> @@ -4249,6 +4261,7 @@ walk_module_binding (tree binding, bitmap partitions,
>     if (tree type = MAYBE_STAT_TYPE (current))
>       {
>         WMB_Flags flags = WMB_None;
> +      location_t loc = UNKNOWN_LOCATION;
>         if (STAT_TYPE_HIDDEN_P (current))
>   	flags = WMB_Flags (flags | WMB_Hidden);
>         if (TREE_CODE (type) == USING_DECL)
> @@ -4258,8 +4271,9 @@ walk_module_binding (tree binding, bitmap partitions,
>   	    flags = WMB_Flags (flags | WMB_Purview);
>   	  if (DECL_MODULE_EXPORT_P (type))
>   	    flags = WMB_Flags (flags | WMB_Export);
> +	  loc = DECL_SOURCE_LOCATION (type);
>   	}
> -      count += callback (type, flags, data);
> +      count += callback (type, flags, data, loc);
>         decl_hidden = STAT_DECL_HIDDEN_P (current);
>       }
>   
> @@ -4270,6 +4284,7 @@ walk_module_binding (tree binding, bitmap partitions,
>         if (!(decl_hidden && DECL_IS_UNDECLARED_BUILTIN (*iter)))
>   	{
>   	  WMB_Flags flags = WMB_None;
> +	  location_t loc = UNKNOWN_LOCATION;
>   	  if (decl_hidden)
>   	    flags = WMB_Flags (flags | WMB_Hidden);
>   	  if (iter.using_p ())
> @@ -4279,8 +4294,9 @@ walk_module_binding (tree binding, bitmap partitions,
>   		flags = WMB_Flags (flags | WMB_Purview);
>   	      if (iter.exporting_p ())
>   		flags = WMB_Flags (flags | WMB_Export);
> +	      loc = iter.source_location ();
>   	    }
> -	  count += callback (*iter, flags, data);
> +	  count += callback (*iter, flags, data, loc);
>   	}
>         decl_hidden = false;
>       }
> @@ -4319,13 +4335,14 @@ walk_module_binding (tree binding, bitmap partitions,
>   		    WMB_Flags flags = WMB_None;
>   		    if (maybe_dups)
>   		      flags = WMB_Flags (flags | WMB_Dups);
> -		    count += callback (bind, flags, data);
> +		    count += callback (bind, flags, data, UNKNOWN_LOCATION);
>   		  }
>   		else if (STAT_HACK_P (bind) && MODULE_BINDING_PARTITION_P (bind))
>   		  {
>   		    if (tree btype = STAT_TYPE (bind))
>   		      {
>   			WMB_Flags flags = WMB_None;
> +			location_t loc = UNKNOWN_LOCATION;
>   			if (maybe_dups)
>   			  flags = WMB_Flags (flags | WMB_Dups);
>   			if (STAT_TYPE_HIDDEN_P (bind))
> @@ -4337,8 +4354,9 @@ walk_module_binding (tree binding, bitmap partitions,
>   			      flags = WMB_Flags (flags | WMB_Purview);
>   			    if (DECL_MODULE_EXPORT_P (btype))
>   			      flags = WMB_Flags (flags | WMB_Export);
> +			    loc = DECL_SOURCE_LOCATION (btype);
>   			  }
> -			count += callback (btype, flags, data);
> +			count += callback (btype, flags, data, loc);
>   		      }
>   		    bool part_hidden = STAT_DECL_HIDDEN_P (bind);
>   		    for (ovl_iterator iter (MAYBE_STAT_DECL (STAT_DECL (bind)));
> @@ -4350,6 +4368,7 @@ walk_module_binding (tree binding, bitmap partitions,
>   			  (!(part_hidden && DECL_IS_UNDECLARED_BUILTIN (*iter)));
>   
>   			WMB_Flags flags = WMB_None;
> +			location_t loc = UNKNOWN_LOCATION;
>   			if (maybe_dups)
>   			  flags = WMB_Flags (flags | WMB_Dups);
>   			if (part_hidden)
> @@ -4361,8 +4380,9 @@ walk_module_binding (tree binding, bitmap partitions,
>   			      flags = WMB_Flags (flags | WMB_Purview);
>   			    if (iter.exporting_p ())
>   			      flags = WMB_Flags (flags | WMB_Export);
> +			    loc = iter.source_location ();
>   			  }
> -			count += callback (*iter, flags, data);
> +			count += callback (*iter, flags, data, loc);
>   			part_hidden = false;
>   		      }
>   		  }
> diff --git a/gcc/cp/name-lookup.h b/gcc/cp/name-lookup.h
> index 5cf6ae6374a..8183cd50159 100644
> --- a/gcc/cp/name-lookup.h
> +++ b/gcc/cp/name-lookup.h
> @@ -499,7 +499,8 @@ enum WMB_Flags
>   };
>   
>   extern unsigned walk_module_binding (tree binding, bitmap partitions,
> -				     bool (*)(tree decl, WMB_Flags, void *data),
> +				     bool (*)(tree decl, WMB_Flags, void *data,
> +					      location_t loc),
>   				     void *data);
>   extern tree add_imported_namespace (tree ctx, tree name, location_t,
>   				    unsigned module,
> diff --git a/gcc/cp/ptree.cc b/gcc/cp/ptree.cc
> index 15e46752d01..4f7fc96cc74 100644
> --- a/gcc/cp/ptree.cc
> +++ b/gcc/cp/ptree.cc
> @@ -299,6 +299,12 @@ cxx_print_xnode (FILE *file, tree node, int indent)
>         print_node (file, "optype", BASELINK_OPTYPE (node), indent + 4);
>         break;
>       case OVERLOAD:
> +      if (location_t loc = OVL_SOURCE_LOCATION (node))
> +	{
> +	  expanded_location xloc = expand_location (loc);
> +	  indent_to (file, indent + 4);
> +	  fprintf (file, "%s:%d:%d", xloc.file, xloc.line, xloc.column);
> +	}
>         print_node (file, "function", OVL_FUNCTION (node), indent + 4);
>         print_node (file, "next", OVL_CHAIN (node), indent + 4);
>         break;
> diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
> index dfd4a3a948b..21d8e0de329 100644
> --- a/gcc/cp/tree.cc
> +++ b/gcc/cp/tree.cc
> @@ -2387,6 +2387,7 @@ ovl_insert (tree fn, tree maybe_ovl, int using_or_hidden)
>   	    OVL_PURVIEW_P (maybe_ovl) = true;
>   	  if (using_or_hidden > 2)
>   	    OVL_EXPORT_P (maybe_ovl) = true;
> +	  OVL_SOURCE_LOCATION (maybe_ovl) = input_location;
>   	}
>       }
>     else
> @@ -2532,6 +2533,7 @@ lookup_maybe_add (tree fns, tree lookup, bool deduping)
>   		  {
>   		    lookup = ovl_make (OVL_FUNCTION (fns), lookup);
>   		    OVL_USING_P (lookup) = true;
> +		    OVL_SOURCE_LOCATION (lookup) = OVL_SOURCE_LOCATION (fns);
>   		  }
>   		else
>   		  lookup = lookup_add (OVL_FUNCTION (fns), lookup);



More information about the Gcc-patches mailing list