[PATCH] D84603: Thread safety analysis: More consistent warning message

2020-07-26 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert created this revision. aaronpuchert added a reviewer: aaron.ballman. Herald added a project: clang. Herald added a subscriber: cfe-commits. Other warning messages for negative capabilities also mention their kind, and the double space was ugly. Repository: rG LLVM Github Monorepo

[PATCH] D84604: Thread safety analysis: Consider global variables in scope

2020-07-26 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert created this revision. aaronpuchert added reviewers: aaron.ballman, delesley. Herald added a project: clang. Herald added a subscriber: cfe-commits. Instead of just mutex members we also consider mutex globals. Unsurprisingly they are always in scope. Now the paper says that The sc

[PATCH] D84604: Thread safety analysis: Consider global variables in scope

2020-07-26 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a subscriber: anarazel. aaronpuchert added a comment. @anarazel, that should fix the issue you reported on IRC. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84604/new/ https://reviews.llvm.org/D84604 _

[PATCH] D84604: Thread safety analysis: Consider global variables in scope

2020-07-26 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. Test failure is expected because I based this on D84603 locally, but I'm not sure how to tell that to Phabricator. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84604/new/ https://re

[PATCH] D84603: Thread safety analysis: More consistent warning message

2020-07-27 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added inline comments. Comment at: clang/lib/Analysis/ThreadSafety.cpp:1644 if (!LDat) { - Analyzer->Handler.handleMutexNotHeld("", D, POK, Cp.toString(), - LK_Shared, Loc); + Analyzer->Handler.handleMutexNotHe

[PATCH] D76038: PR45000: Use Sema::SetParamDefaultArgument in TransformLambdaExpr

2020-04-13 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. In D76038#1978667 , @mstorsjo wrote: > Ping - any update on this one? Are you waiting on @rsmith for input on where > to place a testcase? No, I just didn't get around to writing the test yet, thanks for reminding me. Alth

[PATCH] D76038: PR45000: Use Sema::SetParamDefaultArgument in TransformLambdaExpr

2020-04-15 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. I tested a bit more and there is a problem: some errors now appear twice, and I think they shouldn't. template void f(int x = [](T x = nullptr) -> int { return x; }()); void g() { f(); } The error is expected, but it should only appear once: :2:23: err

[PATCH] D76038: PR45000: Use Sema::SetParamDefaultArgument in TransformLambdaExpr

2020-04-15 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert updated this revision to Diff 257938. aaronpuchert added a comment. Add test case, loosely based on that in the bug report. By choosing a conversion that should error out we can detect the issue in a pure frontend test. However, we would expect the error only once. Further investig

[PATCH] D81332: Thread safety analysis: Support deferring locks

2020-06-06 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert created this revision. aaronpuchert added reviewers: aaron.ballman, delesley. Herald added a project: clang. Herald added a subscriber: cfe-commits. The standard std::unique_lock can be constructed to manage a lock without initially acquiring it by passing std::defer_lock as second pa

[PATCH] D81352: Thread safety analysis: Add note for double unlock

2020-06-07 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert created this revision. aaronpuchert added reviewers: aaron.ballman, delesley. Herald added a project: clang. Herald added a subscriber: cfe-commits. When getting a warning that we release a capability that isn't held it's sometimes not clear why. So just like we do for double locking,

[PATCH] D81352: Thread safety analysis: Add note for double unlock

2020-06-08 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert marked 2 inline comments as done. aaronpuchert added inline comments. Comment at: clang/include/clang/Analysis/Analyses/ThreadSafety.h:111 /// \param Loc -- The SourceLocation of the Unlock + /// \param LocPreviousUnlock -- Optionally the location of a previous U

[PATCH] D81332: Thread safety analysis: Support deferring locks

2020-06-08 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. In D81332#2079489 , @aaron.ballman wrote: > LGTM! Should we update the public docs for this change as well? Specifically, > I am wondering if we should update > https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#mutexhea

[PATCH] D81352: Thread safety analysis: Add note for double unlock

2020-06-08 Thread Aaron Puchert via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. aaronpuchert marked an inline comment as done. Closed by commit rGf70912f885f9: Thread safety analysis: Add note for double unlock (authored by aaronpuchert). Changed prior to commit: https://reviews.llvm.org/D81352?vs=26

[PATCH] D81332: Thread safety analysis: Support deferring locks

2020-06-08 Thread Aaron Puchert via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG1850f56c8aba: Thread safety analysis: Support deferring locks (authored by aaronpuchert). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81332/new/ https://r

[PATCH] D67321: Respect CLANG_LINK_CLANG_DYLIB=ON in libclang and c-index-test

2020-06-19 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. Yup, `c-index-test` crashing was one of the motivators behind this. I think this should handle all cases. I tried it with `CLANG_LINK_CLANG_DYLIB` on/off combined with shared/static/shared+static libraries and inspected the generated `build.ninja`, which looked abo

[PATCH] D76038: PR45000: Use Sema::SetParamDefaultArgument in TransformLambdaExpr

2020-04-16 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert marked 2 inline comments as done. aaronpuchert added inline comments. Comment at: clang/lib/Sema/TreeTransform.h:12162 for (unsigned I = 0, NumParams = NewCallOperator->getNumParams(); I != NumParams; ++I) { rsmith wrote: > rsmith wrote:

[PATCH] D76038: PR45000: Use Sema::SetParamDefaultArgument in TransformLambdaExpr

2020-04-16 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert marked an inline comment as done. aaronpuchert added inline comments. Comment at: clang/lib/Sema/TreeTransform.h:12162 for (unsigned I = 0, NumParams = NewCallOperator->getNumParams(); I != NumParams; ++I) { aaronpuchert wrote: > rsmith

[PATCH] D76038: PR45000: Use Sema::SetParamDefaultArgument in TransformLambdaExpr

2020-04-17 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. Let's have a look at an example. Do we allow this? template int x = [](T = T()){ return 0; }(args...); template int x; // Uses default argument int() = 0. struct S { S(int); }; template int x; // Default argument not well-formed, but not needed. Curr

[PATCH] D76038: PR45000: Let Sema::SubstParmVarDecl handle default args of lambdas in initializers

2020-04-19 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert updated this revision to Diff 258627. aaronpuchert added a comment. Remove loop in TreeTransform::TransformLambdaExpr and make sure Sema::SubstParmVarDecl handles the situation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76038/new/

[PATCH] D76038: PR45000: Let Sema::SubstParmVarDecl handle default args of lambdas in initializers

2020-04-19 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added subscribers: ahatanak, sepavloff. aaronpuchert added a comment. Adding @ahatanak and @sepavloff since I'm effectively reverting D23096 now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76038/new

[PATCH] D76038: PR45000: Let Sema::SubstParmVarDecl handle default args of lambdas in initializers

2020-04-19 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert marked 2 inline comments as done. aaronpuchert added a comment. This seems to be in line with our current behavior, so I hope it's right: struct S { S(int); }; template auto l = [](T x = T()) { return x; }; template struct Fun { T operator()(T x = T()) const { r

[PATCH] D76038: PR45000: Let Sema::SubstParmVarDecl handle default args of lambdas in initializers

2020-04-19 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert updated this revision to Diff 258628. aaronpuchert edited the summary of this revision. aaronpuchert added a comment. Slightly adapt test results: there is an additional error now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76038/new

[PATCH] D76038: PR45000: Let Sema::SubstParmVarDecl handle default args of lambdas in initializers

2020-04-22 Thread Aaron Puchert via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGf43859a099fa: PR45000: Let Sema::SubstParmVarDecl handle default args of lambdas in… (authored by aaronpuchert). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D67112: [Sema] Introduce function reference conversion, NFC

2020-04-22 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. @rsmith, what do you think about this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67112/new/ https://reviews.llvm.org/D67112 ___ cfe-commits mailing list cfe-commits@li

[PATCH] D78853: [Analysis] Fix null pointer dereference warnings [1/n]

2020-04-28 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. Could you explain why we are doing this? Clang has its own static analyzer , which can find null dereferences as well but also tracks constraints to

[PATCH] D132791: Fix formatting in release notes

2022-08-27 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert created this revision. aaronpuchert added reviewers: aaron.ballman, thieta, tstellar. Herald added a reviewer: alexander-shaposhnikov. Herald added a project: All. aaronpuchert requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits

[PATCH] D132791: Fix formatting in release notes

2022-08-27 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. This is for `release/15.x`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132791/new/ https://reviews.llvm.org/D132791 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D132791: Fix formatting in release notes

2022-08-27 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert updated this revision to Diff 456131. aaronpuchert added a comment. Use links instead `:manpage:`, which is meant for man pages cross-referencing each other. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132791/new/ https://reviews.ll

[PATCH] D132791: Fix formatting in release notes

2022-08-29 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. The documentation build (`docs-{llvm,clang}-{html,man}`) is fine after this, in fact it fixes: Warning, treated as error: [...]/build/tools/clang/docs/ReleaseNotes.rst:632:Bullet list ends without a blank line; unexpected unindent. by adding some indentation.

[PATCH] D132791: Fix formatting in release notes

2022-08-29 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert closed this revision. aaronpuchert added a comment. Landed in 0c5ce1d7fba38948c27ed6b875f962cd60895574 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132791/new/ ht

[PATCH] D133009: [libclang] Fix conversion from `StringRef` to `CXString`

2022-08-31 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added inline comments. Comment at: clang/test/Index/comment-lots-of-unknown-commands.c:3 +// XFAIL: * + egorzhdan wrote: > gribozavr2 wrote: > > egorzhdan wrote: > > > This test was never properly passing. Because of the bug in string > > > conver

[PATCH] D133105: [Clang][Comments] Fix `Index/comment-lots-of-unknown-commands.c`

2022-09-01 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert accepted this revision. aaronpuchert added a comment. This revision is now accepted and ready to land. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133105/new/ https://reviews.llvm.org/D133105

[PATCH] D129755: Thread safety analysis: Support copy-elided production of scoped capabilities through arbitrary calls

2022-09-07 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert marked 3 inline comments as done. aaronpuchert added inline comments. Comment at: clang/docs/ThreadSafetyAnalysis.rst:935 +// Same as constructors, but without tag types. (Requires C++17 copy elision.) +static MutexLocker Lock(Mutex *mu) ACQUIRE(mu) NO_THREA

[PATCH] D129755: Thread safety analysis: Support copy-elided production of scoped capabilities through arbitrary calls

2022-09-07 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert updated this revision to Diff 458572. aaronpuchert marked 3 inline comments as done. aaronpuchert added a comment. Use `SmallDenseMap` plus some minor changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129755/new/ https://reviews.l

[PATCH] D129755: Thread safety analysis: Support copy-elided production of scoped capabilities through arbitrary calls

2022-09-07 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added inline comments. Comment at: clang/test/SemaCXX/warn-thread-safety-analysis.cpp:4230-4236 +void testDeferredTemporary() { + SelfLockDeferred(); // expected-warning {{releasing mutex '.mu_' that was not held}} +} + +void testDeferredTemporary2() { + SelfLockD

[PATCH] D129755: Thread safety analysis: Support copy-elided production of scoped capabilities through arbitrary calls

2022-09-08 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. 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

[PATCH] D111283: [clang] template / auto deduction deduces common sugar

2022-09-13 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:12367 + case Type::Class: \ +llvm_unreachable("Unexpected " Kind ": " #Class); + mizvekov wrote: > davrec wrote: > > mizv

[PATCH] D111283: [clang] template / auto deduction deduces common sugar

2022-09-14 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:12367 + case Type::Class: \ +llvm_unreachable("Unexpected " Kind ": " #Class); + mizvekov wrote: > aaronpuchert wrote: >

[PATCH] D122546: Let clang-repl link privately against Clang components

2022-03-27 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert created this revision. aaronpuchert added a reviewer: v.g.vassilev. Herald added a subscriber: mgorny. Herald added a project: All. aaronpuchert requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. First of all, this is the conventi

[PATCH] D122546: Let clang-repl link privately against Clang components

2022-03-28 Thread Aaron Puchert via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG1721d52a6206: Let clang-repl link privately against Clang components (authored by aaronpuchert). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122546/new/ h

[PATCH] D122544: Utilize comparison operation implemented in APInt

2022-03-28 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. Note that `__int128_t` is not available everywhere, generally only on 64-bit platforms. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-sizeof-expression.cpp:175 // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: suspicious usage of si

[PATCH] D129752: Thread safety analysis: Handle additional cast in scoped capability construction

2022-07-28 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert marked an inline comment as done. aaronpuchert added a comment. In D129752#3682977 , @aaron.ballman wrote: > Do you think this warrants a release note (or did it close any open issues in > the tracker)? It's probably too technical/minor to

[PATCH] D106375: Thread safety analysis: Mock getter for private mutexes can be undefined

2021-07-20 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert created this revision. aaronpuchert added a reviewer: aaron.ballman. aaronpuchert requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Usage in an annotation is no odr-use, so I think there needs to be no definition. Upside is that

[PATCH] D101566: Let -Wweak-template-vtables warn on implicit instantiations

2021-07-20 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. @dblaikie, does https://bugs.llvm.org/show_bug.cgi?id=18733#c17 change anything about your position? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101566/new/ https://reviews.llvm.org/D101566

[PATCH] D101566: Let -Wweak-template-vtables warn on implicit instantiations

2021-07-20 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a subscriber: dfaure-kdab. aaronpuchert added a comment. In D101566#2891764 , @dblaikie wrote: > This patch is still conflating two things - effectively removing an existing > warning (which I agree with) and adding a new one (which I

[PATCH] D101566: Let -Wweak-template-vtables warn on implicit instantiations

2021-07-20 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. In D101566#2891972 , @dblaikie wrote: > For Rafael - probably because he didn't look at all the cases the warning > does catch & see that it's pretty much entirely no use Right, he didn't suggest this particular fix but ano

[PATCH] D101566: Let -Wweak-template-vtables warn on implicit instantiations

2021-07-20 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a reviewer: aaron.ballman. aaronpuchert added a subscriber: aaron.ballman. aaronpuchert added a comment. Let's add @aaron.ballman to get a third opinion. The discussion is meandering a bit, but you should understand the gist from the first comments or the bug report. The quest

[PATCH] D101566: Let -Wweak-template-vtables warn on implicit instantiations

2021-07-22 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert abandoned this revision. aaronpuchert added a comment. In D101566#2896911 , @aaron.ballman wrote: > Off-by-default warnings are generally discouraged unless there's a very > compelling reason to have them. There are IMO valuable warnings t

[PATCH] D106375: Thread safety analysis: Mock getter for private mutexes can be undefined

2021-07-23 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. In D106375#2896484 , @aaron.ballman wrote: > LGTM! It took me a minute to convince myself it wasn't really an ODR use (the > argument goes through the usual expression evaluation parsing and sema bits), > but because the a

[PATCH] D106375: Thread safety analysis: Mock getter for private mutexes can be undefined

2021-07-23 Thread Aaron Puchert via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0e64a525c12a: Thread safety analysis: Mock getter for private mutexes can be undefined (authored by aaronpuchert). Repository: rG LLVM Github Mono

[PATCH] D106713: Thread safety: Warn when demoting locks on back edges

2021-07-23 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert created this revision. aaronpuchert added reviewers: aaron.ballman, delesley. aaronpuchert requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Previously in D104261 we warned about dropping locks

[PATCH] D106715: Thread safety analysis: Drop special block handling

2021-07-23 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert created this revision. aaronpuchert added reviewers: aaron.ballman, delesley. aaronpuchert requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Previous changes like D101202 and D104261

[PATCH] D106715: Thread safety analysis: Drop special block handling

2021-07-26 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert updated this revision to Diff 361820. aaronpuchert marked 2 inline comments as done. aaronpuchert added a comment. Drop `SpecialBlocks` and fix clang-tidy issue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106715/new/ https://reviews

[PATCH] D100713: [clang] NFC: refactor multiple implementations of getDecltypeForParenthesizedExpr

2021-07-28 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert accepted this revision. aaronpuchert added a comment. This revision is now accepted and ready to land. It is a good name! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100713/new/ https://reviews.llvm.org/D100713 _

[PATCH] D124127: Thread safety analysis: Pack CapabilityExpr using PointerIntPair (NFC)

2022-04-20 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert created this revision. aaronpuchert added a reviewer: aaron.ballman. Herald added a project: All. aaronpuchert requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. We're storing these quite frequently: FactEntry inherits from Capabi

[PATCH] D124128: Thread safety analysis: Store CapabilityExprs in ScopedLockableFactEntry (NFC)

2022-04-20 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert created this revision. aaronpuchert added a reviewer: aaron.ballman. Herald added a project: All. aaronpuchert requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. For now this doesn't make a whole lot of sense, but it will allow us

[PATCH] D124131: Thread safety analysis: Store capability kind in CapabilityExpr

2022-04-20 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert created this revision. aaronpuchert added a reviewer: aaron.ballman. Herald added a project: All. aaronpuchert requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This should make us print the right capability kind in many more cas

[PATCH] D124132: Thread safety analysis: Don't pass capability kind where not needed (NFC)

2022-04-20 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert created this revision. aaronpuchert added a reviewer: aaron.ballman. Herald added a project: All. aaronpuchert requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. If no capability is held, or the capability expression is invalid, t

[PATCH] D124131: Thread safety analysis: Store capability kind in CapabilityExpr

2022-04-22 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added inline comments. Comment at: clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h:277 + /// The kind of capability as specified by @ref CapabilityAttr::getName. + StringRef CapKind; + aaron.ballman wrote: > Hr, I think this may actu

[PATCH] D97625: fix check-clang-tools tests that fail due to Windows CRLF line endings

2022-04-24 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. In D97625#3337018 , @dmgreen wrote: > I'm not sure it will do much where it is. It won't, to quote **gitattributes**(5): > When deciding what attributes are assigned to a path, Git consults > `$GIT_DIR/info/attributes` file

[PATCH] D124128: Thread safety analysis: Store CapabilityExprs in ScopedLockableFactEntry (NFC)

2022-04-25 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added inline comments. Comment at: clang/lib/Analysis/ThreadSafety.cpp:911-919 +UnderlyingMutexes.push_back(UnderlyingCapability{M, UCK_Acquired}); } void addExclusiveUnlock(const CapabilityExpr &M) { -UnderlyingMutexes.emplace_back(M.sexpr(), UCK_Rel

[PATCH] D124563: Renormalize line endings after ac5f7be6a8688955a282becf00eebc542238a86b

2022-04-27 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. Most of the files changed here are not affected by the `.gitattributes` file, such as clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/readability-duplicate-include.h clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-

[PATCH] D124563: Renormalize line endings after ac5f7be6a8688955a282becf00eebc542238a86b

2022-04-27 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. In D124563#3478625 , @smeenai wrote: > I *think* this would mean that if you're on Windows and have `core.autocrlf` > set to `input`, when you commit changes to this files, Git will convert them > back to LF line endings. N

[PATCH] D124563: Renormalize line endings after ac5f7be6a8688955a282becf00eebc542238a86b

2022-04-27 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. In D124563#3478747 , @aaronpuchert wrote: > How about we remove `* text=auto` for now? Or would that break something? The documentation says that: > If you want to ensure that t

[PATCH] D124563: Renormalize line endings after ac5f7be6a8688955a282becf00eebc542238a86b

2022-04-27 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. In D124563#3478653 , @modimo wrote: > I think the way to go is to revert ac5f7be6a868 > then > land everything as a single stack to prevent this issue.

[PATCH] D124563: Renormalize line endings after ac5f7be6a8688955a282becf00eebc542238a86b

2022-04-27 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert updated this revision to Diff 425670. aaronpuchert edited the summary of this revision. aaronpuchert removed a subscriber: drodriguez. aaronpuchert added a comment. Herald added a subscriber: jdoerfert. Drop `* text=auto`, so that we renormalize only the files that need it. Reposito

[PATCH] D124563: Drop '* text=auto' from .gitattributes and normalize

2022-04-27 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. Would be nice if someone could accept this. I think this comes closest to the intention of D97625 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124563/new/ https://reviews.llvm.org/D

[PATCH] D124563: Drop '* text=auto' from .gitattributes and normalize

2022-04-27 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert commandeered this revision. aaronpuchert edited reviewers, added: modimo; removed: aaronpuchert. aaronpuchert added a comment. Ok, let's do it this way. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124563/new/ https://reviews.llvm.org

[PATCH] D124563: Drop '* text=auto' from .gitattributes and normalize

2022-04-27 Thread Aaron Puchert via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs Revision". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc9a16e8c3d99: Drop '* text=auto' from .gitattributes and normali

[PATCH] D124563: Drop '* text=auto' from .gitattributes and normalize

2022-04-27 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. In D124563#3478937 , @modimo wrote: > Checking locally I'm seeing LF as the line ending in crlf.cpp in the working > directory. Can you double check that everything matches up? I had this too, but checking out again seems t

[PATCH] D124563: Drop '* text=auto' from .gitattributes and normalize

2022-04-27 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. In D124563#3478968 , @modimo wrote: > I used `arc patch` and also saw the same thing. The patch does actually change the files to LF endings. So just applying the patch with non-Git tools will make LF endings, but Git will

[PATCH] D124563: Drop '* text=auto' from .gitattributes and normalize

2022-04-27 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. In D124563#3478978 , @aaronpuchert wrote: > Git will apply the LF -> CRLF transformation when it checks out itself. Git > doesn't show the file as modified because after cleaning the file (i.e. > applying CRLF -> LF) it's

[PATCH] D124563: Drop '* text=auto' from .gitattributes and normalize

2022-04-28 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. In D124563#3479373 , @nikic wrote: > This change seems to have broken switching to a branch before the commit > completely. The only way I was able to recover my git checkout is to comment > out the `* text=auto` line and t

[PATCH] D124563: Drop '* text=auto' from .gitattributes and normalize

2022-04-28 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. As a temporary workaround for switching between branches, just add a local file `clang-tools-extra/test/clang-apply-replacements/.gitattributes` with contents: Inputs/crlf/crlf.cpp binary Inputs/crlf/crlf.cpp.expected binary This will remove all kinds of filter

[PATCH] D124563: Drop '* text=auto' from .gitattributes and normalize

2022-04-28 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. In D124563#3479819 , @MForster wrote: > See D124606 . `-text` seems to be the right > compromise. To quote an earlier comment from this thread: In D124563#3478625

[PATCH] D124606: Use `-text` git attribute instead of `text eol=...'

2022-04-28 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added subscribers: smeenai, aaronpuchert. aaronpuchert added a comment. I think this change broke again what D97625 was trying to fix. These are text files and we want them to keep the specified line endings regardless of the local git configuration

[PATCH] D124606: Use `-text` git attribute instead of `text eol=...'

2022-04-28 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. Fair enough, but don't we want to enforce LF or CRLF, respectively? An editor could inadvertently change the line endings, and someone might not notice before commiting. Explicitly specifying the right line endings gives us the proper line endings even if some edit

[PATCH] D124606: Use `-text` git attribute instead of `text eol=...'

2022-04-28 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. Also I still don't understand what specifically this is fixing. What exactly was wrong about the previous configuration? The commit message talks about "various workflows" but which workflows are that? Does that mean no one can use `.gitattributes` with something e

[PATCH] D124606: Use `-text` git attribute instead of `text eol=...'

2022-04-28 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. In D124606#3480281 , @labath wrote: > In D124606#3480164 , @aaronpuchert > wrote: > >> An editor could inadvertently change the line endings, and someone might not >> notice before

[PATCH] D124131: Thread safety analysis: Store capability kind in CapabilityExpr

2022-04-29 Thread Aaron Puchert via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. aaronpuchert marked 2 inline comments as done. Closed by commit rGf8afb8fdedae: Thread safety analysis: Store capability kind in CapabilityExpr (authored by aaronpuchert). Changed prior to commit: https://reviews.llvm.org

[PATCH] D124127: Thread safety analysis: Pack CapabilityExpr using PointerIntPair (NFC)

2022-04-29 Thread Aaron Puchert via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGdd1790cd05ae: Thread safety analysis: Pack CapabilityExpr using PointerIntPair (NFC) (authored by aaronpuchert). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D124128: Thread safety analysis: Store CapabilityExprs in ScopedLockableFactEntry (NFC)

2022-04-29 Thread Aaron Puchert via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGd65c922450d1: Thread safety analysis: Store CapabilityExprs in ScopedLockableFactEntry (NFC) (authored by aaronpuchert). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D124132: Thread safety analysis: Don't pass capability kind where not needed (NFC)

2022-04-29 Thread Aaron Puchert via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG0314dbac026f: Thread safety analysis: Don't pass capability kind where not needed (NFC) (authored by aaronpuchert). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D124966: Thread safety analysis: Handle compound assignment and ->* overloads

2022-05-04 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert created this revision. aaronpuchert added reviewers: aaron.ballman, delesley. Herald added a project: All. aaronpuchert requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Like regular assignment, compound assignment operators can

[PATCH] D124966: Thread safety analysis: Handle compound assignment and ->* overloads

2022-05-04 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a subscriber: rupprecht. aaronpuchert added a comment. @rupprecht, do you still do integration at Google? This patch might be interesting to try out, since it strengthens some warnings. Comment at: clang/test/SemaCXX/warn-thread-safety-analysis.cpp:85-86 +

[PATCH] D124966: Thread safety analysis: Handle compound assignment and ->* overloads

2022-05-06 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert marked 2 inline comments as done. aaronpuchert added inline comments. Comment at: clang/lib/Analysis/ThreadSafety.cpp:1994 + case OO_Equal: + case OO_PlusEqual: + case OO_MinusEqual: aaron.ballman wrote: > If we're going to be handling

[PATCH] D124966: Thread safety analysis: Handle compound assignment and ->* overloads

2022-05-06 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert updated this revision to Diff 427617. aaronpuchert marked an inline comment as done. aaronpuchert added a comment. Also consider `++` and `--` as writing, test all operators. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124966/new/ ht

[PATCH] D124966: Thread safety analysis: Handle compound assignment and ->* overloads

2022-05-06 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert updated this revision to Diff 427623. aaronpuchert added a comment. Use 1 instead of 0 just to be sure. We don't want to trigger warnings about division by zero. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124966/new/ https://review

[PATCH] D124966: Thread safety analysis: Handle compound assignment and ->* overloads

2022-05-06 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. In D124966#3497305 , @aaron.ballman wrote: > LGTM! You should probably add a release note for this change so users know > about it. Good point, I'll add a note about compound assignment and increment/decrement. I'll leave

[PATCH] D124966: Thread safety analysis: Handle compound assignment and ->* overloads

2022-05-06 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert updated this revision to Diff 427750. aaronpuchert added a comment. Add a release note. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124966/new/ https://reviews.llvm.org/D124966 Files: clang/docs/ReleaseNotes.rst clang/lib/Analys

[PATCH] D124966: Thread safety analysis: Handle compound assignment and ->* overloads

2022-05-06 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added inline comments. Comment at: clang/docs/ReleaseNotes.rst:173 effect. - ``-Wmisexpect`` warns when the branch weights collected during profiling Or maybe I should add it here? Not sure why there is a blank line. Repository: rG LLVM Git

[PATCH] D129514: Thread safety analysis: Support builtin pointer-to-member operators

2022-07-11 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert created this revision. aaronpuchert added a reviewer: aaron.ballman. Herald added a reviewer: NoQ. Herald added a project: All. aaronpuchert requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. We consider an access to x.*pm as acce

[PATCH] D129514: Thread safety analysis: Support builtin pointer-to-member operators

2022-07-14 Thread Aaron Puchert via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGbfe63ab63e22: Thread safety analysis: Support builtin pointer-to-member operators (authored by aaronpuchert). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1

[PATCH] D129752: Thread safety analysis: Handle additional cast in scoped capability construction

2022-07-14 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert created this revision. aaronpuchert added reviewers: aaron.ballman, efriedma. Herald added a reviewer: NoQ. Herald added a project: All. aaronpuchert requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. We might have a CK_NoOp cast

[PATCH] D129752: Thread safety analysis: Handle additional cast in scoped capability construction

2022-07-14 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. @efriedma, added you because this removes one of the `IgnoreParens` calls that you added in D76943 , and that I think should be dead code. (The one after `CXXBindTemporaryExpr`.) Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D129755: Thread safety analysis: Support copy-elided production of scoped capabilities through arbitrary calls

2022-07-14 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert created this revision. aaronpuchert added reviewers: aaron.ballman, delesley. Herald added a reviewer: NoQ. Herald added a project: All. aaronpuchert requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. When support for copy elision

[PATCH] D129755: Thread safety analysis: Support copy-elided production of scoped capabilities through arbitrary calls

2022-07-14 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added inline comments. Comment at: clang/docs/ThreadSafetyAnalysis.rst:935 +// Same as constructors, but without tag types. (Requires C++17 copy elision.) +static MutexLocker Lock(Mutex *mu) ACQUIRE(mu) NO_THREAD_SAFETY_ANALYSIS { + return MutexLocker(m

[PATCH] D129755: Thread safety analysis: Support copy-elided production of scoped capabilities through arbitrary calls

2022-07-14 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added subscribers: efriedma, rsmith. aaronpuchert added a comment. Adding @rsmith because this essentially reverts rGe97654b2f28072ad9123006c05e03efd82852982 , and @efriedma because it partially reverts D76943

[PATCH] D129755: Thread safety analysis: Support copy-elided production of scoped capabilities through arbitrary calls

2022-07-14 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added inline comments. Comment at: clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h:435 /// Used to implement lazy copy and rewriting strategies. class Future : public SExpr { public: By the way, I considered using this, but it didn't seem

[PATCH] D124966: Thread safety analysis: Handle compound assignment and ->* overloads

2022-05-09 Thread Aaron Puchert via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG44ae49e1a725: Thread safety analysis: Handle compound assignment and ->* overloads (authored by aaronpuchert). Changed prior to commit: https://re

<    1   2   3   4   5   6   7   >