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

[unique.ptr.general] Delete note listing unique_ptr use cases #7329

Closed
wants to merge 1 commit into from

Conversation

Eisenwave
Copy link
Contributor

As part of #7261.

[unique.ptr.general] p5 states:

[Note: The uses of unique_ptr include providing exception safety for dynamically allocated memory, passing ownership of dynamically allocated memory to a function, and returning dynamically allocated memory from a function. - end note]

This note should be deleted. It is not incorrect or misleading, but it is worded improperly for the standard and would require too much effort to fix. Specifically, the problems are:

  1. The note provides very little value because everyone and their dog knows what std::unique_ptr is used for at this point.

  2. The standard does not define "dynamically allocated memory", so the note is informal.

  3. The note is needlessly exhaustive. std::unique_ptr with custom deleters can be used to own all sorts of resources. At the very least, the note should state "include, but are not limited to".

  4. The note is not providing guidance to implementers and language lawyers, who are the actual target audience of the document. It seems to be listing possible uses for developers, so it reads like a tutorial note.

  5. [unique.ptr.general] p1 already provides a more general introduction to std::unique_ptr with much less problematic wording.

One could try to tackle all of these issues and turn this into a fantastic note perhaps, but I'm inclined to simply deleting it. It's not worth putting in the time, and it arguably reduces the quality of the standard.

@jensmaurer
Copy link
Member

I disagree; the note looks reasonable. In particular, "include" is not exhaustive by the definition of the English word. "Spectators include females" doesn't exclude males among the spectators.
And p1 doesn't speak to the purposes, it only speaks to the mechanics.
In particular for library components, I don't mind having a mild amount of explanatory material, in particular if it hides in a note and thus is non-normative, and I don't mind informal words such as "dynamically allocated". If we feel "dynamically allocated" needs additional help here, maybe adding a cross-reference to [expr.new] would help.

@Eisenwave
Copy link
Contributor Author

Eisenwave commented Oct 20, 2024

Thank you for your input.

The only thing I can add to that is regarding ...

If we feel "dynamically allocated" needs additional help here, maybe adding a cross-reference to [expr.new] would help.

... I don't believe a cross-reference fixes this issue. "dynamically allocated" memory can be obtained directly via a call to operator new ([new.delete]), or via a new-expression ([expr.new]). std::unique_ptr doesn't strictly require the user to pick between one of these (though std::make_unique does), so it's unclear which cross-reference is appropriate. It could even refer to memory obtained through std::malloc ([c.malloc]).

If we want to keep the note, we should be able to improve it meaningfully, and an \iref doesn't constitute a sufficient fix here in my opinion.

@jensmaurer
Copy link
Member

Well, since std::unique_ptr by default uses delete to free the memory (and destroy the contained object), [expr.new] feels like the primary candidate for a cross-reference. I'm not objecting to cross-references to all three places you mentioned, though.

@Eisenwave Eisenwave closed this Nov 9, 2024
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