aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land.
In D129755#3777038 <https://reviews.llvm.org/D129755#3777038>, @aaronpuchert wrote: > I was under the impression that we've already switched to C++17, but the > Windows pre-submit build failed with: > > > C:\ws\w9\llvm-project\premerge-checks\clang\lib\Analysis\ThreadSafety.cpp(2107): > error C2429: language feature 'init-statements in if/switch' requires > compiler flag '/std:c++17' > > C:\ws\w9\llvm-project\premerge-checks\clang\lib\Analysis\ThreadSafety.cpp(2120): > error C2429: language feature 'init-statements in if/switch' requires > compiler flag '/std:c++17' > > C:\ws\w9\llvm-project\premerge-checks\clang\lib\Analysis\ThreadSafety.cpp(2418): > error C2429: language feature 'init-statements in if/switch' requires > compiler flag '/std:c++17' > > Perhaps I should move the init statements out again? No, I've filed https://github.com/google/llvm-premerge-checks/issues/416 to find out what's going on with the Windows precommit CI bot. The changes LGTM, though it'd still be great if @delesley had some time to put a second set of eyes on the changes. If we don't hear from him by Tue, I think we should go ahead and land this. Please be sure to add a release note for the changes! ================ Comment at: clang/lib/Analysis/ThreadSafetyCommon.cpp:339-342 + if (const Expr *SelfArg = dyn_cast<const Expr *>(Ctx->SelfArg)) + return translate(SelfArg, Ctx->Prev); + else + return cast<til::SExpr *>(Ctx->SelfArg); ---------------- aaronpuchert wrote: > aaron.ballman wrote: > > > No doubt referring to > https://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return. I > was trying to emphasize the dual nature of `llvm::PointerUnion<const Expr *, > til::SExpr *>`, but I can certainly also drop the `else`. I don't have a strong opinion. ================ Comment at: clang/test/SemaCXX/warn-thread-safety-analysis.cpp:4230-4236 +void testDeferredTemporary() { + SelfLockDeferred(); // expected-warning {{releasing mutex '<temporary>.mu_' that was not held}} +} + +void testDeferredTemporary2() { + SelfLockDeferred2(); // expected-warning {{releasing mutex '<temporary>' that was not held}} +} ---------------- aaronpuchert wrote: > Here we're printing `<temporary>`, because we don't have a name for this > object. Ah thank you, this example makes a lot of sense. I think printing `<temporary>` is quite reasonable. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129755/new/ https://reviews.llvm.org/D129755 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
