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))
----------------
ymandel wrote:
> 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"?
I like "propagate", since it conveys the same meaning that I was going for with
"forward", but without the allusion to `std::forward`. I like it better than
"copy" or "assign" because those also have specific meanings in C++, and
"propagate" doesn't.
@sammccall 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
