cplusplus / CWG

Core Working Group
24 stars 7 forks source link

CWG2666 [class.temporary] Wording for lifetime extension through `static_cast` is too strict #153

Open t3nsor opened 2 years ago

t3nsor commented 2 years ago

Full name of submitter (unless configured in github; will be published with the issue): Brian Bi

Reference (section label): [class.temporary]/6

Issue description:

Consider the below:

struct S { S(int) {} };

const S& r1 = 0;  // 1
const S& r2 = static_cast<const S&>(S(0));  // 2
const S& r3 = static_cast<const S&>(0);  // 3
// Are r1, r2, and r3 dangling past this point?

Does lifetime extension occur or not?

In (1), a temporary object is materialized using the S::S(int) constructor and the reference binds to the result of the materialization conversion. This results in lifetime extension (p6.1).

In (2), "the glvalue to which the reference is bound" is not obtained directly from a temporary materialization conversion, so p6.1 doesn't apply. A temporary is materialized in the course of evaluating the static_cast, and the static_cast expression is an lvalue referring to that temporary. We have to look at p6.6 to determine whether lifetime extension occurs. But p6.6. requires the cast to convert "without a user-defined conversion, a glvalue operand that is one of these expressions to a glvalue that refers to the object designated by the operand, or to its complete object or a subobject thereof".

The operand is not a glvalue, at least not initially. [expr.static.cast]/4 applies, because there is an implicit conversion sequence from S(0) to const S&. The effect of the cast is the same as that of the initialization const S& t(S(0)). This results a temporary materialization conversion. But it seems like a stretch to say that the operand of the static_cast is a glvalue. If [expr.static.cast] had any explicit text like "The temporary materialization conversion is applied to the operand. The converted operand is then used in place of the original operand for the remainder of this subclause", it would be more reasonable to say that the static_cast acts upon a glvalue operand for the purposes of [class.temporary]/6.6.

In (3), the cast involves a user-defined conversion, so [class.temporary]/6.6 appears not to apply.

Nevertheless, GCC, Clang, MSVC, and ICX unanimously agree that lifetime extension occurs in (2) and (3). See Godbolt link. It seems that the implementers have inferred the intent of [class.temporary]/6.6, and it's not what the wording says.

Suggested resolution: Will be provided later.

jensmaurer commented 2 years ago

I think it's pretty obvious that the rules apply recursively, and p6.6 needs a glvalue to start with. You'll get that from temporary materialization (p6.1), and then you do the reference binding (which is then without involving a user-defined conversion).

Do you have a specific suggestion how to improve the wording here?

languagelawyer commented 2 years ago

Do you have a specific suggestion how to improve the wording here?

I have. [expr.static.cast] shall say that for non-references type cast, a prvalue operand is expected and for reference — glvalue. Then https://timsong-cpp.github.io/cppwp/n4868/basic.lval#6 and https://timsong-cpp.github.io/cppwp/n4868/basic.lval#7 will apply, and https://timsong-cpp.github.io/cppwp/n4868/expr.static.cast#8.sentence-1 will become unnecessary

frederick-vs-ja commented 2 years ago

Previously discussed in cplusplus/draft#5352.

I think we need to say every form of explicit cast (*_cast, C-style cast, and function-style cast) to a reference type has a glvalue as its operand which may be the result of temporary materialization (and different from the lexical one).

jensmaurer commented 1 year ago

C-style cast and function-style cast are eventually mapped to static_cast etc.

CWG2666

t3nsor commented 1 year ago

Do you have a specific suggestion how to improve the wording here?

I have. [expr.static.cast] shall say that for non-references type cast, a prvalue operand is expected and for reference — glvalue. Then https://timsong-cpp.github.io/cppwp/n4868/basic.lval#6 and https://timsong-cpp.github.io/cppwp/n4868/basic.lval#7 will apply, and https://timsong-cpp.github.io/cppwp/n4868/expr.static.cast#8.sentence-1 will become unnecessary

The first part of that doesn't seem correct; static_cast<T>(r) should not unconditionally perform the lvalue-to-rvalue conversion on r as it may be that T's constructor simply takes r by reference. At best, you can replace sentence 1 in paragraph 8 with something like "Otherwise, the operand shall be a prvalue."

The second part doesn't seem correct either; static_cast<const T&>(T()) is required to create a temporary object of type const T under http://eel.is/c++draft/dcl.init.ref#5.3.sentence-2 ; if we made it so that static_cast implicitly performs a materialization, the temporary would have type T.

t3nsor commented 1 year ago

Here's my rough attempt at a suggested resolution:

Edit [class.temporary]/6.6:

a

  • const_cast ([expr.const.cast]),
  • static_cast ([expr.static.cast]),
  • dynamic_cast ([expr.dynamic.cast]), or
  • reinterpret_cast ([expr.reinterpret.cast]),

~converting, without a user-defined conversion, a glvalue operand that is one of these expressions to a glvalue that refers to the object designated by the operand, or to its complete object or a subobject thereof,~ where

  • the cast does not invoke a user-defined conversion, or
  • the cast invokes a user-defined conversion that yields a prvalue, and the glvalue result of the cast is bound directly ([dcl.init.ref]) to that prvalue.
jensmaurer commented 1 year ago

I've amended CWG2666 accordingly, but I think " the glvalue result of the cast is bound directly" isn't quite right: references bind directly, glvalues are expressions and don't bind anything.

t3nsor commented 1 year ago

We need some way to talk about the invented reference variable, then.

hubert-reinterpretcast commented 5 days ago

This suggested resolution fails to have the "is one of these expressions" structure. This is a problem because the "is one of these expressions" is meant to prevent lifetime extension for cases where the glvalue to a temporary object (or subobject thereof) is arrived at "indirectly" (notice the unary + below):

int glob;
struct A {
  constexpr ~A() { arr[0] = &glob; }
  int *arr[1];
};
constexpr int f() {
  int *&&ref = static_cast<int *&&>(0[+A().arr]);  // lifetime not extended
  delete ref;  // error: attempt to delete `&glob`
  return 0;
}
extern constexpr int x = f();
jensmaurer commented 5 days ago

@t3nsor , could you give that another shot, please?