llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.25k stars 11.66k forks source link

Implicitly-defined default constructor not generated for nested class #49690

Open 4706ef27-75ea-42f3-88fa-731e80a74b18 opened 3 years ago

4706ef27-75ea-42f3-88fa-731e80a74b18 commented 3 years ago
Bugzilla Link 50346
Version trunk
OS Linux
CC @dwblaikie,@DougGregor,@gjasny,@zygoloid,@jwakely

Extended Description

Code sample:

#include <optional>

struct Outer {
    struct Inner {
        int a = 0;
    };
    std::optional<Inner> inner;
};

int main() {
    Outer o;
    o.inner.emplace();
    return 0;
}

https://godbolt.org/z/jKoG9a7e4

In the above, the emplace() call causes an error, stating that the class (Outer::Inner) does not have a constructor that matches the arguments. Note that this is specifically an issue in combination with libstdc++'s impl of std::optional; libc++ (since clang 9) seems to work (though prior to clang 9 it generates the same error).

If you do std::optional<Outer::Inner>, it also fails (though it will succeed if you remove the inner member).

If you change int a = 0 to int a (without the default value), it will succeed.

If you add an Inner() = default, it still fails, however adding Inner() {} will make it succeed.

This may be loosely related to bug 15886.

ec04fc15-fa35-46f2-80e1-5d271f2ef708 commented 3 years ago

Interesting: the bad instantiation of is_constructible happens while implicitly declaring the copy constructor for Outer:

https://godbolt.org/z/KbjYoG84a

So it looks like the compiler divergence here is because GCC declares the copy constructor lazily whereas Clang happens to declare it eagerly in this case.

3a17e7c4-fca4-4827-b2a7-11a54d73d746 commented 3 years ago

I don't know the answer off-hand. I think this is probably undefined, but it's a reasonable thing to expect to work. I always find it surprising that nested types are not complete until the outer type is complete, rather than after the }; of their class body.

I tend to think this should work even if the standard doesn't allow it today.

ec04fc15-fa35-46f2-80e1-5d271f2ef708 commented 3 years ago

This is bizarre, but I think it's not a Clang bug.

The problem here is that the libstdc++ implementation of optional queries is_constructible while instantiating the class optional, which happens before we reach the end of class Outer. At that point, the '= 0' initializer for 'a' has not yet been parsed, so the default constructor for Inner cannot yet be semantically checked, so Inner is not constructible yet. (Whether libstdc++ is within its rights to make that query is not clear to me.)

Now, because querying is_constructible instantiates a class template, that instantiation is cached, so when the same query happens later, in the call to o.inner.emplace(), the cached result 'false' is returned, and the emplace() function is not viable.

Ultimately, the problem is that (libstdc++'s) optional does not support types T that are not completely-defined, and Inner is not completely-defined at the point where it's used as a template argument to optional in this example, because the '= 0' is not parsed until the '}' of 'Outer'.

Interestingly, if you attempt to default-construct an Outer::Inner before the optional instantiation, then GCC agrees with Clang that Outer::Inner is not default-constructible, and moreover agrees that is_constructible caches that result:

https://godbolt.org/z/v463bE5hh

I'm not sure why GCC doesn't observe the same result without the explicit preceding use of Outer::Inner::Inner().

+Jonathan Wakely, who might be able to shed some light on whether this is a bug in libstdc++ or whether this kind of example is violating the standard library's type completeness rules, or if my analysis is incorrect.

llvmbot commented 5 months ago

@llvm/issue-subscribers-clang-frontend

Author: None (4706ef27-75ea-42f3-88fa-731e80a74b18)

| | | | --- | --- | | Bugzilla Link | [50346](https://llvm.org/bz50346) | | Version | trunk | | OS | Linux | | CC | @dwblaikie,@DougGregor,@gjasny,@zygoloid,@jwakely | ## Extended Description Code sample: ```cpp #include <optional> struct Outer { struct Inner { int a = 0; }; std::optional<Inner> inner; }; int main() { Outer o; o.inner.emplace(); return 0; } ``` https://godbolt.org/z/jKoG9a7e4 In the above, the `emplace()` call causes an error, stating that the class (`Outer::Inner`) does not have a constructor that matches the arguments. Note that this is specifically an issue in combination with libstdc++'s impl of std::optional; libc++ (since clang 9) seems to work (though prior to clang 9 it generates the same error). If you do `std::optional<Outer::Inner>`, it also fails (though it will succeed if you remove the `inner` member). If you change `int a = 0` to `int a` (without the default value), it will succeed. If you add an `Inner() = default`, it still fails, however adding `Inner() {}` will make it succeed. This may be loosely related to bug 15886.
Endilll commented 5 months ago

Confirmed on post-18 trunk: https://godbolt.org/z/5xqn1raq8