[PATCH] D103395: PR45879: Keep evaluated expression in LValue object

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

P1330R0 (https://wg21.link/P1330R0) allowed changing the active member of a union in constexpr expressions. To implement it the Constant Evaluator starting from
https://github.com/llvm/llvm-project/commit/31c69a3d6363463c08b86914c0c8cfc5c929c37e checks if an assignment changes active member of a union. This is why a change proposed for union only affects compilation of code that does not contain unions at all. The issue is in the assignment treatment.

The check is made by the function `HandleUnionActiveMemberChange`, which is called from `VisitBinAssign` and `HandleFunctionCall`, they handle builtin and trivial assignment operators respectively. The check is made according to the rule specified in the standard (http://eel.is/c++draft/class.union#general-6), which is based on syntactic form. Constant evaluator on the other hand uses objects of class `APValue` and related `LValue` to make the evaluations.  In some cases it is not possible to get `Expr` used for the evaluation.

Consider slightly modified test case provided in the bug report:

  struct Base { int m; };
  struct Derived : Base {};
   
  constexpr int func_01(int x) {
    Base v1{1};
    Derived v2{x};
    v1 = v2;
    return v1.m;
  }
   
  constexpr int k = func_01(0);

When Constant Evaluator evaluates the expression `v1 = v2`, it eventually calls `LValueExprEvaluator::handleCallExpr`  as this expression is a call of assignment operator. This is a method call, so the function evaluates the object reference, an `LValue`, which represents `v1` as `ValueDecl`. Then `HandleFunctionCall` is called, provided with the `LValue` for `v1` and an expression for `v2`. This function calls `HandleUnionActiveMemberChange`, which checks if the assignment changes an union active member.

`HandleUnionActiveMemberChange` makes the check according to the standard, so it needs `v1` in the form of expression. However, `v1` is represented by `LValue` in `HandleFunctionCall`, which contains `ValueDecl` and the original expression is unavailable. It makes the check impossible.

To make the check `HandleUnionActiveMemberChange` must be provided by the expression representing `v1`.  A simple solution proposed here is to keep the original expression in `LValue`. It is set when `LValue` is evaluated when by `LValueExprEvaluator` in its method `Evaluate`. If the evaluation produces `LValue` based on something other than an expression, the reference to the original method remains in that `LValue` and may be used when the original expression is required.

The obtained solution is low-invasive. Actually only the changes in `LValueExprEvaluator::Evaluate` and `Lvalue::set` are essential for maintaining expression reference in `LValue`, and the change in `HandleUnionActiveMemberChange` is the only usage of it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103395/new/

https://reviews.llvm.org/D103395