19

A really strange and unexpected behaviour of clang 5 was detected when switching to c++17 and replacing custom std::optional solution with the standard one. For some reason, emplace() was being disabled due to faulty evaluation of a std::is_constructible trait of the parameter class.

Some specific preconditions must be satisfied before it reproduces:

#include <optional>

/// Precondition #1: T must be a nested struct
struct Foo
{
    struct Victim
    {
        /// Precondition #2: T must have an aggregate-initializer
        /// for one of its members
        std::size_t value{0};
    };

    /// Precondition #3: std::optional<T> must be instantiated in this scope
    std::optional<Victim> victim;

    bool foo()
    {
        std::optional<Victim> foo;

        // An error
        foo.emplace(); 
        /// Assertion is failed
        static_assert(std::is_constructible<Victim>::value);
    }
};

Live example on godbolt.org

Change any of the preconditions and it compiles as expected. Is there some unknown inconsistency in the standard that makes clang reject this code while being compliant?

As a side note: GCC 7.1 and GCC 7.2 have no problem with the above code.

Bug report at: bugs.llvm.org

GreenScape
  • 7,191
  • 2
  • 34
  • 64
  • Could very well be a compiler bug. – Cris Luengo Dec 26 '17 at 07:00
  • @CrisLuengo, I hope so, because it's easier to fix than the standard. – GreenScape Dec 26 '17 at 07:01
  • 1
    At its core, yours is a language lawyer question, really. It should be answered as such. – StoryTeller - Unslander Monica Dec 26 '17 at 08:16
  • Check [this](https://godbolt.org/g/PC9T25). It's a simple implementation of the traits involved, which should all be correct in this context. gcc 7.2 has the most verbose error message _"constructor required before non-static data member for 'Foo::Victim::value' has been parsed"_ – Passer By Dec 26 '17 at 08:21
  • It seems like the combination of any default initializer with optional member all nested in a struct is breaking this. ` = 0` instead of {0} on `value` fails as well. – kabanus Dec 26 '17 at 09:05

2 Answers2

7

This looks like a compiler bug. From [class]

A class is considered a completely-defined object type (or complete type) at the closing } of the class-specifier.

Which means Victim is complete at std::optional<Victim>, making it no different than any other type in this context.

From [meta]

The predicate condition for a template specialization is_­constructible<T, Args...> shall be satisfied if and only if the following variable definition would be well-formed for some invented variable t: T t(declval<Args>()...);

Which is direct-initializing t with arguments of type Args..., or if sizeof...(Args) == 0, it's value-initializing t.

In this case, value-initializing t is to default-initialize t, which is valid hence std::is_constructible_v<Victim> should be true.

With all that said, compilers seems to be struggling a lot compiling this.

Passer By
  • 19,325
  • 6
  • 49
  • 96
  • 1
    From the same paragraph: "Within the class member-specification, the class is regarded as complete within [...] default member initializers (including such things in nested classes)." This seems to imply that the compilers cannot process the default member initializers until they see the closing `}` (and for nested classes, until they see the closing `}` of the enclosing class). – cpplearner Dec 26 '17 at 10:30
  • 2
    @cpplearner I believe that refers to the fact that the (enclosing) class is incomplete within its own body, not that its nested classes are incomplete – Passer By Dec 26 '17 at 12:00
  • 1
    My point is, the default member initializers are still token soup (i.e. they are not parsed) within the enclosing class, and initializing the aggregate will need to refer to these default member initializers. This is more obvious outside of SFINAE context: `struct Outer { struct Inner { int x = 4; }; decltype(Inner()) a; };` – cpplearner Dec 26 '17 at 17:33
  • @cpplearner Oh. That's very bad indeed. I can't find what's explicitly making it illegal though, yet it definitely seems like it should be – Passer By Dec 26 '17 at 23:54
3

Alright, dug up the relevant quotes. The crux of the matter is how std::is_constructible should handle Victim. The most conclusive authority is C++17 (n4659). First [meta.unary.prop/8]:

The predicate condition for a template specialization is_­constructible<T, Args...> shall be satisfied if and only if the following variable definition would be well-formed for some invented variable t:

T t(declval<Args>()...);

[ Note: These tokens are never interpreted as a function declaration.  — end note ] Access checking is performed as if in a context unrelated to T and any of the Args. Only the validity of the immediate context of the variable initialization is considered.

The note I highlighted is not normative (on account of being a note), but it coincides with [temp.variadic]/7:

... When N is zero, the instantiation of the expansion produces an empty list. Such an instantiation does not alter the syntactic interpretation of the enclosing construct, even in cases where omitting the list entirely would otherwise be ill-formed or would result in an ambiguity in the grammar.

So for the purposes of is_­constructible, this T t(); indeed makes t a variable declaration. This initialization is value initialization because [dcl.init/11] says as much:

An object whose initializer is an empty set of parentheses, i.e., (), shall be value-initialized.

That means that the trait ends up checking if Victim can be value-initialized. Which it may. It's an aggregate, but an implicitly defaulted default c'tor is still defined by the compiler (to support value initialization, obviously).

Long story short. Clang has a bug, you should report it.

StoryTeller - Unslander Monica
  • 165,132
  • 21
  • 377
  • 458