volatile read discarded

[llvmbot]

Bugzilla Link	10809
Resolution	INVALID
Resolved on	Jun 14, 2012 14:44
Version	trunk
OS	Linux
Reporter	LLVM Bugzilla Contributor
CC	@efriedma-quic,@rjmccall

Extended Description

Clang discards the volatile read in this testcase:

extern int could_be_volatile;
void foo() {
static_cast<volatile int>(&could_be_volatile);
}

C++11 [expr]/5 describes this as a discarded-value-expression and explains that they are to be evaluated. The volatile operation is whitelisted as undergoing lvalue-to-rvalue conversion because it is an indirection of a volatile typed object.

[efriedma-quic]

Where do I get a copy of the draft that says this?

[llvmbot]

I'm looking at the FDIS. The relevant text is:

"In some contexts, an expression only appears for its side effects. Such an expression is called a discarded-value expression. The expression is evaluated and its value is discarded. The array-to-pointer (4.2) and function-to-pointer (4.3) standard conversions are not applied. The lvalue-to-rvalue conversion (4.1) is applied only if the expression is an lvalue of volatile-qualified type and it has one of the following forms:
[...]

• indirection (5.3.1),
  "

[llvmbot]

I realize this could be interpreted either as a prohibition or as a requirement that the conversion be applied. The text "lvalue-to-rvalue conversion is applied only if [certain conditions]" should probably use "when" in place of "only if". I think it's clear from the context of the DR: http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1054 that they intended the conversion to be performed.

[rjmccall]

Oh, did they sneak that in after all? Hooray for source incompatibility.

It should be easy to change IgnoredValueConversions() to do the lvalue-to-rvalue conversion in exactly these cases.

I'm ambivalent about whether we should do this unconditionally or only in C++11 mode.

[rjmccall]

Lovely drafting, too. (foo(), *v) gets a conversion, but (foo(), (foo(), *v)) doesn't. :)

[llvmbot]

Also, "conditional expression (5.16 [expr.cond]) where both the second and the third operand are one of the above, or" So:

bool zonk;
volatile int a, b;
zonk ? a : b; // is evaluated
zonk ? (zonk ? a : b) : b; // is not evaluated

because you can't nest conditional expressions.

How does everyone else feel about 'interpreting' "one of the above" as meaning "satisfies the conditions in this list"? That means that both of the above (and John's example in comment 4) would get conversions. Or should we take it up with the committee first, and implement it exactly as written until then?

It would be very convenient for me if these conversions happened in C++03 :) It also closes the gap between volatile behaviour between C and C++, which is desirable.

[rjmccall]

How does everyone else feel about 'interpreting' "one of the above" as meaning
"satisfies the conditions in this list"? That means that both of the above (and
John's example in comment 4) would get conversions.

I agree that this is the only reasonable interpretation for both of these.

Other areas for interpretation:

1. Does the lvalue-to-rvalue conversion happen for non-POD volatile types, too? Presumably yes. Does it matter that very few types provide a volatile copy constructor?

2. Are these syntactic forms or semantic? Note the explicit presence of subscripting, when that's defined to be equivalent to an indirection. So:

• Does "subscripting" include user-defined operator[]s that yield volatile l-values?
• Does "indirection" include user-defined operator*s that yield volatile l-values?
• Does "pointer-to-member operation" include user-defined operator->*s that yield volatile l-values?
• Does "comma" include user-defined operator,s?
  I really don't know how to interpret this one.

It would be very convenient for me if these conversions happened in C++03 :) It
also closes the gap between volatile behaviour between C and C++, which is
desirable.

It would have been great if this difference had never existed, but at this point, you're talking about silently changing the behavior of twenty-year-old code. This is why we have -std=c++0x.

[llvmbot]

It would have been great if this difference had never existed, but at this
point, you're talking about silently changing the behavior of twenty-year-old
code. This is why we have -std=c++0x.

Well, I initially discovered this as a difference between gcc and clang in C++03 mode; gcc preserves the volatile dereference, clang doesn't. For many users that compatibility break is already going to be there.

AFAIK, C++ never says anywhere that volatile accesses are tied to lvalue-to-rvalue conversion. It does say in a note that "in general, the semantics of volatile are intended to be the same in C++ as they are in C", and in this case C does require that a dereference occur in "volatile int *v; *v;".

I really consider this to be a clarification to unspecified behaviour that ought to be applied in 03 mode -- if you still aren't convinced, I'll defer to your interpretation.

[rjmccall]

It does say in a note that "in general, the semantics of volatile are intended
to be the same in C++ as they are in C",

I take this to mean that volatile accesses are expected to have the same
semantics they do in C, not that we should infer volatile accesses in
exactly the same places. I mean, we don't take this as license to synthesize
volatile copy constructors for aggregates; nor does the new rule do exactly
the same thing as C for conditional operators.

I really consider this to be a clarification to unspecified behaviour that
ought to be applied in 03 mode -- if you still aren't convinced, I'll defer to
your interpretation.

I'll talk it over with Doug. The problem is that it really wasn't unspecified;
it was specified perfectly clearly in exactly the same way that C specified it,
i.e. by applying the generic rules about when to perform lvalue-to-rvalue
conversions.

Also, GCC does not appear to do lvalue-to-rvalue conversion on volatile
non-PODs, and that's pretty clearly wrong by the new rule.

[llvmbot]

partial fix
Attached is my unfinished fix for the bug; I don't know what to do about this case:
struct S { int i; };
volatile struct S s;
s;
I'm thinking that since the struct has no volatile copy or assign, that we should issue a syntax error, but I wasn't able to navigate my way through Sema to make that happen.

Thus far, the patch appears to replicate GCC's behaviour (emitting no code for the "s;" line), which is not really what we want.

[llvmbot]

Closing "debatably valid and probably wontfix even if it is". We've worked around (or fixed) this in our codebase and this is something that it more solidly defined in C++11, so I don't really see the point in this PR any more.