On 10/24/19 5:11 AM, Jakub Jelinek wrote:
Hi!

Jonathan has showed me a testcase with std::allocator<T>::{,de}allocate
and std::construct_at which FAILs with the current constexpr new
implementation.

There are two problems that make the testcase rejected, and further issues
(not addressed by this patch) where supposedly invalid C++20 code is
accepted.

The first problem was that cxx_replaceable_global_alloc_fn was actually
treating placement new as replaceable global allocation function, when it is
not.
The second problem is that std::construct_at uses placement new under the
hood.  From what I understood, placement new is not allowed in C++20 in
constexpr context and it is unspecified whatever magic std::construct_at
uses to get similar effect.

The fix for the first problem is easy, just add the
DECL_IS_REPLACEABLE_OPERATOR_NEW_P || DECL_IS_OPERATOR_DELETE_P
checks to cxx_replaceable_global_alloc_fn, placement new nor some random
user added ::operator new (size_t, float, double) etc. should not have that
set.
For the second one, the patch is allowing placement new in constexpr
evaluation only in std::construct_at and not in other functions.

With this, Jonathan's testcase works fine.  Ok for trunk if it passes
bootstrap/regtest?

OK.

Now, for the accepts invalid issues.
 From what I understood, in constant evaluation
::operator new{,[]} or ::operator delete{,[]}
can't be called from anywhere, only from new/delete expressions or
std::allocator<T>::{,de}allocate, is that correct?
If so, perhaps we need some CALL_EXPR flag that we'd set on the call
coming from new/delete expressions and disallow calls to
replaceable global allocator functions in constexpr evaluation unless
that flag is set or it is in std::allocator<T>::{,de}allocate.

Looks like there used to be a TREE_CALLS_NEW flag in TREE_LANG_FLAG_1, but that flag is now free for CALL_EXPR.

Or we could check whether the call is wrapped as expected by maybe_wrap_new_for_constexpr, but that doesn't help delete.

Or we could build NEW_EXPR and DELETE_EXPR in non-dependent context and lower them later.

Another thing is that even with that change,
   std::allocator<int> a;
   auto p = a.allocate (1);
   *p = 1;
   a.deallocate (p, 1);
would be accepted during constexpr evaluation, because allocate already
has the cast which turns "heap uninit" variable into "heap " and assigns
it a type, so there is nothing that will prevent the store from succeeding.

What's wrong with the store?

Any thoughts on what to do with that?  Even if the magic cast (perhaps
with some flag on it) is moved from whatever the first cast to non-void*
is to the placement new or start of corresponding constructor, would we
need to unmark it somehow if we say std::destroy_at it but allow
next std::construct_at to construct it again?

2019-10-24  Jakub Jelinek  <ja...@redhat.com>

        PR c++/91369 - Implement P0784R7: constexpr new
        * constexpr.c (cxx_replaceable_global_alloc_fn): Don't return true
        for placement new.
        (cxx_placement_new_fn, is_std_construct_at): New functions.
        (cxx_eval_call_expression): Allow placement new in std::construct_at.
        (potential_constant_expression_1): Likewise.

        * g++.dg/cpp2a/constexpr-new5.C: New test.

--- gcc/cp/constexpr.c.jj       2019-10-23 20:37:59.981872274 +0200
+++ gcc/cp/constexpr.c  2019-10-24 10:20:57.127193138 +0200
@@ -1601,7 +1601,41 @@ cxx_replaceable_global_alloc_fn (tree fn
  {
    return (cxx_dialect >= cxx2a
          && IDENTIFIER_NEWDEL_OP_P (DECL_NAME (fndecl))
-         && CP_DECL_CONTEXT (fndecl) == global_namespace);
+         && CP_DECL_CONTEXT (fndecl) == global_namespace
+         && (DECL_IS_REPLACEABLE_OPERATOR_NEW_P (fndecl)
+             || DECL_IS_OPERATOR_DELETE_P (fndecl)));
+}
+
+/* Return true if FNDECL is a placement new function that should be
+   useable during constant expression evaluation of std::construct_at.  */
+
+static inline bool
+cxx_placement_new_fn (tree fndecl)
+{
+  if (cxx_dialect >= cxx2a
+      && IDENTIFIER_NEW_OP_P (DECL_NAME (fndecl))
+      && CP_DECL_CONTEXT (fndecl) == global_namespace
+      && !DECL_IS_REPLACEABLE_OPERATOR_NEW_P (fndecl)
+      && TREE_CODE (TREE_TYPE (fndecl)) == FUNCTION_TYPE)
+    {
+      tree first_arg = TREE_CHAIN (TYPE_ARG_TYPES (TREE_TYPE (fndecl)));
+      if (TREE_VALUE (first_arg) == ptr_type_node
+         && TREE_CHAIN (first_arg) == void_list_node)
+       return true;
+    }
+  return false;
+}
+
+/* Return true if FNDECL is std::construct_at.  */
+
+static inline bool
+is_std_construct_at (tree fndecl)
+{
+  if (!decl_in_std_namespace_p (fndecl))
+    return false;
+
+  tree name = DECL_NAME (fndecl);
+  return name && id_equal (name, "construct_at");
  }
/* Subroutine of cxx_eval_constant_expression.
@@ -1738,6 +1772,27 @@ cxx_eval_call_expression (const constexp
              return t;
            }
        }
+      /* Allow placement new in std::construct_at, just return the second
+        argument.  */
+      if (cxx_placement_new_fn (fun)
+         && ctx->call
+         && ctx->call->fundef
+         && is_std_construct_at (ctx->call->fundef->decl))
+       {
+         const int nargs = call_expr_nargs (t);
+         tree arg1 = NULL_TREE;
+         for (int i = 0; i < nargs; ++i)
+           {
+             tree arg = CALL_EXPR_ARG (t, i);
+             arg = cxx_eval_constant_expression (ctx, arg, false,
+                                                 non_constant_p, overflow_p);
+             VERIFY_CONSTANT (arg);
+             if (i == 1)
+               arg1 = arg;
+           }
+         gcc_assert (arg1);
+         return arg1;
+       }
        if (!ctx->quiet)
        {
          if (!lambda_static_thunk_p (fun))
@@ -6448,7 +6503,11 @@ potential_constant_expression_1 (tree t,
                    && !fndecl_built_in_p (fun)
                    /* In C++2a, replaceable global allocation functions
                       are constant expressions.  */
-                   && !cxx_replaceable_global_alloc_fn (fun))
+                   && !cxx_replaceable_global_alloc_fn (fun)
+                   /* Allow placement new in std::construct_at.  */
+                   && (!cxx_placement_new_fn (fun)
+                       || current_function_decl == NULL_TREE
+                       || !is_std_construct_at (current_function_decl)))
                  {
                    if (flags & tf_error)
                      {
--- gcc/testsuite/g++.dg/cpp2a/constexpr-new5.C.jj      2019-10-24 
10:40:14.058475938 +0200
+++ gcc/testsuite/g++.dg/cpp2a/constexpr-new5.C 2019-10-24 10:44:25.678622415 
+0200
@@ -0,0 +1,79 @@
+// P0784R7
+// { dg-do compile { target c++2a } }
+
+namespace std
+{
+  typedef __SIZE_TYPE__ size_t;
+
+  template <typename T>
+  struct allocator
+  {
+    constexpr allocator () noexcept {}
+
+    constexpr T *allocate (size_t n)
+    { return static_cast<T *> (::operator new (n * sizeof(T))); }
+
+    constexpr void
+    deallocate (T *p, size_t n)
+    { ::operator delete (p); }
+  };
+
+  template <typename T, typename U = T &&>
+  U __declval (int);
+  template <typename T>
+  T __declval (long);
+  template <typename T>
+  auto declval () noexcept -> decltype (__declval<T> (0));
+
+  template <typename T>
+  struct remove_reference
+  { typedef T type; };
+  template <typename T>
+  struct remove_reference<T &>
+  { typedef T type; };
+  template <typename T>
+  struct remove_reference<T &&>
+  { typedef T type; };
+
+  template <typename T>
+  constexpr T &&
+  forward (typename std::remove_reference<T>::type &t) noexcept
+  { return static_cast<T&&> (t); }
+
+  template<typename T>
+  constexpr T &&
+  forward (typename std::remove_reference<T>::type &&t) noexcept
+  { return static_cast<T&&> (t); }
+
+  template <typename T, typename... A>
+  constexpr auto
+  construct_at (T *l, A &&... a)
+  noexcept (noexcept (::new ((void *) 0) T (std::declval<A> ()...)))
+  -> decltype (::new ((void *) 0) T (std::declval<A> ()...))
+  { return ::new ((void *) l) T (std::forward<A> (a)...); }
+
+  template <typename T>
+  constexpr inline void
+  destroy_at (T *l)
+  { l->~T (); }
+}
+
+inline void *operator new (std::size_t, void *p) noexcept
+{ return p; }
+
+constexpr bool
+foo ()
+{
+  std::allocator<int> a;
+  auto p = a.allocate (2);
+  std::construct_at (p, 1);
+  std::construct_at (p + 1, 2);
+  if (p[0] != 1 || p[1] != 2)
+    throw 1;
+  std::destroy_at (p);
+  std::destroy_at (p + 1);
+  a.deallocate (p, 2);
+  return true;
+}
+
+static_assert (foo ());

        Jakub


Reply via email to