-
Notifications
You must be signed in to change notification settings - Fork 800
[class] remove redundant "constexpr-suitable" references to determine constexpr-ness of constructors/destructors #8108
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
[class] remove redundant "constexpr-suitable" references to determine constexpr-ness of constructors/destructors #8108
Conversation
…t be coroutines (since P3533R2), therefore they are always constexpr-suitable and removed wording is not needed.
|
The changes seems correct to me, but maybe CWG should have a look at this anyway to be sure. I would be in favor of deleting "constexpr-suitable" entirely; the term is just adding unnecessary indirection at this point, seeing that it's synonymous with "not a coroutine". |
|
The library parts of the changes overlap with LWG2833. Although I'd be happy to see these parts being fixed editorially. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes look good to me.
We have a pending paper in CWG (for C++29) that gets rid of the "coroutine" condition, too. I wouldn't want to touch those places twice; instead that paper should just remove all remaining uses of constexpr-suitable. |
| \remarks | ||
| This function is \keyword{constexpr} if and only if the | ||
| value-initialization of the alternative type $\tcode{T}_0$ | ||
| would be constexpr-suitable\iref{dcl.constexpr}. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change in particular seems to be removing an imperfect attempt to specify something which is not easily deducible from other normative wording.
This says that whether the constructor is a constant expression relies only on initialisation of that one alternative, and nothing else. The equivalent wording for duration and pair and tuple does the same, but it's arguably easier to deduce that from the exposition-only members of those types. Here I think we're actually losing useful wording.
Maybe the correct fix is to say an evaluation of this constructor is a constant expression if...
But just removing these words doesn't seem like an improvement to me. Until we resolve the much larger "what does the library mean when it says something is a constexpr function" issue this wording is not redundant and is load-bearing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you really need to say that? This what you are saying is conditional constexpr, but now constexpr is just an opt-in to constant-evaluation, it's not guarantee, you can just "this function is marked constexpr", and if any of the subtype constructors is not, it will fail to call it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you need specify conditional constexpr only for these types (pair/tuple/time_duration) it feels like a redundancy and language wording is enough. Especially with templates you must explicitly aim it on make something constexpr not evaluatable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The conditional constexpr described in these Remarks would currently always be present, and it's an improvement of the status quo to make it unconditionally constexpr. The issue described is likely broad enough to warrant a paper, so it seems a bit silly to hold back this correct editorial change.
What seems to be missing here is a Constant When specification, but that would require non-editorial changes. It also seems to me that there's a much broader issue of under-specifying in the standard library when calls to constexpr functions are constant expressions and not. For example, we don't say anywhere in [algorithms] that running constexpr algorithms is a constant expression only if every operation performed on the iterators is a constant expression, do we? Should that be specified for each algorithm or as blanket wording? Big questions ...
Can't we just address that as blanket wording in [library] anyway?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this particular paragraph, it is stated that the variant value-initializes the first alternative, so it seems clear enough that it wouldn't produce a constant expression if that value-initialization isn't a constant expression.
That doesn't guarantee that the constexprness relies only on the constexprness of that initialization, but the same applies to basically anything that's not specified as "Effects Equivalent to:" We never say that std::countl_zero doesn't use goto internally, and yet it's marked constexpr, and yet it always produces a constant expression in practice.
If that is okay, then we shouldn't need to point out when exactly std::variant initialization is a constant expression either, since we do that basically nowhere in the standard library and it seems like a big effort to attempt consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The conditional
constexprdescribed in these Remarks would currently always be present,
It is not saying that the constexpr keyword is conditionally present.
and it's an improvement of the status quo to make it unconditionally
constexpr.
It is unconditionally constexpr, the keyword is right there on the declaration.
The issue described is likely broad enough to warrant a paper, so it seems a bit silly to hold back this correct editorial change.
The issue is LWG2833.
What seems to be missing here is a Constant When specification,
That is exactly what this wording is for!
As I keep saying, it's not saying it correctly, but unless we fix the wording to say it correctly, I object to replacing imperfect wording with nothing.
but that would require non-editorial changes. It also seems to me that there's a much broader issue of under-specifying in the standard library when calls to
constexprfunctions are constant expressions and not.
Yes, LWG2833
For example, we don't say anywhere in [algorithms] that running
constexpralgorithms is a constant expression only if every operation performed on the iterators is a constant expression, do we? Should that be specified for each algorithm or as blanket wording? Big questions ...
Yes, that's LWG2833
Can't we just address that as blanket wording in [library] anyway?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this particular paragraph, it is stated that the variant value-initializes the first alternative, so it seems clear enough that it wouldn't produce a constant expression if that value-initialization isn't a constant expression.
That doesn't guarantee that the constexprness relies only on the constexprness of that initialization,
Which is precisely my point. The wording that this PR removes says it relies only on that.
but the same applies to basically anything that's not specified as "Effects Equivalent to:" We never say that
std::countl_zerodoesn't usegotointernally, and yet it's markedconstexpr, and yet it always produces a constant expression in practice.
Yes, and ideally we would have a proper spec not just something that works in practice.
If that is okay
It's not OK really.
then we shouldn't need to point out when exactly
std::variantinitialization is a constant expression either, since we do that basically nowhere in the standard library and it seems like a big effort to attempt consistency.
So we should be consistently bad and say nothing at all, instead of doing slightly better in a few places?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jwakely thanks for clarifying. I didn't quite understand that saying "this function is constexpr" is actually attempting to say that it's conditionally a constant expression. I thought you meant it's a bad attempt at doing the latter by actually superseding the constexpr on the declaration with a conditional constexpr keyword.
Now it all makes sense, and considering LWG2833, I agree that we should fix this paragraph instead of just getting rid of it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be clear, are you trying to say this should go something like
An invocation of this constructor is a core constant expression if and only if the value value-initialization of an object of the alternative type
$T_0$ is a constant expression ([expr.const]).
(possibly rephrased as Constant When)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did rollback the library part, going forward only with the language part.
I don't like the library changes. We keep coming back to this: removing the imperfect library wording should happen after it's replaced with something better, not before. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nack, for the library parts
de4c007 to
781688e
Compare
constexpr-suitable references to determine constexpr-ness of constructors/destructorsconstexpr-suitable references to determine constexpr-ness of constructors/destructors
|
Editorial meeting: the change (now only for the core wording) seems fine, but should be witnessed by CWG. Then fine to go in. |
constexpr-suitable references to determine constexpr-ness of constructors/destructorsThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CWG 2025-11-05: Approved.
Library parts of this PR have been removed, so library review is moot.
It occurred to me constructors/destructors can't be coroutines. Currently only remaining things which makes a function NOT
constexpr-suitableis function being a coroutine. Hence constructors and destructors can always be constexpr. Removed wording is redundant and no longer has any meaning.Question: I can go further and remove
constexpr-suitablecompletely by removing its definition in [dcl.constexpr], and replacing its uses withfunction is not a coroutine.