[PATCH] D150655: [clang][dataflow] Use `Strict` accessors in more places in Transfer.cpp.

[Martin Böhme via Phabricator via cfe-commits]

mboehme added inline comments.


================
Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:138
 
+static void forwardValue(const Expr &From, const Expr &To, Environment &Env) {
+  if (auto *Val = Env.getValueStrict(From))
----------------
sammccall wrote:
> the name "forward" isn't clear to me - if anything suggesting std::forward, but not assignment.
> 
> Maybe `assignValue` or `copyValue`, and `aliasLocation` for the storageloc variant?
> 
> That leaves "forwardValueOrStorageLocation", but that only has one callsite and part of the point of this refactoring is that we're best thinking about these cases explicitly, right? So could just inline the if.
(Haven't made any changes yet because I'd like to discuss first.)

> That leaves "forwardValueOrStorageLocation", but that only has one callsite and part of the point of this refactoring is that we're best thinking about these cases explicitly, right? So could just inline the if.

This function will end up being used in more places though. I have some draft patches that use it (for the comma operator, for example) but these aren't ready for review yet. But as I know the operation will be used elsewhere, it made sense to create an abstraction now (and we'll definitely need a name for it at some point).

I think the most generic of the names you suggest is `copy`, and I think it works reasonably well: `copyValue()`, `copyStorageLocation()`, `copyValueOrStorageLocation()`. WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150655