tomasz-kaminski-sonarsource marked an inline comment as done.
tomasz-kaminski-sonarsource added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp:556
+ bool IsLocal =
+ isa_and_nonnull<VarRegion, CXXLifetimeExtendedObjectRegion>(MR) &&
+ isa<StackSpaceRegion>(MR->getMemorySpace());
----------------
xazax.hun wrote:
> I think strictly speaking this might be "unsound" resulting in false
> positives in scenarios like:
> ```
> void f(bool reset) {
> static T&& extended = getTemp();
> if (reset) {
> extended = getTemp();
> return;
> }
> consume(std::move(extended));
> f(true);
> extended.use();
> }
> ```
>
> In case the call to `f(true)` is not inlined (or in other cases there is some
> aliasing the analyzer does not know about), we might not realize that the
> object is reset and might falsely report a use after move.
>
> But I think this is probably sufficiently rare that we do not care about
> those.
Aren't such case already excluded by the
`isa<StackSpaceRegion>(MR->getMemorySpace())`, in a same way as we do for
variables?
In such case, we should be creating `getCXXStaticLifetimeExtendedObjectRegion`
so it will not be located on stack.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp:101
+ QualType Ty = LER->getValueType().getLocalUnqualifiedType();
+ os << "stack memory associated with temporary object of type '";
+ Ty.print(os, Ctx.getPrintingPolicy());
----------------
xazax.hun wrote:
> Is this a good wording for static lifetime extended temporaries?
Lifetime extended static temporaries should never enter here, as they are not
in `StackSpaceRegion`.
Previously we have had `CXXStaticTempObjects` and now they are turned into
`CXXStaticLifetimeExtendedObjectRegion`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D151325/new/
https://reviews.llvm.org/D151325
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
