diff mbox series

[pushed] c++: avoid initializer_list<string> [PR105838]

Message ID 20221208184111.1649145-1-jason@redhat.com
State New
Headers show
Series [pushed] c++: avoid initializer_list<string> [PR105838] | expand

Commit Message

Jason Merrill Dec. 8, 2022, 6:41 p.m. UTC
Tested x86_64-pc-linux-gnu, applying to trunk.

-- 8< --

When constructing a vector<string> from { "strings" }, first is built an
initializer_list<string>, which is then copied into the strings in the
vector.  But this is inefficient: better would be treat the { "strings" }
as a range and construct the strings in the vector directly from the
string-literals.  We can do this transformation for standard library
classes because we know the design patterns they follow.

	PR c++/105838

gcc/cp/ChangeLog:

	* call.cc (list_ctor_element_type): New.
	(braced_init_element_type): New.
	(has_non_trivial_temporaries): New.
	(maybe_init_list_as_array): New.
	(maybe_init_list_as_range): New.
	(build_user_type_conversion_1): Use maybe_init_list_as_range.
	* parser.cc (cp_parser_braced_list): Call
	recompute_constructor_flags.
	* cp-tree.h (find_temps_r): Declare.

gcc/testsuite/ChangeLog:

	* g++.dg/tree-ssa/initlist-opt1.C: New test.
---
 gcc/cp/cp-tree.h                              |   1 +
 gcc/cp/call.cc                                | 138 ++++++++++++++++++
 gcc/cp/parser.cc                              |   1 +
 gcc/testsuite/g++.dg/tree-ssa/initlist-opt1.C |  25 ++++
 4 files changed, 165 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/tree-ssa/initlist-opt1.C


base-commit: 1e1847612d7f169f82c985b0b3a5e3301d6fe999

Comments

Stephan Bergmann Dec. 14, 2022, 3:08 p.m. UTC | #1
On 12/8/22 19:41, Jason Merrill via Gcc-patches wrote:
> Tested x86_64-pc-linux-gnu, applying to trunk.

Bisecting shows this started to break

> $ cat test.cc
> #include <vector>
> template<typename> struct ConstCharArrayDetector;
> template<int N> struct ConstCharArrayDetector<char const[N]> { using Type = int; };
> struct OUString {
>     template<typename T> OUString(T &, typename ConstCharArrayDetector<T>::Type = 0);
> };
> std::vector<OUString> f() { return {""}; }

> $ g++ -fsyntax-only test.cc
> In file included from .../include/c++/13.0.0/vector:65,
>                  from test.cc:1:
> .../include/c++/13.0.0/bits/stl_uninitialized.h: In instantiation of ‘constexpr bool std::__check_constructible() [with _ValueType = OUString; _Tp = const char* const&]’:
> .../include/c++/13.0.0/bits/stl_uninitialized.h:182:4:   required from ‘_ForwardIterator std::uninitialized_copy(_InputIterator, _InputIterator, _ForwardIterator) [with _InputIterator = const char* const*; _ForwardIterator = OUString*]’
> .../include/c++/13.0.0/bits/stl_uninitialized.h:373:37:   required from ‘_ForwardIterator std::__uninitialized_copy_a(_InputIterator, _InputIterator, _ForwardIterator, allocator<_Tp>&) [with _InputIterator = const char* const*; _ForwardIterator = OUString*; _Tp = OUString]’
> .../include/c++/13.0.0/bits/stl_vector.h:1690:33:   required from ‘void std::vector<_Tp, _Alloc>::_M_range_initialize(_ForwardIterator, _ForwardIterator, std::forward_iterator_tag) [with _ForwardIterator = const char* const*; _Tp = OUString; _Alloc = std::allocator<OUString>]’
> .../include/c++/13.0.0/bits/stl_vector.h:706:23:   required from ‘std::vector<_Tp, _Alloc>::vector(_InputIterator, _InputIterator, const allocator_type&) [with _InputIterator = const char* const*; <template-parameter-2-2> = void; _Tp = OUString; _Alloc = std::allocator<OUString>; allocator_type = std::allocator<OUString>]’
> test.cc:7:39:   required from here
> .../include/c++/13.0.0/bits/stl_uninitialized.h:90:56: error: static assertion failed: result type must be constructible from input type
>    90 |       static_assert(is_constructible<_ValueType, _Tp>::value,
>       |                                                        ^~~~~
> .../include/c++/13.0.0/bits/stl_uninitialized.h:90:56: note: ‘std::integral_constant<bool, false>::value’ evaluates to false
diff mbox series

Patch

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 581ac2b1817..0d6c234b3b0 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7087,6 +7087,7 @@  extern void set_global_friend			(tree);
 extern bool is_global_friend			(tree);
 
 /* in init.cc */
+extern tree find_temps_r			(tree *, int *, void *);
 extern tree expand_member_init			(tree);
 extern void emit_mem_initializers		(tree);
 extern tree build_aggr_init			(tree, tree, int,
diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index 459e86b5f09..33b5e7f87f5 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -4154,6 +4154,134 @@  add_list_candidates (tree fns, tree first_arg,
 		  access_path, flags, candidates, complain);
 }
 
+/* Given C(std::initializer_list<A>), return A.  */
+
+static tree
+list_ctor_element_type (tree fn)
+{
+  gcc_checking_assert (is_list_ctor (fn));
+
+  tree parm = FUNCTION_FIRST_USER_PARMTYPE (fn);
+  parm = non_reference (TREE_VALUE (parm));
+  return TREE_VEC_ELT (CLASSTYPE_TI_ARGS (parm), 0);
+}
+
+/* If EXPR is a braced-init-list where the elements all decay to the same type,
+   return that type.  */
+
+static tree
+braced_init_element_type (tree expr)
+{
+  if (TREE_CODE (expr) == CONSTRUCTOR
+      && TREE_CODE (TREE_TYPE (expr)) == ARRAY_TYPE)
+    return TREE_TYPE (TREE_TYPE (expr));
+  if (!BRACE_ENCLOSED_INITIALIZER_P (expr))
+    return NULL_TREE;
+
+  tree elttype = NULL_TREE;
+  for (constructor_elt &e: CONSTRUCTOR_ELTS (expr))
+    {
+      tree type = TREE_TYPE (e.value);
+      type = type_decays_to (type);
+      if (!elttype)
+	elttype = type;
+      else if (!same_type_p (type, elttype))
+	return NULL_TREE;
+    }
+  return elttype;
+}
+
+/* True iff EXPR contains any temporaries with non-trivial destruction.
+
+   ??? Also ignore classes with non-trivial but no-op destruction other than
+   std::allocator?  */
+
+static bool
+has_non_trivial_temporaries (tree expr)
+{
+  auto_vec<tree*> temps;
+  cp_walk_tree_without_duplicates (&expr, find_temps_r, &temps);
+  for (tree *p : temps)
+    {
+      tree t = TREE_TYPE (*p);
+      if (!TYPE_HAS_TRIVIAL_DESTRUCTOR (t)
+	  && !is_std_allocator (t))
+	return true;
+    }
+  return false;
+}
+
+/* We're initializing an array of ELTTYPE from INIT.  If it seems useful,
+   return INIT as an array (of its own type) so the caller can initialize the
+   target array in a loop.  */
+
+static tree
+maybe_init_list_as_array (tree elttype, tree init)
+{
+  /* Only do this if the array can go in rodata but not once converted.  */
+  if (!CLASS_TYPE_P (elttype))
+    return NULL_TREE;
+  tree init_elttype = braced_init_element_type (init);
+  if (!init_elttype || !SCALAR_TYPE_P (init_elttype) || !TREE_CONSTANT (init))
+    return NULL_TREE;
+
+  tree first = CONSTRUCTOR_ELT (init, 0)->value;
+  if (TREE_CODE (init_elttype) == INTEGER_TYPE && null_ptr_cst_p (first))
+    /* Avoid confusion from treating 0 as a null pointer constant.  */
+    first = build1 (UNARY_PLUS_EXPR, init_elttype, first);
+  first = (perform_implicit_conversion_flags
+	   (elttype, first, tf_none, LOOKUP_IMPLICIT|LOOKUP_NO_NARROWING));
+  if (first == error_mark_node)
+    /* Let the normal code give the error.  */
+    return NULL_TREE;
+
+  /* Don't do this if the conversion would be constant.  */
+  first = maybe_constant_init (first);
+  if (TREE_CONSTANT (first))
+    return NULL_TREE;
+
+  /* We can't do this if the conversion creates temporaries that need
+     to live until the whole array is initialized.  */
+  if (has_non_trivial_temporaries (first))
+    return NULL_TREE;
+
+  init_elttype = cp_build_qualified_type (init_elttype, TYPE_QUAL_CONST);
+  tree arr = build_array_of_n_type (init_elttype, CONSTRUCTOR_NELTS (init));
+  return finish_compound_literal (arr, init, tf_none);
+}
+
+/* If we were going to call e.g. vector(initializer_list<string>) starting
+   with a list of string-literals (which is inefficient, see PR105838),
+   instead build an array of const char* and pass it to the range constructor.
+   But only do this for standard library types, where we can assume the
+   transformation makes sense.
+
+   Really the container classes should have initializer_list<U> constructors to
+   get the same effect more simply; this is working around that lack.  */
+
+static tree
+maybe_init_list_as_range (tree fn, tree expr)
+{
+  if (BRACE_ENCLOSED_INITIALIZER_P (expr)
+      && is_list_ctor (fn)
+      && decl_in_std_namespace_p (fn))
+    {
+      tree to = list_ctor_element_type (fn);
+      if (tree init = maybe_init_list_as_array (to, expr))
+	{
+	  tree begin = decay_conversion (TARGET_EXPR_SLOT (init), tf_none);
+	  tree nelts = array_type_nelts_top (TREE_TYPE (init));
+	  tree end = cp_build_binary_op (input_location, PLUS_EXPR, begin,
+					 nelts, tf_none);
+	  begin = cp_build_compound_expr (init, begin, tf_none);
+	  return build_constructor_va (init_list_type_node, 2,
+				       NULL_TREE, begin, NULL_TREE, end);
+	}
+    }
+
+  return NULL_TREE;
+}
+
 /* Returns the best overload candidate to perform the requested
    conversion.  This function is used for three the overloading situations
    described in [over.match.copy], [over.match.conv], and [over.match.ref].
@@ -4425,6 +4553,16 @@  build_user_type_conversion_1 (tree totype, tree expr, int flags,
       return cand;
     }
 
+  /* Maybe pass { } as iterators instead of an initializer_list.  */
+  if (tree iters = maybe_init_list_as_range (cand->fn, expr))
+    if (z_candidate *cand2
+	= build_user_type_conversion_1 (totype, iters, flags, tf_none))
+      if (cand2->viable == 1)
+	{
+	  cand = cand2;
+	  expr = iters;
+	}
+
   tree convtype;
   if (!DECL_CONSTRUCTOR_P (cand->fn))
     convtype = non_reference (TREE_TYPE (TREE_TYPE (cand->fn)));
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index e8a50904243..4798aae1fbb 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -25445,6 +25445,7 @@  cp_parser_braced_list (cp_parser* parser, bool* non_constant_p)
   location_t finish_loc = cp_lexer_peek_token (parser->lexer)->location;
   braces.require_close (parser);
   TREE_TYPE (initializer) = init_list_type_node;
+  recompute_constructor_flags (initializer);
 
   cp_expr result (initializer);
   /* Build a location of the form:
diff --git a/gcc/testsuite/g++.dg/tree-ssa/initlist-opt1.C b/gcc/testsuite/g++.dg/tree-ssa/initlist-opt1.C
new file mode 100644
index 00000000000..053317b59d8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-ssa/initlist-opt1.C
@@ -0,0 +1,25 @@ 
+// PR c++/105838
+// { dg-additional-options -fdump-tree-gimple }
+// { dg-do compile { target c++11 } }
+
+// Test that we do range-initialization from const char *.
+// { dg-final { scan-tree-dump {_M_range_initialize<const char\* const\*>} "gimple" } }
+
+#include <string>
+#include <vector>
+
+void g (const void *);
+
+void f (const char *p)
+{
+  std::vector<std::string> lst = {
+  "aahing", "aaliis", "aarrgh", "abacas", "abacus", "abakas", "abamps", "abands", "abased", "abaser", "abases", "abasia",
+  "abated", "abater", "abates", "abatis", "abator", "abattu", "abayas", "abbacy", "abbess", "abbeys", "abbots", "abcees",
+  "abdabs", "abduce", "abduct", "abears", "abeigh", "abeles", "abelia", "abends", "abhors", "abided", "abider", "abides",
+  "abject", "abjure", "ablate", "ablaut", "ablaze", "ablest", "ablets", "abling", "ablins", "abloom", "ablush", "abmhos",
+  "aboard", "aboded", "abodes", "abohms", "abolla", "abomas", "aboral", "abords", "aborne", "aborts", "abound", "abouts",
+  "aboves", "abrade", "abraid", "abrash", "abrays", "abrazo", "abrege", "abrins", "abroad", "abrupt", "abseil", "absent",
+  };
+
+  g(&lst);
+}