We're rather lax about setting DECL_CONTEXT correctly on several classes of builtins. This is affecting modules. But in cleaning it up I noticed cxx_builtin_function & cxx_builtin_function_ext_scope * are doing very similar work (they're identical except for a boolean value). * through a worker that repeats work (builtin_function_1 repeats its analysis on the ::std version). * and the latter incorrectly pushes things intended for ::std into ::, (because it uses pushdecl_top_level (decl))

The bug I encountered was that the version pushed to :: had a NULL context (rather than FROB_CONTEXT (global_namespace), which is the translation unit decl).

This patch
* implements cxx_builtin_function_ext_scope in terms of cxx_builtin_function, by wrapping that call in a push_nested_namespace (global_namespace).
* folds builtin_function_1 into cxx_builtin_function
* applies the transforms to the incoming decl before cloning it
* uses copy_decl, a c++ decl-specific copy_node, which correctly duplicates the lang-specific data
* uses IDENTIFIER_LENGTH rather than strlen
* set DECL_CONTEXT to FROB_CONTEXT (current_namespace).

applying to trunk.

nathan
--
Nathan Sidwell
2019-10-17  Nathan Sidwell  <nat...@acm.org>

	* decl.c (builtin_function_1): Merge into ...
	(cxx_builtin_function): ... here.  Nadger the decl before maybe
	copying it.  Set the context.
	(cxx_builtin_function_ext_scope): Push to top level, then call
	cxx_builtin_function.

Index: decl.c
===================================================================
--- decl.c	(revision 277098)
+++ decl.c	(working copy)
@@ -72,5 +72,4 @@ static tree grokvardecl (tree, tree, tre
 static void check_static_variable_definition (tree, tree);
 static void record_unknown_type (tree, const char *);
-static tree builtin_function_1 (tree, tree, bool);
 static int member_function_or_else (tree, tree, enum overload_flags);
 static tree local_variable_p_walkfn (tree *, int *, void *);
@@ -4630,10 +4629,11 @@ cp_make_fname_decl (location_t loc, tree
 }
 
-static tree
-builtin_function_1 (tree decl, tree context, bool is_global)
-{
-  tree          id = DECL_NAME (decl);
-  const char *name = IDENTIFIER_POINTER (id);
+/* Install DECL as a builtin function at current (global) scope.
+   Return the new decl (if we found an existing version).  Also
+   installs it into ::std, if it's not '_*'.  */
 
+tree
+cxx_builtin_function (tree decl)
+{
   retrofit_lang_decl (decl);
 
@@ -4645,45 +4645,33 @@ builtin_function_1 (tree decl, tree cont
   DECL_VISIBILITY_SPECIFIED (decl) = 1;
 
-  DECL_CONTEXT (decl) = context;
-
-  /* A function in the user's namespace should have an explicit
-     declaration before it is used.  Mark the built-in function as
-     anticipated but not actually declared.  */
+  tree id = DECL_NAME (decl);
+  const char *name = IDENTIFIER_POINTER (id);
   if (name[0] != '_' || name[1] != '_')
+    /* In the user's namespace, it must be declared before use.  */
+    DECL_ANTICIPATED (decl) = 1;
+  else if (IDENTIFIER_LENGTH (id) > strlen ("___chk")
+	   && 0 != strncmp (name + 2, "builtin_", strlen ("builtin_"))
+	   && 0 == memcmp (name + IDENTIFIER_LENGTH (id) - strlen ("_chk"),
+			   "_chk", strlen ("_chk") + 1))
+    /* Treat __*_chk fortification functions as anticipated as well,
+       unless they are __builtin_*_chk.  */
     DECL_ANTICIPATED (decl) = 1;
-  else if (strncmp (name + 2, "builtin_", strlen ("builtin_")) != 0)
-    {
-      size_t len = strlen (name);
-
-      /* Treat __*_chk fortification functions as anticipated as well,
-	 unless they are __builtin_*.  */
-      if (len > strlen ("___chk")
-	  && memcmp (name + len - strlen ("_chk"),
-		     "_chk", strlen ("_chk") + 1) == 0)
-	DECL_ANTICIPATED (decl) = 1;
-    }
-
-  if (is_global)
-    return pushdecl_top_level (decl);
-  else
-    return pushdecl (decl);
-}
 
-tree
-cxx_builtin_function (tree decl)
-{
-  tree          id = DECL_NAME (decl);
-  const char *name = IDENTIFIER_POINTER (id);
   /* All builtins that don't begin with an '_' should additionally
      go in the 'std' namespace.  */
   if (name[0] != '_')
     {
-      tree decl2 = copy_node(decl);
+      tree std_decl = copy_decl (decl);
+
       push_namespace (std_identifier);
-      builtin_function_1 (decl2, std_node, false);
+      DECL_CONTEXT (std_decl) = FROB_CONTEXT (std_node);
+      pushdecl (std_decl);
       pop_namespace ();
     }
 
-  return builtin_function_1 (decl, NULL_TREE, false);
+  DECL_CONTEXT (decl) = FROB_CONTEXT (current_namespace);
+  decl = pushdecl (decl);
+
+  return decl;
 }
 
@@ -4697,18 +4685,9 @@ tree
 cxx_builtin_function_ext_scope (tree decl)
 {
+  push_nested_namespace (global_namespace);
+  decl = cxx_builtin_function (decl);
+  pop_nested_namespace (global_namespace);
 
-  tree          id = DECL_NAME (decl);
-  const char *name = IDENTIFIER_POINTER (id);
-  /* All builtins that don't begin with an '_' should additionally
-     go in the 'std' namespace.  */
-  if (name[0] != '_')
-    {
-      tree decl2 = copy_node(decl);
-      push_namespace (std_identifier);
-      builtin_function_1 (decl2, std_node, true);
-      pop_namespace ();
-    }
-
-  return builtin_function_1 (decl, NULL_TREE, true);
+  return decl;
 }
 

Reply via email to