Assignment expression in C++ returns its right operand (A possible bug)

Hi everyone,

I found a surprise in the llvm code generated by clang. (I am using
clang version 3.0. And the revision is git-svn-id:
https://llvm.org/svn/llvm-project/cfe/trunk@137823). Though my
original example is the swap function for STL string, I created one
simple example as below to show the problem I found.

class A {
public:
  int *foo(int *s) {
    return x = s;
  }
private:
  int *x;
};

int main() {
  A a;
  int x;
  int *px = &x;
  a.foo(px);
  return 0;
}

In the above example, foo supposes to return x. However, its
corresponding llvm bitcode (by clang) seems to return s. The llvm
bitcode is as below:

define linkonce_odr i32* @_ZN1A3fooEPi(%class.A* %this, i32* %s) nounwind uwtable ssp align 2 {
entry:
  %this.addr = alloca %class.A*, align 8, !tbaa !10, !clang.vardecl.type !10
  %s.addr = alloca i32*, align 8, !tbaa !5, !clang.vardecl.type !5
  store %class.A* %this, %class.A** %this.addr, align 8, !tbaa !10
  store i32* %s, i32** %s.addr, align 8, !tbaa !5
  %this1 = load %class.A** %this.addr, !tbaa !10
  %tmp = load i32** %s.addr, align 8, !tbaa !5
  %x = getelementptr inbounds %class.A* %this1, i32 0, i32 0
  store i32* %tmp, i32** %x, align 8, !tbaa !5
  ret i32* %tmp
}

Note that the register value denoted by %tmp is returned, but %tmp is
the value of the second parameter. (This does not agree with the c++
semantics.)

Again I am using clang version 3.0. And the revision is git-svn-id:
https://llvm.org/svn/llvm-project/cfe/trunk@137823

If I am correct and the above problem has been fixed, can someone
point to me the commit where the problem is fixed? If I am somehow
wrong, can someone give me any explanation?

Thanks!

Xiaolong

clang -cc1 -ast-dump
would show there is an lvalue-to-rvalue cast for 'x=s;'

The relevant code is in ScalarExprEmitter::VisitBinAssign(const BinaryOperator *E).

于 2012/2/17 6:37, Xiaolong Tang 写道:

As David says, it's not a problem. You are correct that C++ semantics
say that the RHS is stored to the LHS and then the result of the
expression is the same l-value as the original LHS. However, when
that l-value is non-volatile, we are not strictly bound by those
semantics. In this case, it is clear that the object identified by the LHS
is not modified by this thread between the store and the load, and
any other thread modifying it would be a forbidden race condition.
Therefore, if the result of this assignment is needed as an r-value,
we can simply forward the original value (suitably truncated if the
l-value is a bitfield).

John.

BTW: John, wouldn't it be a very good thing to have a
BitfieldTruncateExpr AST node (or an extension of ImplicitCastExpr)?

I don't see this as tremendously useful; it's not particularly difficult to
detect that there's a potential bitfield truncation occurring as part of an
assignment. The bigger problem, though, is that this doesn't really
model the language; the truncation happens as part of the assignment,
not as part of the evaluation of the right-hand side. That distinction
matters a lot when the operation is a compound assignment or when
(and many sarcastic thanks go to C++ for this wonderful feature) the
LHS is a conditional bitfield l-value.

John.

For the former case (compound assignment) we wanted to suggest to use
OpaqueValueExpr to properly model that case (as it seems sensible and
appropriate).

But about the conditional bitfield l-value, I definitely didn't know
that... I'm astonished. I've also noted that clang is currenty unable to
compile that.