sgatev marked an inline comment as done.
sgatev added inline comments.
================
Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:114
+ } else if (S->getCastKind() == CK_NoOp) {
+ auto *SubExprLoc = Env.getStorageLocation(*SubExpr, SkipPast::None);
+ if (SubExprLoc == nullptr)
----------------
xazax.hun wrote:
> An alternative way to handle Noop operations would be to make location lookup
> function always skip certain nodes, so we do not need to store locations for
> those subexpressions. I don't have a strong feeling for either solution, this
> is fine as it is, just wanted to be sure that both were considered.
It makes sense to consider it. Right now I don't have signals that one is
better than the other. I think the current implementation fits the general
model a bit better. Nevertheless, I added a FIXME to keep this option in mind.
================
Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:277
+ void VisitCallExpr(const CallExpr *S) {
+ if (S->isCallToStdMove()) {
+ assert(S->getNumArgs() == 1);
----------------
xazax.hun wrote:
> Interesting, I only see `isCallToStdMove` in the CallExpr API, although I
> imagine, `std::forward` has a similar level of importance.
I haven't looked into `std::forward` yet, but happy to do that in the next
patch.
================
Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:39
protected:
+ enum class CppVersion {
+ k14 = 14,
----------------
xazax.hun wrote:
> Shouldn't we piggyback on `clang::LangStandard::Kind`? If it is not easy to
> convert that to the appropriate command line flag feel free to ignore this.
Makes sense. Thanks for pointing this out!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D117218/new/
https://reviews.llvm.org/D117218
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
