[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added inline comments.


================
Comment at: lib/Sema/SemaExpr.cpp:11292
+}
+
+/// Argument's value might be modified, so update the info.
----------------
Hmm, I don't think that this will work. Suppose that you have an expression like `(a, b) += c`  You want to mark `b` as modified, but not `a`. What is needed I believe is:

Given an expression `E` find the variable that this expression designate if any, and if it is a function parameter mark it "known to be modified".

Visiting all of the children just looking for `DeclRefExpr`s is a bit too blunt I believe.


================
Comment at: lib/Sema/SemaExpr.cpp:11316
     IsLV = Expr::MLV_InvalidMessageExpression;
-  if (IsLV == Expr::MLV_Valid)
+  if (IsLV == Expr::MLV_Valid) {
+    EmitArgumentsValueModification(E);
----------------
> Hmm, we should avoid marking variables modification if this emits an error. But, we should emit it if `CheckForModifiableLvalue`  returns false, since in the case of returning true an error will be emitted.

You are right, I believed for a moment that `CheckForModifiableLvalue` returned false on error.


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

https://reviews.llvm.org/D58035