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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
