Issue of `if constexpr` with non-placement new

Question

I'm trying to write a utility that invokes either new T{...} or new T(...) based on whether T is an aggregate type. What I have reached so far is as follows. Note that I'm using a macro instead of a function template because of this issue.

#define MAKE(p, T, ...) \
  T* p; \
  if constexpr (::std::is_aggregate_v<T>) { \
    p = new T{__VA_ARGS__}; \
  } \
  else { \
    p = new T(__VA_ARGS__); \
  }

I tried to test it on gcc 7.2.0 with

struct pr_t {
  int a, b;
};

int main() {
  MAKE(p, pr_t, 1, 2);
}

The following error then occurred (live).

prog.cc: In function 'int main()':
prog.cc:9:26: error: new initializer expression list treated as compound expression [-fpermissive]
     p = new T(__VA_ARGS__); \
                          ^
prog.cc:17:3: note: in expansion of macro 'MAKE'
   MAKE(p, pr_t, 1, 2);
   ^~~~
prog.cc:9:26: warning: left operand of comma operator has no effect [-Wunused-value]
     p = new T(__VA_ARGS__); \
                          ^
prog.cc:17:3: note: in expansion of macro 'MAKE'
   MAKE(p, pr_t, 1, 2);
   ^~~~
prog.cc:9:26: error: no matching function for call to 'pr_t::pr_t(int)'
     p = new T(__VA_ARGS__); \
                          ^
prog.cc:17:3: note: in expansion of macro 'MAKE'
   MAKE(p, pr_t, 1, 2);
   ^~~~
prog.cc:12:8: note: candidate: pr_t::pr_t()
 struct pr_t {
        ^~~~
prog.cc:12:8: note:   candidate expects 0 arguments, 1 provided
prog.cc:12:8: note: candidate: constexpr pr_t::pr_t(const pr_t&)
prog.cc:12:8: note:   no known conversion for argument 1 from 'int' to 'const pr_t&'
prog.cc:12:8: note: candidate: constexpr pr_t::pr_t(pr_t&&)
prog.cc:12:8: note:   no known conversion for argument 1 from 'int' to 'pr_t&&'

The compiler talked about something wrong with p = new T(__VA_ARGS__);. But shouldn't it not be considered at all when ::std::is_aggregate_v<T> is true?

Note that a similar pattern using if constexpr with placement new has worked. Quoted from cppref example.

template<class T, class... Args>
T* construct(T* p, Args&&... args) {
    if constexpr(std::is_aggregate_v<T>) {
        return ::new (static_cast<void*>(p)) T{std::forward<Args>(args)...};
    }
    else {
        return ::new (static_cast<void*>(p)) T(std::forward<Args>(args)...);
    }
}

I guess something is special about the non-placement version?

Answer 1

This has nothing to do with placement vs non-placement new. The issue arises due to your expansion of __VA_ARGS__ in a macro instead of using perfect forwarding in a function. Here, the compiler still tries to compile (after expansion) p = new pr_t(1, 2); even though it will never be executed, which then fails. The reason for this is (as pointed out in the duplicate linked by Quentin) that discarded statements arising from if constexpr are only not instantiated in an enclosing template instantiation. As an easy demonstration, consider that this will fail as well:

#include <iostream>

struct pr_t {
  int a, b;
};

int main() {
  if constexpr (false) {
    pr_t* p = new pr_t(1, 2);
  }
}

If you use the non-placement new in a function analogously to the placement version, it works:

#include <iostream>
#include <memory>
#include <type_traits>

template<class T, class... Args>
std::unique_ptr<T> makeFunc(Args&&... args) {
  if constexpr (std::is_aggregate_v<T>) {
    return std::unique_ptr<T>(new T{std::forward<Args>(args)...});
  }
  else {
    return std::make_unique<T>(std::forward<Args>(args)...);
  }
}

struct Aggregate {
  int a, b;
};

class No_Aggregate {
  int a, b;
public:
  explicit No_Aggregate(int a, int b) : a{a}, b{b} {}
  auto get_a() {
    return a;
  }
  auto get_b() {
    return b;
  }
};

int main() {
  auto agg = makeFunc<Aggregate>(1, 2);
  std::cout << std::is_aggregate_v<Aggregate> << ' '
            << agg->a << ' ' << agg->b << '\n';
  auto n_agg = makeFunc<No_Aggregate>(3, 4);
  std::cout << std::is_aggregate_v<No_Aggregate> << ' ' 
            << n_agg->get_a() << ' ' << n_agg->get_b() << '\n';
}

Output:

1 1 2
0 3 4

Answer 2

It's entirely because you are trying to expand __VA_ARGS__ in a macro, rather than std::forward<Args>(args)... in a template.

template<class T, class... Args>
T* make(Args&&... args) {
    if constexpr(std::is_aggregate_v<T>) {
        return ::new T{std::forward<Args>(args)...};
    }
    else {
        return ::new T(std::forward<Args>(args)...);
    }
}