ymandel accepted this revision.
ymandel added inline comments.
This revision is now accepted and ready to land.
================
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))
----------------
mboehme wrote:
> 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?
I see Martin's point in not wanting to use copy/assign, since the same value
will be shared between the locations. but, I see why "forward" has multiple
meanings. Maybe "share" or "propogate"?
================
Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:751-752
+ Env.setValue(Loc, *Val);
+ } else {
+ if (Value *Val = Env.createValue(S->getType()))
+ Env.setValueStrict(*S, *Val);
----------------
just `else if`?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D150655/new/
https://reviews.llvm.org/D150655
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
