Skip to content

Conversation

eernstg
Copy link
Member

@eernstg eernstg commented May 28, 2025

This PR changes the rules about augmentation of constructors such that an incomplete factory is able to defer the decision about implementing the constructor by a redirection or by a body. See #4172 for details.

@munificent
Copy link
Member

I like the general direction here, but if we're going to loosen the proposal so that augmentations can fill in default values, I think we should go all the way and let them do it for all functions.

@eernstg
Copy link
Member Author

eernstg commented Jun 10, 2025

I changed the rules to allow all kinds of augmentable functions (incl. constructors) to specify a default value for an optional parameter in the introductory declaration or in an augmentation, but at most once, PTAL!

@eernstg
Copy link
Member Author

eernstg commented Jun 13, 2025

@munificent, WDYT?

@eernstg eernstg force-pushed the specify_augment_redirection_may25 branch from 4ac903e to ae929b8 Compare July 9, 2025 10:35
Copy link
Member

@munificent munificent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can loosen this up a little more. If you're OK with that, LGTM!

@@ -840,13 +841,21 @@ Augmenting constructors works similar to augmenting a function, with some extra
rules to handle features unique to constructors like redirections and
initializer lists.

It is **not** a compile-time error for an incomplete factory constructor to
omit default values. *That is, they are treated similarly to abstract
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This touches on #3690. With augmentations, it's also not a compile error for a static method declaration to have a ; body, for a constructor declaration to not have initializers are all final instance fields, etc. As long as the final declaration ("definition"?) once all augmentations have been applied is valid, it's fine.

I'm not sure to what degree we want to spell this all out in the proposal but I'm cautious about having too much normative text that is potentially redundant or implied by other parts of the proposal. I don't want implementers to have to read between the lines to figure out what the proposal is saying, but I also don't want them to have to compare paragraphs and try to tell if the two paragraphs are defining different things or just restating a point. Finding the right balance here is hard.

Copy link
Member Author

@eernstg eernstg Aug 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually about the final declaration and not about the ordering of diagnostics. The point is that this allows a factory constructor to be declared without default values of some parameters whose type is potentially non-nullable, and also without a body. This makes it a non-error to turn it into a regular factory (adding the default values in an augmenting declaration), and it also makes it a non-error to make it a redirecting factory (whose parameters can not have any default values).

In other words, it allows the direct/redirecting choice to be made by the augmenter. It seems reasonable to me that we treat this choice as an implementation detail, not an API property.

If we don't allow this approach (and insist that the incomplete factory must have all the required default values) then it is not possible to make such constructors redirecting. I think that's a loss of expressive power that we can just avoid, so why not?

@eernstg eernstg force-pushed the specify_augment_redirection_may25 branch from ae929b8 to 0aa6add Compare August 1, 2025 15:57
@eernstg eernstg force-pushed the specify_augment_redirection_may25 branch from 0aa6add to 5355a4b Compare August 22, 2025 14:13
@eernstg eernstg force-pushed the specify_augment_redirection_may25 branch from 5355a4b to bd43d37 Compare August 22, 2025 14:43
@eernstg
Copy link
Member Author

eernstg commented Aug 22, 2025

@munificent, this one was rebased around the file renaming operations, PTAL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants