[PATCH] D113793: Comment Sema: Run checks only when appropriate (NFC)

aaronpuchert added a comment. Herald added a project: All. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113793/new/ https://reviews.llvm.org/D113793 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D125422: Comment parsing: Specify argument numbers for some block 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. The command traits have a member NumArgs for which all the parsing infr

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

aaronpuchert updated this revision to Diff 428800. aaronpuchert added a comment. Add an AST test to check that we parse the arguments correctly. (We don't use them for diagnostics currently.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125422/new

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

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's required to support `\n`, but can also used for other commands.

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

aaronpuchert added inline comments. Comment at: clang/include/clang/AST/Comment.h:303 - -Argument(SourceRange Range, StringRef Text) : Range(Range), Text(Text) { } }; Removing that allows me to build an array without initializing all members right away. A

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

aaronpuchert marked an inline comment as done. aaronpuchert added inline comments. Comment at: clang/include/clang/AST/CommentCommands.td:93 +def N : InlineCommand<"n", 0>; + gribozavr2 wrote: > Could you add a test that shows that the text after \n is not

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

aaronpuchert updated this revision to Diff 428968. aaronpuchert marked an inline comment as done. aaronpuchert added a comment. Unify `Argument` types and parsing. Add AST test. Simplify TableGen. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125429

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

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

[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&id=429203#toc Repo

[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

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 l

[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 &operator<<(const StreamingDiagnostic &DB, int I); inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB, int64_t I); inline const StreamingDiagnostic &operator<<(const StreamingDiagno

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

[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: https://stackoverflow.com/questions/11603818/why-is-there-ambiguity-between-ui

[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 https://reviews.llvm.o

[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 &operator<<(std::ostream &Str, + const clang::tidy::test::Param &Value) { + return Str << "Matched: " << std::boolalpha <

[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 &operator<<(std::ostream &Str, + const clang::tidy::test::Param &Value) { + return Str << "Matched: " << std::boolalpha <

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

[PATCH] D66919: Warn about zero-parameter K&R definitions in -Wstrict-prototypes

aaronpuchert created this revision. aaronpuchert added reviewers: arphaman, bruno, rsmith. Herald added subscribers: cfe-commits, dexonsmith. Herald added a project: clang. Zero-parameter K&R definitions specify that the function has no parameters, but they are still not prototypes, so calling the

[PATCH] D55326: [Driver] Fix incorrect GNU triplet for PowerPC on SUSE Linux

aaronpuchert updated this revision to Diff 217757. aaronpuchert added a comment. Herald added a subscriber: kbarton. Added a test case, verified that it fails before the change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55326/new/ https://revie

[PATCH] D66919: Warn about zero-parameter K&R definitions in -Wstrict-prototypes

aaronpuchert added a comment. Note that I'm just copying GCC, which seems the be the original intent behind the warning (https://bugs.llvm.org/show_bug.cgi?id=20796). So people who use both compilers will have seen that warning already. Note also that there is no warning if any declaration prov

[PATCH] D66919: Warn about zero-parameter K&R definitions in -Wstrict-prototypes

aaronpuchert added a comment. In D66919#1651108 , @dexonsmith wrote: > we just don't warn on non-prototype defining declarations, where the meaning > is unambiguous: > > void foo() {} > "Meaning" is a difficult term. What we know is that this is a K&R

[PATCH] D66919: Warn about zero-parameter K&R definitions in -Wstrict-prototypes

aaronpuchert added a comment. By the way, I'm open to adding a fix-it hint for zero-parameter K&R-style definitions, since the fix is pretty straightforward. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66919/new/ https://reviews.llvm.org/D66919

[PATCH] D66919: Warn about zero-parameter K&R definitions in -Wstrict-prototypes

aaronpuchert added a comment. We had a discussion on IRC yesterday and @aaron.ballman pointed out that the K&R syntax has been considered "obsolescent" (= deprecated) since C89, 30 years ago. He added that there are thoughts within the standards committee about removing the syntax entirely from

[PATCH] D66919: Warn about zero-parameter K&R definitions in -Wstrict-prototypes

aaronpuchert added a comment. > IIRC, when we rolled out `-Wstrict-prototypes` we explicitly excluded this > case since it hit a lot of code without finding bugs. I'm not a historian, but I believe this was suggested by @rsmith in https://bugs.llvm.org/show_bug.cgi?id=20796#c8, and I think we a

[PATCH] D66919: Warn about zero-parameter K&R definitions in -Wstrict-prototypes

aaronpuchert added a comment. In D66919#1655048 , @dexonsmith wrote: > I went back to read notes from when we deployed `-Wstrict-prototypes` (which > we have had on-by-default for our users for a couple of years, since it > catches lots of `-fblocks`-rel

[PATCH] D67112: [Sema] Add implicit cast for conversion of function references

aaronpuchert created this revision. aaronpuchert added a reviewer: rsmith. Herald added a project: clang. Herald added a subscriber: cfe-commits. aaronpuchert marked 2 inline comments as done. aaronpuchert added a comment. If anyone shares my feeling that the boolean output parameters of `Compare

[PATCH] D67112: [Sema] Add implicit cast for conversion of function references

aaronpuchert marked 2 inline comments as done. aaronpuchert added a comment. If anyone shares my feeling that the boolean output parameters of `CompareReferenceRelationship` should rather move to the return value, I would be happy to do that. Comment at: clang/test/CXX/dcl.de

[PATCH] D67113: ICK_Function_Conversion and ICK_Qualification are third kind conversions

aaronpuchert created this revision. aaronpuchert added a reviewer: rsmith. Herald added a project: clang. Herald added a subscriber: cfe-commits. Not sure if this has any effect, but it was inconsistent before. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D67113 Files: clan

[PATCH] D67113: ICK_Function_Conversion and ICK_Qualification are third kind conversions

aaronpuchert updated this revision to Diff 218550. aaronpuchert added a comment. Remove wrong changes in SemaExprCXX.cpp. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67113/new/ https://reviews.llvm.org/D67113 Files: clang/lib/Sema/SemaOverload

[PATCH] D66919: Warn about zero-parameter K&R definitions in -Wstrict-prototypes

aaronpuchert added a comment. In D66919#1658355 , @aaron.ballman wrote: > We do have numerous warnings that are default errors, you can look for > `DefaultError` in the diagnostic .td files to see the uses. Thanks, I hadn't seen that before. It seems t

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

aaronpuchert added inline comments. Comment at: clang/tools/libclang/CMakeLists.txt:115 +clang_target_link_libraries(libclang + PRIVATE + ${CLANG_LIB_DEPS} This might not be correct for static builds, I think we need `INTERFACE` here. Repository: rG LLVM Gi

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

aaronpuchert created this revision. aaronpuchert added reviewers: akyrtzi, beanz, tstellar. Herald added subscribers: cfe-commits, arphaman, dexonsmith, mgorny. Herald added a project: clang. aaronpuchert added inline comments. Comment at: clang/tools/libclang/CMakeLists.txt:115

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

aaronpuchert added a comment. I'm thinking about the following, but I'm not sure if that's the proper way to do it. --- a/clang/cmake/modules/AddClang.cmake +++ b/clang/cmake/modules/AddClang.cmake @@ -174,8 +174,11 @@ macro(add_clang_symlink name dest) endmacro() function(clang_

[PATCH] D68923: Don't warn about missing declarations for partial template specializations

aaronpuchert added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68923/new/ https://reviews.llvm.org/D68923 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.o

[PATCH] D66919: Warn about zero-parameter K&R definitions in -Wstrict-prototypes

aaronpuchert added a comment. @aaron.ballman We could make the warning even stricter, but that should be a separate discussion. Is this change Ok for now? Comment at: clang/test/Sema/warn-strict-prototypes.c:60-62 // K&R function definition with previous prototype declared is

[PATCH] D65184: [Sema] Thread Safety Analysis: Make negative capability typeless.

aaronpuchert requested changes to this revision. aaronpuchert added a comment. This revision now requires changes to proceed. > A fix would be to let all three produce exclusive negative capability, which > essentially means there is no type associated with negative capability. This > fix could

[PATCH] D87629: Thread safety analysis: Improve documentation for ASSERT_CAPABILITY

aaronpuchert added reviewers: aaron.ballman, aaronpuchert. aaronpuchert added a comment. We don't really have a good understanding of `ASSERT_CAPABILITY` ourselves. For example, there is this loophole: void h() { mu.AssertHeld(); mu.Unlock(); } One would expect to get a warning forc

[PATCH] D87629: Thread safety analysis: Improve documentation for ASSERT_CAPABILITY

aaronpuchert added a comment. You can view it this way: there is a dynamic set and a static set of capabilities. The static set is always the same at any particular point in a function, regardless of the circumstances we're called from. It's what we determine in the analysis. The dynamic set de

[PATCH] D87629: Thread safety analysis: Improve documentation for ASSERT_CAPABILITY

aaronpuchert added a comment. In D87629#2275639 , @ryanofsky wrote: > The mistakes about exceptions came from me taking "(no return)" in the > previous documentation too literally thinking it was referring to > https://en.cppreference.com/w/cpp/language/

[PATCH] D87629: Thread safety analysis: Improve documentation for ASSERT_CAPABILITY

aaronpuchert added a comment. In D87629#2278383 , @ajtowns wrote: > Not sure the `try { AssertHeld } catch (...) { a = 0; }` example reveals very > much: it seems like thread safety annotations aren't checked within a catch > block at all? > > Mutex m;

[PATCH] D87629: Thread safety analysis: Improve documentation for ASSERT_CAPABILITY

aaronpuchert added a comment. Looks pretty good, thanks! I think this clears up the misunderstandings. Since I'm not a native speaker I'd like to wait for @aaron.ballman's opinion before we merge this. Comment at: clang/docs/ThreadSafetyAnalysis.rst:128-135 The set of capabi

[PATCH] D87629: Thread safety analysis: Improve documentation for ASSERT_CAPABILITY

aaronpuchert added a comment. In D87629#2280475 , @ajtowns wrote: > Sure, it makes perfect sense that it's too hard. It's not really too hard, there is an existing parameter somewhere in the CFG generation that controls these exception handling edges, an

[PATCH] D87629: Thread safety analysis: Improve documentation for ASSERT_CAPABILITY

aaronpuchert accepted this revision. aaronpuchert added a comment. Maybe we'll have to clarify this further in the future, but for now this is an improvement. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87629/new/ https://reviews.llvm.org/D87629

[PATCH] D87629: Thread safety analysis: Improve documentation for ASSERT_CAPABILITY

This revision was automatically updated to reflect the committed changes. Closed by commit rGf702a6fa7c9e: Thread safety analysis: Improve documentation for ASSERT_CAPABILITY (authored by ryanofsky, committed by aaronpuchert). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

[PATCH] D86207: (De-)serialize BindingDecls before DecompositionDecl

aaronpuchert created this revision. aaronpuchert added a reviewer: rsmith. Herald added a project: clang. Herald added a subscriber: cfe-commits. aaronpuchert requested review of this revision. When parsing a C++17 binding declaration, we first create the BindingDecls in Sema::ActOnDecompositionDe

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

aaronpuchert added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84603/new/ https://reviews.llvm.org/D84603 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org

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

aaronpuchert added a comment. @aaron.ballman, can you have a look again? I think this change is consistent with what we're already doing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84603/new/ https://reviews.llvm.org/D84603 ___

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

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] D39937: [Sema] Improve diagnostics for const- and ref-qualified member functions

aaronpuchert added inline comments. Comment at: test/CXX/over/over.match/over.match.funcs/p4-0x.cpp:22-24 + void lvalue() &; // expected-note 2 {{'lvalue' declared here}} + void const_lvalue() const&; + void rvalue() &&; // expected-note {{'rvalue' declared here}}

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

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] D67113: ICK_Function_Conversion is a third kind conversion

aaronpuchert added a reviewer: aaron.ballman. aaronpuchert added a comment. Maybe this is to trivial for a review. The comment on `StandardConversionSequence::Third` in clang/Sema/Overload.h says > The third conversion can be a qualification conversion or a function > conversion. Repository:

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

aaronpuchert added a comment. Ping. Maybe this change is too silly, but I think it's better to be precise and not muddle qualification conversions together with casting away `noexcept`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67112/new/ http

[PATCH] D67113: ICK_Function_Conversion is a third kind conversion

aaronpuchert added a comment. Herald added a subscriber: danielkiss. In D67113#2244463 , @aaron.ballman wrote: > There may be a deeper issue here in that there are four standard conversion > sequences, not three: http://eel.is/c++draft/conv#1, but otherw

[PATCH] D67113: ICK_Function_Conversion is a third kind conversion

This revision was automatically updated to reflect the committed changes. Closed by commit rGb4a2d36c3f74: [Sema] ICK_Function_Conversion is a third kind conversion (authored by aaronpuchert). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67113/new/

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

aaronpuchert updated this revision to Diff 288820. aaronpuchert added a comment. Herald added a subscriber: danielkiss. Remove "holding" and moved to a separate message because the overlap was getting smaller. Also introduced a separate function because it's more consistent with the other funct

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

aaronpuchert marked 4 inline comments as done. aaronpuchert added inline comments. Comment at: clang/include/clang/Analysis/Analyses/ThreadSafety.h:205 + /// Warn when calling a function that the negative capability is not held. + /// \param D -- The decl for the function req

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

This revision was automatically updated to reflect the committed changes. aaronpuchert marked an inline comment as done. Closed by commit rG8ca00c5cdc0b: Thread safety analysis: More consistent warning message (authored by aaronpuchert). Changed prior to commit: https://reviews.llvm.org/D84603?

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

aaronpuchert updated this revision to Diff 289316. aaronpuchert added a comment. Rebase on top of D84603 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84604/new/ https://reviews.llvm.org/D84604 Files: clang/lib

[PATCH] D84455: [Concepts] Fix a deserialization crash.

aaronpuchert added a comment. > in some cases (e.g. implicit deduction guide templates synthesized from the > constructor), hasTypeConstraint returns true, and getTypeConstraint returns a > nullptr. If that's the case, can we reproduce this with a parsed AST? I've only seen this error on deser

[PATCH] D86207: (De-)serialize BindingDecls before DecompositionDecl

aaronpuchert added a comment. In D86207#2251802 , @riccibruno wrote: > Is my comment on the deserialization of `BindingDecl`s in > https://reviews.llvm.org/D85613?id=284364 related to this change? Not sure. The `FIXME` comment on the code is correct, and

[PATCH] D85613: [clang] Look through bindings when checking whether a default argument references a local entity.

aaronpuchert added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:91 + auto CheckAndDiagnoseLocalEntity = [&](const VarDecl *VD, unsigned DiagID, + const auto &... DiagArgs) -> bool { +if (VD->isLocalVarDecl() && !DRE->isN

[PATCH] D85613: [clang] Look through bindings when checking whether a default argument references a local entity.

aaronpuchert added a comment. Ah, I guess Ianded on an older version of this through a link and didn't notice. Nevermind. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85613/new/ https://reviews.llvm.org/D85613 ___

[PATCH] D86207: (De-)serialize BindingDecls before DecompositionDecl

aaronpuchert added a comment. I'll go with what @rsmith proposed to fix the bug. If the entire deserialization process doesn't rely on invariants, the order should indeed be irrelevant. In D86207#2252557 , @riccibruno wrote: > I agree, but I think that

[PATCH] D86207: (De-)serialize BindingDecls before DecompositionDecl

aaronpuchert added a comment. Everything compiles with `ValueDecl* Decomp` instead of a `LazyDeclPtr`, but I'll leave it until we figure out whether we might actually need it. Maybe we want to store the decomposition for a binding, just like we store the bindings for a decomposition. Although i

[PATCH] D86207: (De-)serialize BindingDecls before DecompositionDecl

aaronpuchert updated this revision to Diff 289607. aaronpuchert added a comment. Fix as suggested by @rsmith instead: set InvalidDecl directly. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86207/new/ https://reviews.llvm.org/D86207 Files: clang

[PATCH] D87064: Thread safety analysis: Test and document release_generic_capability

aaronpuchert created this revision. aaronpuchert added a reviewer: aaron.ballman. Herald added a project: clang. Herald added a subscriber: cfe-commits. aaronpuchert requested review of this revision. The old locking attributes had a generic release, but as it turns out the capability-based attrib

[PATCH] D87065: Thread safety analysis: Document how try-acquire is handled

aaronpuchert created this revision. aaronpuchert added a reviewer: aaron.ballman. Herald added a project: clang. Herald added a subscriber: cfe-commits. aaronpuchert requested review of this revision. I don't think this is obvious, since try-acquire seemingly contradicts our usual requirements of

[PATCH] D87066: Thread safety analysis: Improve documentation for scoped capabilities

aaronpuchert created this revision. aaronpuchert added a reviewer: aaron.ballman. Herald added a project: clang. Herald added a subscriber: cfe-commits. aaronpuchert requested review of this revision. They are for more powerful than the current documentation implies, this adds - adopting a lock,

[PATCH] D87066: Thread safety analysis: Improve documentation for scoped capabilities

aaronpuchert added a comment. Not sure about the added comments, I can also remove them if they're not helpful. @aaron.ballman, this addresses your earlier comment D81332#2079489 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

[PATCH] D87066: Thread safety analysis: Improve documentation for scoped capabilities

aaronpuchert added inline comments. Comment at: clang/docs/ThreadSafetyAnalysis.rst:908 +// Assumes mu is held, implicitly acquires *this and connects it to mu. +MutexLocker(t_mutex &mu, adopt_lock_t) REQUIRES(mu) : mut(mu) {} + `s/t_mutex/Mutex/g`

[PATCH] D87066: Thread safety analysis: Improve documentation for scoped capabilities

aaronpuchert updated this revision to Diff 289826. aaronpuchert marked an inline comment as done. aaronpuchert added a comment. - More detailed description how scoped capabilities work. - Make the comment wording more consistent with existing comments and the previous explanation. - Properly impl

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

aaronpuchert added inline comments. Comment at: clang/test/SemaCXX/warn-thread-safety-negative.cpp:87 +Mutex globalMutex; +void f() EXCLUSIVE_LOCKS_REQUIRED(!globalMutex); + aaron.ballman wrote: > Can you add a test that uses `!::globalMutex`? I'd like to verify

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

aaronpuchert updated this revision to Diff 289833. aaronpuchert marked 2 inline comments as done. aaronpuchert added a comment. Add tests with qualified names, let tests rely on shadowing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84604/new/ ht

[PATCH] D87064: Thread safety analysis: Test and document release_generic_capability

aaronpuchert updated this revision to Diff 290045. aaronpuchert added a comment. Add some prose, not just code. Otherwise our list of attributes would be incomplete. @aaron.ballman, I think you should have another look. Sorry for missing that in my first patch. Repository: rG LLVM Github Mo

[PATCH] D86207: Set InvalidDecl directly when deserializing a Decl

This revision was automatically updated to reflect the committed changes. aaronpuchert marked an inline comment as done. Closed by commit rG16975a638df3: Set InvalidDecl directly when deserializing a Decl (authored by aaronpuchert). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D87065: Thread safety analysis: Document how try-acquire is handled

This revision was automatically updated to reflect the committed changes. Closed by commit rG8544defdcb09: Thread safety analysis: Document how try-acquire is handled (authored by aaronpuchert). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87065/new

[PATCH] D87194: Thread safety analysis: Use access specifiers to decide about scope

aaronpuchert created this revision. aaronpuchert added a reviewer: aaron.ballman. Herald added a project: clang. Herald added a subscriber: cfe-commits. aaronpuchert requested review of this revision. Public members are always in scope, protected members in derived classes with sufficient access.

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG9dcc82f34ea9: Thread safety analysis: Consider global variables in scope (authored by aaronpuchert). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84604/new/

[PATCH] D87066: Thread safety analysis: Improve documentation for scoped capabilities

This revision was automatically updated to reflect the committed changes. Closed by commit rGbbb3baf6205c: Thread safety analysis: Improve documentation for scoped capabilities (authored by aaronpuchert). Changed prior to commit: https://reviews.llvm.org/D87066?vs=289826&id=290145#toc Reposito

[PATCH] D87064: Thread safety analysis: Test and document release_generic_capability

This revision was automatically updated to reflect the committed changes. Closed by commit rGcc6713a2c35e: Thread safety analysis: Test and document release_generic_capability (authored by aaronpuchert). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D92886: [Sema] Warn about unused functions even when they're inline

aaronpuchert created this revision. aaronpuchert added reviewers: aaron.ballman, efriedma, rsmith. aaronpuchert requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Being inline doesn't change anything about a function having internal linkage, s

[PATCH] D92886: [Sema] Warn about unused functions even when they're inline

aaronpuchert added a comment. In D92886#2441290 , @Quuxplusone wrote: > I agree with your reasoning, but in practice I still see a lot of people > using `static inline` for functions (especially function templates) in .h > files. That's also what I was

[PATCH] D92886: [Sema] Warn about unused functions even when they're inline

aaronpuchert added a comment. In D92886#2443176 , @aaronpuchert wrote: >> Non-static inline functions in C have confusing semantics. > > I'm not terribly familiar with C, but judging from C11 6.7.4 it seems that C > is a bit more permissive, but then say

[PATCH] D68845: Don't emit unwanted constructor calls in co_return statements

aaronpuchert added inline comments. Comment at: clang/lib/Sema/SemaCoroutine.cpp:896 + if (S.CanPerformCopyInitialization(Entity, &AsRvalue)) +return true; +} else if (auto *FTD = dyn_cast(D)) { aaronpuchert wrote: > Quuxplusone wrote: > > aaronp

[PATCH] D87194: Thread safety analysis: Use access specifiers to decide about scope

aaronpuchert planned changes to this revision. aaronpuchert added a comment. That's a good point, while `mu_` is public, `params` is a local variable. I need to take into account the left-hand side of a `til::Project`, which we're currently ignoring. Repository: rG LLVM Github Monorepo CHAN

[PATCH] D68845: Don't emit unwanted constructor calls in co_return statements

aaronpuchert updated this revision to Diff 305353. aaronpuchert added a comment. Herald added a subscriber: lxfind. Rebase and add a comment about the limitations of this implementation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68845/new/ http

[PATCH] D90123: [Sema] Improve notes for value category mismatch in overloading

aaronpuchert added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:4280 "candidate %sub{select_ovl_candidate_kind}0,1,2 not viable: " -"expects an l-value for " +"expects an %select{l-value|r-value}5 for " "%select{%ordinal4 argumen

[PATCH] D90123: [Sema] Improve notes for value category mismatch in overloading

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6f84779674a9: [Sema] Improve notes for value category mismatch in overloading (authored by aaronpuchert). Repository: rG LLVM Github Monorepo CHA

[PATCH] D90123: [Sema] Improve notes for value category mismatch in overloading

aaronpuchert marked an inline comment as done. aaronpuchert added a comment. Fixed the spelling issue in rGdea31f135ceae6e860e6301f9bb66d3b3adb1357 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

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

aaronpuchert added a comment. Ping. 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@lists.llvm.org https://lists.llvm.org

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

aaronpuchert added a comment. In D67112#2398577 , @rsmith wrote: > Looks fine as far as it goes, but it looks like we're also missing a cast in > function pointer initialization via function conversion: > [...] > > |-VarDecl 0x105143e8 col:8 p 'void (*

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG825f80e111f2: [Sema] Introduce function reference conversion, NFC (authored by aaronpuchert). Changed prior to commit: https://reviews.llvm.org/D67112?vs=246862&id=306929#toc Repository: rG LLVM Gith

[PATCH] D91898: [attributes] Add a facility for defining and enforcing a Trusted Computing Base.

aaronpuchert added a comment. Thread safety attributes want **callers** of a function to have the same attribute, while this change wants **callees** to have the same attribute. So the attributes propagate in different directions. By contraposition

[PATCH] D86308: [CMake][compiler-rt][libunwind] Compile assembly files as ASM not C, unify workarounds

aaronpuchert added subscribers: tstellar, aaronpuchert. aaronpuchert added a comment. @tstellar, can we have this in 11.0.1? I've already ported it back in openSUSE because we were having problems with the CMake 3.19 update. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

[PATCH] D86308: [CMake][compiler-rt][libunwind] Compile assembly files as ASM not C, unify workarounds

aaronpuchert added a comment. In D86308#2421237 , @tambre wrote: > This has already been backported to 11.0.1: > https://github.com/llvm/llvm-project/commit/03565ffd5da8370f5b89b69cd9868f32e2d75403 Thanks! I didn't see any mention of that here so I assum

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

aaronpuchert reopened this revision. aaronpuchert added a comment. This revision is now accepted and ready to land. Almost forgot about that. I think I've figured it out. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84604/new/ https://reviews.llvm

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

aaronpuchert updated this revision to Diff 300507. aaronpuchert added a comment. `LiteralPtr`s aren't always globals, local variables are also translated that way. So we ask the stored `ValueDecl` if it `isDefinedOutsideFunctionOrMethod`. That seems like the right thing. Repository: rG LLVM

[PATCH] D90123: [Sema] Improve notes for value category mismatch in overloading

aaronpuchert created this revision. aaronpuchert added reviewers: aaron.ballman, rsmith. Herald added subscribers: cfe-commits, dexonsmith. Herald added a project: clang. aaronpuchert requested review of this revision. When an overloaded member function has a ref-qualifier, like: class X { voi

<    1   2   3   4   5   6   7   >