aaronpuchert added reviewers: aaron.ballman, aaronpuchert.
aaronpuchert added a comment.
Thanks for working on this! Someone recently pointed out to me that we have a
gap there.
================
Comment at: clang/include/clang/Basic/DiagnosticGroups.td:1046
def ThreadSafetyPrecise : DiagGroup<"thread-safety-precise">;
def ThreadSafetyReference : DiagGroup<"thread-safety-reference">;
+def ThreadSafetyReturn : DiagGroup<"thread-safety-return">;
----------------
Why not under `-Wthread-safety-reference`, as it's return-by-reference that
you're warning on? This seems too small for a separate flag to me.
================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3805
+// Thread safety warnings on return
+def warn_guarded_return_by_reference : Warning<
----------------
Or do you expect more warnings on return?
================
Comment at: clang/lib/Analysis/ThreadSafety.cpp:2159-2161
+ // If returning by reference, add the return value to the set to check on
+ // function exit.
+ if (RetVal->isLValue()) {
----------------
Wouldn't it be more straightforward to check the actual return type? We have
the `FunctionDecl` and could store it in `ThreadSafetyAnalyzer` instead of
`CurrentMethod`.
================
Comment at: clang/lib/Analysis/ThreadSafety.cpp:2162
+ if (RetVal->isLValue()) {
+ Analyzer->ReturnValues.insert(RetVal);
+ }
----------------
You're presumably collecting them because automatic destructor calls are after
`return` in the CFG, right?
If that's the case, can't we immediately check against the declared exit set?
It should be known before we walk the CFG, unless I'm missing something.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153131/new/
https://reviews.llvm.org/D153131
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
