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

[stmt.jump]/2 Fixed wording that does not apply to objects whose lifetime has ended #2872

Closed
wants to merge 2 commits into from

Conversation

sdkrystian
Copy link
Contributor

Fixes #2871

@jensmaurer
Copy link
Member

I agree the existing wording is sub-optimal, but it's not at all clear to me that we want to go this way. For example, [stmt.dcl] p2 wants to say the same thing we say in [stmt.jump], but says that "variables" are destroyed, not objects. Also, the issue of a mutilated object that we claim is "destroyed" is more pervasive, see e.g. [expr.delete] p1. However, for this case, p3 seems to fix the issue.

@sdkrystian
Copy link
Contributor Author

sdkrystian commented May 5, 2019

@jensmaurer Throughout the standard, variable is used as a synonym for object, so it would be appropriate to keep it that way for the sake of consistency. Hence, in the proposed wording, "object to which the variable refers" is used to disambiguate from variables that denote references. As for [stmt.dcl]/2, I believe that is just a small issue with the wording, as reference do not have a storage duration, nor can they be destroyed as they are not objects.

@jensmaurer
Copy link
Member

[basic] p6

A variable is introduced by the declaration of a reference other than a non-static data member or of an
object. The variable’s name, if any, denotes the reference or object.

[basic.stc] p3

The storage duration categories apply to references as well.

Maybe it simplifies the presentation if we claim that references can be destroyed as well (except it's a no-op)? This is moving away from "editorial" land, though.

@sdkrystian
Copy link
Contributor Author

sdkrystian commented May 5, 2019

@jensmaurer Specifying that references can be destroyed would be the way to go then, as it would clear up the wording for this particular instance as well as make it clear what is meant in [stmt.dcl]/2.

It would also resolve the numerous instsnces where variables are said to be destroyed:
http://eel.is/c++draft/basic#stc.thread-2
http://eel.is/c++draft/basic#stc.auto-3
http://eel.is/c++draft/basic#start.main-1.sentence-2

@jensmaurer
Copy link
Member

Given those other locations, maybe the minimal fix is simply to replace "object" with "variable" in the text at issue? This would make the text parallel to the other appearances.

@sdkrystian
Copy link
Contributor Author

sdkrystian commented May 6, 2019

@jensmaurer I think that is good, but some minor changes should made, such as replacing "constructed" with "declared", as without this change this would occur:
foo f; { new (&f) foo; } // Constructed in this scope, wording says it should be destroyed

@jensmaurer
Copy link
Member

jensmaurer commented May 6, 2019

On the other hand, we really want "constructed" in the sense of "control flowed through the declaration". It's not enough that the variable is declared in block scope; you must have also actually reached the declaration:

{
  foo f1;   // constructed
  goto out;
  foo f2;   // not constructed, not destroyed
} // f1 is destroyed
out:

@zygoloid
Copy link
Member

zygoloid commented May 6, 2019

I think we really want "objects" not "variables that refer to objects", in order to deal properly with lifetime-extended temporaries.

Can someone explain the problem with the status quo? [basic.life]p9 seems to already cover the case where an explicit destructor call on a variable precedes a goto, and we should not repeat that rule here.

@jensmaurer
Copy link
Member

@zygoloid, temporaries (also lifetime-extended ones) are fully covered by [class.temporary] p7 (nowadays under [basic]), so don't seem to need special treatment here.

The problem with "object" is that being an "object" is a run-time property. So, in this example:

{
  C x;
  x.~C();
}  #1 // does not destroy "x"

Since the "x" object has already been destroyed at #1, arguably the phrase "objects with automatic storage duration are destroyed" doesn't apply to "x" (the object is already gone according to [intro.object] p1). However, we want this case to be undefined behavior per [basic.life] by invoking the destructor of C on the automatic storage which used to contain the object "x".

@zygoloid
Copy link
Member

zygoloid commented May 6, 2019

I don't think [class.temporary]p7 says when or whether the temporary in this case is destroyed:

{
    int &&r = 0;
}

The first sentence does not apply. The second sentence does not apply. And the third sentence adds no constraints.

But the wording (surprisingly) found in [stmt.jump]p2 currently does cover it: the int temporary is an automatic storage duration object, so it's destroyed on exit from the scope (however accomplished). Once you know it's being destroyed, the rules in [class.temporary]p7 tell you what order to destroy it in relative to other objects that are destroyed at the end of the scope.

This wording probably belongs in [basic.life], likely immediately preceding paragraph 7, which talks about an "implicit destructor call" as if we'd already said there would be one, which we didn't. In fact, the first place where we actually say that destructors get implicitly called seems to be in [class.temporary]/4, and the primary place where we say that destructors get run seems to be [class.dtor]p12. [stmt.dcl]/2 also says "Variables with automatic storage duration [...] are destroyed on exit from the block" but doesn't cover temporaries.

We have a lot of redundancy here, some wording using the wrong terminology (these days, we should be talking about objects being destroyed, not about destructors), and plenty of wording in surprising places. :-(

I think a more substantial overhaul would be a good idea, with the intention being:

  • Only normatively specify where and when implicit destruction happens in one place (ideally in [basic.life])
  • Be sure that we talk more broadly about "destruction" where appropriate, rather than narrowly about "destructors"
  • Convert the duplicated normative wording into notes with cross-references

@sdkrystian
Copy link
Contributor Author

sdkrystian commented May 6, 2019

@zygoloid Is there even a way to specify using objects? As stated in [basic.life] p4, the wording in the standard only applies to objects during their lifetime. So in a basic case such as

{
  foo f;
  f.~foo();
}

There would be no way to specify this in terms of an object, as one simply does not exist. Specifying this in terms of a variable is one of the ways to do it, however, references currently are not destroyed, so we would either have to specify that this only applies to variables referring to objects, or specify that references can be destroyed.

Rewording and reorganizing where implicit destruction is specified sounds like a good idea, as currently the wording is spread out all over the place, inconsistent, and in this case of the clause referenced by the issue, incorrect.

@jensmaurer
Copy link
Member

@zygoloid, I follow your argument, but that makes [basic.stc.thread] p2 insufficient, too, because it also doesn't directly talk about references. My approach:

  • Introduce "destroyed" for references, too.
  • That makes "a variable is destroyed" meaningful.
  • Lifetime-extended temporaries get the treatment of the reference they're bound to [class.temporary] p6.
  • So, when we destroy variables at end-of-scope, we destroy objects and references in reverse declaration order, and temporaries come along indirectly because their references are destroyed in the wash.
  • Talk about "destructor" only once by saying "An object of class type is destroyed by invoking its destructor."
  • Somehow find a way to phrase this such that it also applies to not-yet and no-longer objects in the sense of [basic.life] p6 and p7.

I'm feeling this is getting large enough to escape editorial scope, but let's see.

@jensmaurer
Copy link
Member

jensmaurer commented Jun 5, 2019

Editorial meeting: This needs a paper to CWG, which should also fix the incorrect assumption that a variable is-an object. (Instead, a variable refers to an object or similar.) This should also address that re-entering a function creates fresh objects for the block-scope variables. Also, the destructor call at the end of a scope presumably uses a pointer to the original object, which may not point to a replacement meanwhile created at the same storage location.

@jensmaurer jensmaurer added cwg Issue must be reviewed by CWG. not-editorial Issue is not deemed editorial; the editorial issue is kept open for tracking. labels Jun 5, 2019
@jensmaurer jensmaurer closed this Jun 5, 2019
@sdkrystian
Copy link
Contributor Author

@jensmaurer Alright, I'll start on that then.

@sdkrystian
Copy link
Contributor Author

@jensmaurer Finally getting around to writing the paper, one approach I was thinking about goes as follows:

  • Define "region of storage", associate its acquisition with the definition of a variable, call to an allocation function, or instance of a string literal
  • Regions of storage have a duration (maybe storage class would be more appropriate) , (static, thread, dynamic, automatic), if the storage isn't dynamic it has an associated type determined from the type of the variable definition where it was acquired
  • Creating an object in storage whose associated type is const -> UB
  • When control passes through the definition of a variable, a region of storage with the appropriate duration is acquired, and an object is created within that storage. Likewise, upon exit from a scope (for automatic duration), termination of the program (for static duration) or thread exit (for thread duration), storage acquired in that block/thread/program is released.
  • When storage is released, the object of associated type that occupies the storage is destroyed. If it has an associated type and the type has a non-trivial destructor, an object of the associated type must occupy the storage, otherwise UB.
  • Remove the concept of storage duration, since objects are not storage. The duration of the storage acquired by a variable definition depends on the context it appears in and its storage class specifiers.
  • Introduce "points to storage" for pointers and "refers to storage" for glvalues. Pointer values that aren't invalid or null point to storage at the address they represent. Unary & applied to a glvalue referring to storage yields a pointer to storage. Allocation functions return pointers to storage.
  • A temporary object occupies storage that a hypothetical variable would in the same context.

Thoughts?

@jensmaurer jensmaurer changed the title [stmt.jump]/2 Fixed wording that does not apply to objects who's lifetime has ended [stmt.jump]/2 Fixed wording that does not apply to objects whose lifetime has ended Apr 1, 2020
@jensmaurer
Copy link
Member

Initial thoughts:

  • Region of storage: the exception object presumably also lives in a region of storage, but is not any of the listed things.
  • "definition of a variable" is not a runtime-dynamic thing, but we need that, as in "control flow passes through the definition of an automatic variable, at which point storage is obtained and an object is created" or so
  • Storage is released "atomically" upon (e.g.) return from a function, but the destruction of n local variables happens in some order. Since some kinds of storage (e.g. dynamic) don't have implicit destruction, maybe it's better to talk about leaving the scope of a variable (or thread/program exit) to trigger destruction, and leave storage release somewhat separate.
  • We definitely want pointer values to point to objects, not to storage.
  • It's not clear that we should require that the storage for temporary objects lives much longer than its contained object; it seems perfectly fine for the storage of a non-lifetime-extended temporary to go away at the end of the full-expression.

@sdkrystian
Copy link
Contributor Author

The exception object presumably also lives in a region of storage, but is not any of the listed things.

Whoops! Forgot to include that here. Something along the lines of "the region of storage occupied by an exception object is acquired in an unspecified way, except as explicitly noted in [...]"

"definition of a variable" is not a runtime-dynamic thing, but we need that

Maybe we add a term "defining statement" in [stmt.dcl] which would be relative to a variable.

We definitely want pointer values to point to objects, not to storage.

Perhaps I phrased this incorrectly. Pointers will still point to objects, however they will at the same time point to the storage at the address the represent. However, the fact that pointer to object can point to an object that no longer exists still remains problematic.

Thank you for the feedback :)

@languagelawyer
Copy link
Contributor

foo f; { new (&f) foo; } // Constructed in this scope, wording says it should be destroyed

Objects created by new-expression have dynamic storage duration, not automatic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cwg Issue must be reviewed by CWG. not-editorial Issue is not deemed editorial; the editorial issue is kept open for tracking.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[stmt.jump]/2 Wording does not apply to objects whose lifetime has ended
4 participants