0

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?

Lingxi
  • 14,579
  • 2
  • 37
  • 93
  • 1
    Why you try to use preprocessor macros in C++ anymore? If your code with constexpr if works, forget about all the macro stuff. – Klaus Apr 06 '18 at 07:58
  • @Klaus I've updated the question. I do have a reason not to use a function template. – Lingxi Apr 06 '18 at 08:51
  • 1
    Whoops, hammered. The crux of the question is equivalent to that other one (using `if constexpr` outside of a template), but maybe the connection should be made in an answer here. – Quentin Apr 06 '18 at 08:57
  • @Quentin I've added it to my answer. – jotasi Apr 06 '18 at 09:04

2 Answers2

3

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

jotasi
  • 5,077
  • 2
  • 29
  • 51
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)...);
    }
}
Caleth
  • 52,200
  • 2
  • 44
  • 75