-
Notifications
You must be signed in to change notification settings - Fork 68
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
Unify group
code snippet with other non-user-constructible classes
#627
base: main
Are you sure you want to change the base?
Unify group
code snippet with other non-user-constructible classes
#627
Conversation
`group` is not user-constructible, but explicit information about its constructors was missing from the code snippet.
Adding the editorial label. The spec was already clear that these types are not user constructible, so this PR just clarifies that in the synopsis. I'll wait a few days before merging in case others have comments. |
Just curious about this implementation detail leakage: I have the feeling that "not user-constructible" does not mean it has no default constructor, just it cannot be constructed by the user. For example the constructor could be |
This situation occurs in several places in the SYCL spec. We want to document that one of the implicitly-defined member functions is not available to the application. We currently shows these cases as There is a similar situation with |
I agree with Ronan here. We need to be precise about what "not user constructible" means, and I think it's different to saying that the default constructor is deleted. I ran some experiments (see https://godbolt.org/z/d1h4o8Ge3), and the results are a little surprising. clang doesn't complain about the construction of struct S2 {
S2() = delete;
};
S2 a; // this is invalid
S2 b{}; // this is (surprisingly) valid, even though the constructor is deleted
struct S3 {
private:
S3() {};
};
S3 c; // this is invalid
S3 d{}; // this is also invalid Does anybody understand this behavior? Regardless, I think that in order to accept |
@Pennycook There was a core issue on this (I can't check at the moment because open-std.org is down). The reason it compiles in C++17 is because |
Oof. Does that mean that we can't use |
I think we can use We might have to call out that these types are not aggregates. |
I don't think they're required to be, but I think they currently can be. After the discussion in #532, we reached the conclusion that classes like
Would we just say something like "These types must not be aggregates." and leave it up to individual implementations to decide how to enforce that? I can imagine some implementations will want to rely on private/protected members, while others will want to rely on a private/protected constructor. |
Stateless means empty, which does not seem like an aggregate to me, since in C++ it will have a size of 1 anyway to enforce uniqueness of objects. Otherwise, I was looking at some types in C++. For example https://en.cppreference.com/w/cpp/types/type_info describes the constructor as deleted but looking more carefully at https://eel.is/c++draft/type.info it is just that at least there is no exposed public constructor. At the end, this is not super important. We could always have a public constructor |
> Would we just say something like "These types must not be aggregates." and leave it up to individual implementations to decide how to enforce that? I think so, especially since this is really just an issue for those compilers supporting C++17. Any version of SYCL that requires C++20 or later will likely just delete the default constructor. |
Looking into 16.3.2.4 Detailed specifications it says the following:
This makes me think that we should actually omit the description of any constructors to say that there aren't any (user-visible) constructors. |
|
group
is not user-constructible, but explicit information about its constructors was missing from the code snippet.