Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clang c++20: Associated constraints added to a default constructor are excessively checked #60293

Open
frederick-vs-ja opened this issue Jan 25, 2023 · 13 comments
Labels
c++20 clang:frontend Language frontend issues, e.g. anything involving "Sema"

Comments

@frederick-vs-ja
Copy link
Contributor

In the following code snippet, the declaration of typedef-name CW is accepted by msvc and gcc, but rejected by clang. Godbolt link.

I think clang is probably buggy here. When determining whether decltype(ConstrainedWrapper<Wrapped>{std::declval<Wrapped>()}) is well-formed, clang seemly excessively requires the associated constraints of the default constructor to be satisfied, even if the default constructor is not selected.

#include <concepts>
#include <cstddef>
#include <utility>

struct NoDefaultCtor {
    constexpr explicit NoDefaultCtor(std::nullptr_t) {}
};

template<class T>
struct Wrapper {
    Wrapper() = default;
    constexpr explicit Wrapper(T x) : val(std::move(x)) {}

    T val{};
};

template<class T>
struct ConstrainedWrapper {
    ConstrainedWrapper() requires std::default_initializable<T> = default;

    constexpr explicit ConstrainedWrapper(T x) : val(std::move(x)) {}

    T val{};
};

using Wrapped             = Wrapper<NoDefaultCtor>;
using W [[maybe_unused]]  = decltype(Wrapper<Wrapped>{std::declval<Wrapped>()});
using CW [[maybe_unused]] = decltype(ConstrainedWrapper<Wrapped>{std::declval<Wrapped>()}); // <- clang is buggy here
@EugeneZelenko EugeneZelenko added c++20 clang:frontend Language frontend issues, e.g. anything involving "Sema" and removed new issue labels Jan 25, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 25, 2023

@llvm/issue-subscribers-c-20

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 25, 2023

@llvm/issue-subscribers-clang-frontend

@royjacobson
Copy link
Contributor

Have taken a quick look at this, do you think this might be a missing SFINAE trap?

@royjacobson royjacobson added the concepts C++20 concepts label Jan 28, 2023
@frederick-vs-ja
Copy link
Contributor Author

Have taken a quick look at this, do you think this might be a missing SFINAE trap?

Yes. After further investigation, I think the issue is not triggered by associated constraints, and thus perhaps the issue shouldn't be tagged with c++20/concepts (c++11 may be appropriate).

This example (Godbolt link) also triggers the issue. From which I think this issue is because that clang's __is_constructible fails to handle defaulted but implicitly deleted default constructors.

#include <concepts>
#include <cstddef>
#include <type_traits>
#include <utility>

struct NoDefaultCtor {
    constexpr explicit NoDefaultCtor(std::nullptr_t) {}
};

template<class T>
struct Wrapper {
    Wrapper() = default;
    explicit Wrapper(T x) : val(std::move(x)) {}

    T val{};
};

static_assert(std::is_default_constructible<Wrapper<NoDefaultCtor>>::value, "");

@royjacobson
Copy link
Contributor

Good catch!

There IS a SFINAE trap in SemaExprCXX.cpp:5455 but it's not working for some reason.

@royjacobson royjacobson removed the concepts C++20 concepts label Jan 28, 2023
@royjacobson
Copy link
Contributor

So, this is a tricky one. If I understand the standard correctly, the clause that would delete Wrapper's default constructor is this, but since val has a brace initializer, the Wrapper has a non-deleted default constructor, but one that can never be instantiated. I'm not sure if is_constructible is even supposed to instantiate the default constructor? And then there's this note:

The evaluation of the initialization can result in side effects such as the instantiation of class template specializations and function template specializations, the generation of implicitly-defined functions, and so on.
Such side effects are not in the “immediate context” and can result in the program being ill-formed.

So I think Clang is technically correct on this one. But even if it, we should at least have some reasonable error message for this silly language edge case.

@frederick-vs-ja
Copy link
Contributor Author

Oh, I see. Probably, the defaulted default ctor of Wrapper<NoDefaultCtor> shouldn't be implicitly deleted according to current wording.

However, I guess the note is a bit superfluous, as it doesn't need to generate the definition to determine whether the initialization is well-formed in the immediate context.

This example reveals implementation divergence (Godbolt link):

#include <type_traits>

template <class T>
struct Wrapper {
    T x = (void*)nullptr;
};

#ifndef __clang__
#if defined(_MSC_VER) || defined(__ICC)
static_assert(std::is_default_constructible_v<Wrapper<int>>);
#elif defined(__GNUC__)
static_assert(!std::is_default_constructible_v<Wrapper<int>>);
#endif
#else
static_assert(true || std::is_default_constructible_v<Wrapper<int>>); // rejected by clang
#endif

MSVC and ICC seemly stop further process when they know the default constructor of Wrapper<int> is not deleted (and hence well-formed in immediate context). They are also possibly technically correct IIUC.

@royjacobson Do you think we need a CWG/LWG issue?

@royjacobson
Copy link
Contributor

I think this is 'working as intended' and GCC have a bug here. Clang/MSVC/ICC are correct and the note permits this implementation divergence. Since there's explicit language for both the member initialization non-deletion and the trait instantiation causing the program to be ill-formed I tend to think this might have been intentional.

Clang explicitly tries to instantiate the default constructor in this case, which is a BIT weird, but I don't think it's necessarily a bad idea (as otherwise the trait would just return true!).

@shafik
Copy link
Collaborator

shafik commented Jan 29, 2023

It might be worth a gcc bug report and if they disagree then we can escalate to a core or lwg issue.

@royjacobson
Copy link
Contributor

I sent https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108588, let's see what they say about it.

@CaseyCarter
Copy link
Member

I'm not sure if is_constructible is even supposed to instantiate the default constructor?

I think this is the disconnect between Clang's behavior and the other compilers. I believe the body of the default constructor shouldn't be instantiated: since "Only the validity of the immediate context of the variable initialization is considered", the constructor isn't ODR-used. If this interpretation isn't correct, we'll need to correct the specification of several types in the Standard Library that use this style.

@royjacobson
Copy link
Contributor

royjacobson commented Feb 1, 2023

@CaseyCarter I think we will be able to change the behavior to not instantiate the constructor. I guess technically it isn't ODR used so it shouldn't be instantiated, even if the note kind of permits it.

But it's a bit odd that a type is default constructible, is explicitly checked for default constructability, and its default constructor is never instantiated later, no? Wouldn't instantiating the default constructor of the wrapping type be ill formed just as well?

@frederick-vs-ja
Copy link
Contributor Author

But it's a bit odd that a type is default constructible, is explicitly checked for default constructability, and its default constructor is never instantiated later, no?

Unfortunately (?), this seems a common pattern in the standard library (parts for iterators and ranges) since C++20...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++20 clang:frontend Language frontend issues, e.g. anything involving "Sema"
Projects
None yet
Development

No branches or pull requests

6 participants