[PATCH] D153131: [clang analysis][thread-safety] Handle return-by-reference...

aaronpuchert added a comment. In D153131#4653564 , @courbet wrote: > We have a large number of users of `-Werror -Wthread-safety-analysis` > internally. When we make the new warnings part of that flag we cannot > integrate because we're breaking all

[PATCH] D153131: [clang analysis][thread-safety] Handle return-by-reference...

aaronpuchert added a comment. In D153131#4653412 , @courbet wrote: > I also had some push back internally on adding this to the existing flag. I'm > going to add `-Wthread-safety-reference-return`, can we start by not > temporarily including it in

[PATCH] D153131: [clang analysis][thread-safety] Handle return-by-reference...

aaronpuchert added a comment. In D153131#4653345 , @aeubanks wrote: > This is finding lots of real issues in code, which is awesome, but could I > request that this be put under a separate warning flag so we can toggle off > just the new functionality

[PATCH] D153132: [clang analysis][NFCI] Preparatory work for D153131.

aaronpuchert accepted this revision. aaronpuchert added a comment. This revision is now accepted and ready to land. Got it, so it's because we want to work on a different fact set than what `BuildLockset` holds. Yeah, I think this makes sense. And after all the methods aren't too far away from

[PATCH] D153131: [clang analysis][thread-safety] Handle return-by-reference...

aaronpuchert accepted this revision. aaronpuchert added a comment. Looks still good to me. As I wrote on D153132 , I don't think we need it anymore, but if you disagree I think I can accept it as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D113838: Sema: Don't erroneously reject `void{}`

aaronpuchert added a comment. Yeah, we should get this over the line. I'm still not quite sure where to put the check. Reading @rsmith's comment again, SemaInit might perhaps be acceptable for now, except that I should add the additional tests (in case we don't have them already). I think we

[PATCH] D153132: [clang analysis][NFCI] Preparatory work for D153131.

aaronpuchert added a comment. In D153132#4431627 , @courbet wrote: >> 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

[PATCH] D153131: [clang analysis][thread-safety] Handle return-by-reference...

aaronpuchert accepted this revision. aaronpuchert added a subscriber: rupprecht. aaronpuchert added a comment. This revision is now accepted and ready to land. Looks good to me, but let's wait a few days in case @aaron.ballman has anything to add. @rupprecht, in case you're still doing

[PATCH] D153131: [clang analysis][thread-safety] Handle return-by-reference...

aaronpuchert added a comment. Sorry for letting this collect dust. I think it's a valuable addition, and looks pretty good, except that I think we should use the expected exit set instead of the entry set. These can be legitimately different for appropriately annotated functions, i.e. with

[PATCH] D152504: [clang][ThreadSafety] Analyze cleanup functions

aaronpuchert added inline comments. Comment at: clang/test/Sema/warn-thread-safety-analysis.c:26 + // Define the mutex struct. Oh, I wouldn't mind if you were to drop this spurious added newline. But you can do this when committing, no need for a new patch

[PATCH] D152504: [clang][ThreadSafety] Analyze cleanup functions

aaronpuchert accepted this revision. aaronpuchert added a comment. Thanks, looks good! I'd like to hear from @aaron.ballman if this warrants a release note, but that can be added separately. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152504/new/ https://reviews.llvm.org/D152504

[PATCH] D157385: [clang][CFG] Cleanup functions

aaronpuchert added inline comments. Comment at: clang/test/Analysis/scopes-cfg-output.cpp:1472 +// CHECK-NEXT:2: (CXXConstructExpr, [B1.3], F) +// CHECK-NEXT:3: F f __attribute__((cleanup(cleanup_F))); +// CHECK-NEXT:4: CleanupFunction (cleanup_F)

[PATCH] D152504: [clang][ThreadSafety] Analyze cleanup functions

aaronpuchert added a comment. Side note: I hope you've seen the failing test `Analysis/scopes-cfg-output.cpp`, but since that belongs to the predecessor change I assume it's an issue there. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152504/new/ https://reviews.llvm.org/D152504

[PATCH] D152504: [clang][ThreadSafety] Analyze cleanup functions

aaronpuchert accepted this revision. aaronpuchert added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Analysis/ThreadSafety.cpp:1776 /// \param D The callee declaration. /// \param Self If \p Exp = nullptr, the implicit this

[PATCH] D152504: [clang][ThreadSafety] Analyze cleanup functions

aaronpuchert added inline comments. Comment at: clang/lib/Analysis/ThreadSafetyCommon.cpp:320 // Substitute call arguments for references to function parameters -assert(I < Ctx->NumArgs); -return translate(Ctx->FunArgs[I], Ctx->Prev); +if (const

[PATCH] D152246: [clang][ThreadSafety] Analyze known function pointer values

aaronpuchert added a comment. In D152246#4639495 , @tbaeder wrote: > Do you think warning on assignment of function pointers with mismatched > attributes would be a viable way forward? Yes, that sounds like the right approach. (Slightly relaxed

[PATCH] D152504: [clang][ThreadSafety] Analyze cleanup functions

aaronpuchert added a comment. Sorry that it took so long, I had to debug it and think a bit. This should do it: diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h index 9d28325c1ea6..13e37ac2b56b 100644 ---

[PATCH] D158775: [NFC] Initialize member pointer and avoid potential null dereference

aaronpuchert added inline comments. Comment at: clang/lib/Analysis/ThreadSafety.cpp:2243 const auto *CurrentFunction = dyn_cast(D); CurrentMethod = dyn_cast(D); It seems to me that `CurrentMethod` is unconditionally assigned a value here, before it is

[PATCH] D157385: [clang][CFG] Cleanup functions

aaronpuchert added a comment. Looks good to me, but let's wait for the CFG maintainers to approve it. Comment at: clang/lib/Analysis/ThreadSafety.cpp:2429 } + case CFGElement::TemporaryDtor: { Could you remove the added line? CHANGES SINCE

[PATCH] D157385: [clang][CFG] Cleanup functions

aaronpuchert added inline comments. Comment at: clang/test/Analysis/scopes-cfg-output.cpp:1480 +public: + ~F() {} +}; tbaeder wrote: > aaronpuchert wrote: > > As with the cleanup function, a definition shouldn't be necessary. > Is there a way to test whether

[PATCH] D157385: [clang][CFG] Cleanup functions

aaronpuchert added inline comments. Comment at: clang/test/Analysis/scopes-cfg-output.cpp:1473-1474 +// CHECK-NEXT:3: F f __attribute__((cleanup(cleanup_F))); +// CHECK-NEXT:4: [B1.3].~F() (Implicit destructor) +// CHECK-NEXT:5: CleanupFunction (cleanup_F) +//

[PATCH] D152246: [clang][ThreadSafety] Analyze known function pointer values

aaronpuchert added a comment. In D152246#4484366 , @tbaeder wrote: > So, the problem with this (type of) analysis is that we don't have a perfect > view of the (global) program state, right? The CFG is per-function, and any > other function (etc.)

[PATCH] D157385: [clang][CFG] Cleanup functions

aaronpuchert added a comment. For me this looks good, but I'd like @NoQ to sign off on it. Comment at: clang/lib/Analysis/CFG.cpp:1874 +if (needsAutomaticDestruction(D)) DeclsNonTrivial.push_back(D); I'm wondering if you should rename this

[PATCH] D153001: [clang][ThreadSafety] Add __builtin_instance_member (WIP)

aaronpuchert added a comment. We probably parse the attributes delayed in C++ but not in C. This probably also means you can't refer to later members in the attribute, so the mutex always needs to come first, right? Not sure if such a limitation is expected for C developers. Repository: rG

[PATCH] D153131: [clang analysis][thread-safety] Handle return-by-reference...

aaronpuchert added a comment. Tried this on our code base, and the number of new warnings seems acceptable. I'll still need to look through them in more detail, but there is one suspicious warning that boils down to this: > cat reference-bug.cpp struct __attribute__((capability("mutex")))

[PATCH] D152504: [clang][ThreadSafety] Analyze cleanup functions

aaronpuchert added inline comments. Comment at: clang/lib/Analysis/ThreadSafety.cpp:2436 +CF.getVarDecl()->getLocation()); + break; +} aaronpuchert wrote: > tbaeder wrote: > > This handles the function call,

[PATCH] D153131: [clang analysis][thread-safety] Handle return-by-reference...

aaronpuchert added inline comments. Comment at: clang/include/clang/Basic/DiagnosticGroups.td:1046 def ThreadSafetyPrecise: DiagGroup<"thread-safety-precise">; def ThreadSafetyReference : DiagGroup<"thread-safety-reference">; +def ThreadSafetyReturn :

[PATCH] D152504: [clang][ThreadSafety] Analyze cleanup functions

aaronpuchert added inline comments. Comment at: clang/lib/Analysis/ThreadSafety.cpp:2436 +CF.getVarDecl()->getLocation()); + break; +} tbaeder wrote: > This handles the function call, but without the instance

[PATCH] D153001: [clang][ThreadSafety] Add __builtin_instance_member (WIP)

aaronpuchert added a comment. I'm wondering why you chose this over a direct equivalent to `this`, say `__builtin_instance()`, such that instead of `__builtin_instance_member(M)` one would write `__builtin_instance().M` or `__builtin_instance()->M`? While allowing to reference the same fields,

[PATCH] D153132: [clang analysis][NFCI] Preparatory work for D153131.

aaronpuchert added a comment. Is this actually required for the subsequent change? I don't see the connection. That being said, I haven't spent a lot of thought on what belongs in `ThreadSafetyAnalyzer` and what in `BuildLockset`. Comment at:

[PATCH] D153131: [clang analysis][thread-safety] Handle return-by-reference...

aaronpuchert added inline comments. Comment at: clang/lib/Analysis/ThreadSafety.cpp:2165 + + VisitorBase::VisitReturnStmt(S); +} Also wondering why we're doing this—no other visitor function seems to bother the `VisitorBase = ConstStmtVisitor`. Are these not

[PATCH] D153131: [clang analysis][thread-safety] Handle return-by-reference...

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:

[PATCH] D153033: [NFC][CLANG] Fix potential null pointer dereference bugs

aaronpuchert added inline comments. Comment at: clang/lib/Analysis/ThreadSafety.cpp:505-511 const unsigned *i = C.lookup(D); llvm::errs() << " -> "; + if (!i) { +llvm::errs() << "<>"; +return; + } dumpVarDefinitionName(*i);

[PATCH] D152246: [clang][ThreadSafety] Analyze known function pointer values

aaronpuchert added a comment. That's a tough one. The change seems to be correct, and makes more patterns visible. Of course I wonder how it comes that we see calls of a function pointer with a uniquely determined value, as that would seem like obfuscation. Maybe you can show an example how

[PATCH] D150931: [NFC][Clang][Coverity] Fix Static Code Analysis Concerns with copy without assign

aaronpuchert added a comment. Changes to `ThreadSafetyTIL.h` look good to me, thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150931/new/ https://reviews.llvm.org/D150931 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D150931: [NFC][Clang][Coverity] Fix Static Code Analysis Concerns with copy without assign

aaronpuchert added a comment. Can only comment on `ThreadSafetyTIL.h`. Thanks for addressing this, I agree that these expressions should not be assignable. Comment at: clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h:323-324 + // The copy assignment operator is

[PATCH] D150411: [NFC][Clang][Coverity] Fix Static Code Analysis Concerns with copy without assign

aaronpuchert added a comment. I'd probably abstain from explicitly deleting what is already implicitly deleted, but otherwise this looks good to me. Thanks! Comment at: clang/include/clang/Sema/Sema.h:1790 SemaDiagnosticBuilder(const SemaDiagnosticBuilder &) = default; +

[PATCH] D150411: [NFC][Clang][Coverity] Fix Static Code Analysis Concerns with copy without assign

aaronpuchert added a comment. In several cases it's not completely obvious to me whether a copy assignment operator should or can be defined. But perhaps this doesn't need to be addressed right now: we seem to compile with `-Wextra`, which contains `-Wdeprecated-copy`. That should warn if a

[PATCH] D147875: [clang][Diagnostics] Show line numbers when printing code snippets

aaronpuchert added inline comments. Comment at: clang/lib/Frontend/TextDiagnostic.cpp:1121-1138 +static unsigned getNumDisplayWidth(unsigned N) { + if (N < 10) +return 1; + if (N < 100) +return 2; + if (N < 1'000) +return 3; aaronpuchert wrote: >

[PATCH] D147875: [clang][Diagnostics] Show line numbers when printing code snippets

aaronpuchert added inline comments. Comment at: clang/include/clang/Driver/Options.td:2544 +defm diagnostics_show_line_numbers : BoolFOption<"diagnostics-show-line-numbers", + DiagnosticOpts<"ShowLineNumbers">, DefaultTrue, + NegFlag, aaron.ballman wrote: >

[PATCH] D103929: [clang] P2085R0: Consistent defaulted comparisons

aaronpuchert added a comment. Herald added a project: All. In the meantime, this has apparently been addressed via D104478 . CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103929/new/ https://reviews.llvm.org/D103929

[PATCH] D109727: [Driver] Remove unneeded *-suse-* triples

aaronpuchert added a comment. Herald added a subscriber: pcwang-thead. Herald added a project: All. Basically this should be Ok. We set the `LLVM_HOST_TRIPLE` to match the GCC triple on almost all platforms now. But we'll need to patch `isGNUEnvironment` like D110900

[PATCH] D110899: [Driver][XRay][test] XFAIL on linux with no environment specified

aaronpuchert added a comment. Herald added a project: All. Should be obsolete after 016785d9316d8c5abc5fdf3cdb86479095bbb677 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D141968: [NFC] Consolidate llvm::CodeGenOpt::Level handling

aaronpuchert added inline comments. Comment at: flang/lib/Frontend/FrontendActions.cpp:590 + std::optional OptLevelOrNone = + CodeGenOpt::getLevel(CGOpts.OptimizationLevel); + assert(OptLevelOrNone && "Invalid optimization level!"); Does this need

[PATCH] D137043: [clang] add implicit include for Linux/gnu compatibility

aaronpuchert added a comment. In D137043#3935526 , @Origami404 wrote: > In D137043#3935129 , @aaronpuchert > wrote: > >> This include-if-exists mechanism seems brittle to me. > > Do you mean the way that we

[PATCH] D137043: [clang] add implicit include for Linux/gnu compatibility

aaronpuchert added a comment. This include-if-exists mechanism seems brittle to me. Can we not make it dependent on the triple, i.e. include the file if we're using a libc implementation that's known to provide (and require) this file? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D136282: [clang] [CMake] Link libclangBasic against libatomic when necessary.

aaronpuchert added inline comments. Comment at: clang/lib/Basic/CMakeLists.txt:114-117 +target_link_libraries(clangBasic + PRIVATE + ${LLVM_ATOMIC_LIB} +) Arfrever wrote: > mgorny wrote: > > Is this the right place? Grepping for `std::atomic`, I see

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

aaronpuchert added a comment. Thanks for the detailed write-up, very much appreciated. In D129755#3866887 , @rupprecht wrote: > 1. (2x) Returning a MutexLock-like structure, e.g. > > MutexLock Lock() { > return MutexLock(); > } > > this is

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

aaronpuchert added a comment. In D129755#3865342 , @rupprecht wrote: > I'm seeing a fair number of breakages from this patch (not really sure how > many we truly have, I've hit ~5-10 so far in widely used libraries, but I > suspect we have far more in

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG54bfd0484615: Thread safety analysis: Support copy-elided production of scoped capabilities… (authored by aaronpuchert). Repository: rG LLVM

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

aaronpuchert added a comment. @aaron.ballman, would like some feedback on the release notes. Should I additionally write something under "Potentially Breaking Changes", or is it enough to mention this under "Improvements to Clang's diagnostics"? Though I guess we could also add this later on

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

aaronpuchert marked 5 inline comments as done. aaronpuchert added a comment. @hans, where you able to fix or work around the warnings? I'd like to land this again, but if you need more time it can also wait a bit. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

aaronpuchert updated this revision to Diff 466923. aaronpuchert added a comment. Add sentence to release notes that we now look into more constructor calls and that this could lead to additional warnings being emitted. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

aaronpuchert added a comment. In D129755#3843144 , @gulfem wrote: > We also started seeing `-Wthread-safety-precise` error in our Fuchsia code. >

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

aaronpuchert updated this revision to Diff 466172. aaronpuchert added a comment. This revision is now accepted and ready to land. - Pass `til::LiteralPtr *Self` for automatic object destructors into handling of `require_capability` and `locks_excluded` attributes. - Add `[[maybe_unused]]`

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

aaronpuchert planned changes to this revision. aaronpuchert added a comment. In D129755#3843206 , @aaronpuchert wrote: > Presumably `Cursor` is some kind of alias to `VmoCursor`, as we don't look at > base destructors yet. Since the code is not easily

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

aaronpuchert added a comment. In D129755#3843144 , @gulfem wrote: > We also started seeing `-Wthread-safety-precise` error in our Fuchsia code. >

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

aaronpuchert added a comment. In D129755#3842729 , @hans wrote: > We're hitting a false positive in grpc after this: > > > ../../third_party/grpc/src/src/core/lib/gprpp/ref_counted_ptr.h:335:31: > error: calling function 'TlsSessionKeyLoggerCache'

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

aaronpuchert added inline comments. Comment at: clang/lib/Analysis/ThreadSafety.cpp:1789 + auto inserted = ConstructedObjects.insert({Exp, Placeholder.first}); + assert(inserted.second && "Are we visiting the same expression again?"); + if (isa(Exp))

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0041a69495f8: Thread safety analysis: Support copy-elided production of scoped capabilities… (authored by aaronpuchert). Repository: rG LLVM

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. aaronpuchert marked an inline comment as done. Closed by commit rGd8fa40dfa7ad: Thread safety analysis: Handle additional cast in scoped capability construction

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

aaronpuchert updated this revision to Diff 465782. aaronpuchert added a comment. Add trimmed-down documentation back in and a release note. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129755/new/ https://reviews.llvm.org/D129755 Files:

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

aaronpuchert added a comment. In D129755#3779997 , @aaron.ballman wrote: > Please be sure to add a release note for the changes! Any opinion as to what the release note might say? I'm asking since we dropped the documentation changes. Perhaps I

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

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] D111283: [clang] template / auto deduction deduces common sugar

aaronpuchert added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:12367 + case Type::Class: \ +llvm_unreachable("Unexpected " Kind ": " #Class); + mizvekov wrote: > davrec wrote: > >

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

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] D129755: Thread safety analysis: Support copy-elided production of scoped capabilities through arbitrary calls

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() { +

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

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/

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

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)

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

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] D133009: [libclang] Fix conversion from `StringRef` to `CXString`

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 > > >

[PATCH] D132791: Fix formatting in release notes

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/

[PATCH] D132791: Fix formatting in release notes

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

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/

[PATCH] D132791: Fix formatting in release notes

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

[PATCH] D132791: Fix formatting in release notes

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:

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

aaronpuchert added a comment. Ping. 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

[PATCH] D129160: libclang.so: Make SONAME the same as LLVM version

aaronpuchert added a comment. In D129160#3721943 , @isuruf wrote: > Sure. If an application links to `libclang.so` when the application is being > built, the application will hardcode `libclang.so.13` in it and will look for > it. > When the SONAME

[PATCH] D129160: libclang.so: Make SONAME the same as LLVM version

aaronpuchert added a comment. > Also, this change did not really acheive it's purpose of allowing apps to use > newer versions of libclang.so without rebuilding, because a new version of > libclang.so requires a new version of libLLVM.so, which does not have a > stable ABI. Could you

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

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

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

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] D129755: Thread safety analysis: Support copy-elided production of scoped capabilities through arbitrary calls

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

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

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

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

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

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

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

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] D129514: Thread safety analysis: Support builtin pointer-to-member operators

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

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

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

[PATCH] D125429: Comment parsing: Allow inline commands to have 0 or more than 1 argument

aaronpuchert added a comment. In D125429#3512642 , @aaronpuchert wrote: > Proposed this in D125580 . Landed this two days ago and no one complained about it so far, so let's hope that solved it. Repository: rG

[PATCH] D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum

aaronpuchert added inline comments. Comment at: clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp:210-214 +std::ostream <<(std::ostream , + const clang::tidy::test::Param ) { + return Str << "Matched: " << std::boolalpha << Value.Matched <<

[PATCH] D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum

aaronpuchert added inline comments. Comment at: clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp:210-214 +std::ostream <<(std::ostream , + const clang::tidy::test::Param ) { + return Str << "Matched: " << std::boolalpha << Value.Matched <<

[PATCH] D125580: Resolve overload ambiguity on Mac OS when printing size_t in diagnostics

This revision was automatically updated to reflect the committed changes. Closed by commit rGac7a9ef0ae3a: Resolve overload ambiguity on Mac OS when printing size_t in diagnostics (authored by aaronpuchert). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D125429: Comment parsing: Allow inline commands to have 0 or more than 1 argument

aaronpuchert added a comment. In D125429#3511648 , @aaronpuchert wrote: > And it appears that `unsigned long` does not have a clear favorite here. Found it on StackOverflow:

[PATCH] D125580: Resolve overload ambiguity on Mac OS when printing size_t in diagnostics

aaronpuchert created this revision. aaronpuchert added reviewers: aeubanks, rsmith. Herald added a project: All. aaronpuchert requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Precommit builds cover Linux and Windows, but this ambiguity

[PATCH] D125429: Comment parsing: Allow inline commands to have 0 or more than 1 argument

aaronpuchert added a comment. So we have overloads inline const StreamingDiagnostic <<(const StreamingDiagnostic , int I); inline const StreamingDiagnostic <<(const StreamingDiagnostic , int64_t I); inline const StreamingDiagnostic <<(const StreamingDiagnostic , unsigned I); inline

[PATCH] D125429: Comment parsing: Allow inline commands to have 0 or more than 1 argument

aaronpuchert added a comment. Hmm, didn't see this on Linux. I'll try to disambiguate. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125429/new/ https://reviews.llvm.org/D125429 ___ cfe-commits mailing

[PATCH] D125473: Comment parsing: Treat properties as zero-argument inline commands

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd2396d896ee1: Comment parsing: Treat properties as zero-argument inline commands (authored by aaronpuchert). Repository: rG LLVM Github Monorepo

[PATCH] D125429: Comment parsing: Allow inline commands to have 0 or more than 1 argument

This revision was automatically updated to reflect the committed changes. Closed by commit rGd3a4033d6ee1: Comment parsing: Allow inline commands to have 0 or more than 1 argument (authored by aaronpuchert). Changed prior to commit: https://reviews.llvm.org/D125429?vs=428968=429203#toc

[PATCH] D125422: Comment parsing: Specify argument numbers for some block commands

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG99d35826a043: Comment parsing: Specify argument numbers for some block commands (authored by aaronpuchert). Repository: rG LLVM Github Monorepo

[PATCH] D125473: Comment parsing: Treat properties as zero-argument inline commands

aaronpuchert created this revision. aaronpuchert added a reviewer: gribozavr2. Herald added a project: All. aaronpuchert requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. That is more accurate, and using a separate class in TableGen seems

  1   2   3   4   5   6   7   >