courbet added a comment.
> Is this actually required for the subsequent change? I don't see the
> connection.
In the followup change, we have to check the returns after the enter and exit
CFG block are computed. We can't analyze the returns as they are seen because ,
because what matters for the returns is the locks that are live at the end of
the function, not those that are live at the point where the `return` happens.
The `BuildLockset` class only lives for the duration of the analysis of a
single block, while the `ThreadSafetyAnalyzer` lives for the whole function. So
return checking is done in the `ThreadSafetyAnalyzer`, so we need the
check/warn functions to be available here.
From a design perspective I think it might actually make more sens for them to
be in the analyzer as `warnIfMutexNotHeld` and friends actually inspects quite
a lot of the `Analyzer` state.
================
Comment at: clang/lib/Analysis/ThreadSafety.cpp:1541
class BuildLockset : public ConstStmtVisitor<BuildLockset> {
+ using VisitorBase = ConstStmtVisitor<BuildLockset>;
friend class ThreadSafetyAnalyzer;
----------------
aaronpuchert wrote:
> Why the alias? I find this just obfuscates.
I'm using it one more time in the followup patch, but no strong opinion,
removed.
================
Comment at: clang/lib/Analysis/ThreadSafety.cpp:1588
/// of at least the passed in AccessKind.
-void BuildLockset::warnIfMutexNotHeld(const NamedDecl *D, const Expr *Exp,
- AccessKind AK, Expr *MutexExp,
- ProtectedOperationKind POK,
- til::LiteralPtr *Self,
- SourceLocation Loc) {
+void ThreadSafetyAnalyzer::warnIfMutexNotHeld(
+ const FactSet &FSet, const NamedDecl *D, const Expr *Exp, AccessKind AK,
----------------
aaronpuchert wrote:
> Hmm, functions of the same class naturally want to be together, but if you
> move them, it "destroys" the Git history.
Yes, I decided to make review/history tracking esaier by not moving them, but I
can move them if you want.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153132/new/
https://reviews.llvm.org/D153132
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
