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

[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] 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] 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 p

[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 : DiagGroup<"thread

[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, bu

[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"))) M

[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] 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 defin

[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 https://l

[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: > I'

[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: > e

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

[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] 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 thi

[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] 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: DiagGr

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

[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: clang/lib/Analysis/ThreadSafety.c

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

aaronpuchert updated this revision to Diff 404689. aaronpuchert added a comment. Apply `clang-format`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113793/new/ https://reviews.llvm.org/D113793 Files: clang/include/clang/AST/CommentSema.h clan

[PATCH] D113795: Comment Sema: Eliminate or factor out DeclInfo inspection (NFC)

aaronpuchert updated this revision to Diff 404693. aaronpuchert added a comment. Rename `checkDecl` to `hasDeclThat`, which should hopefully address most issues. We leave `isFunctionPointerVarDecl` inlined for now, since it's probably wrong anyway. (I think we should look for fields.) Reposito

[PATCH] D71966: [Wdocumentation][RFC] Improve identifier's of \param

aaronpuchert added a comment. Herald added a project: clang-tools-extra. @Mordante, do you have plans to get back to this? Otherwise I'd like to pick it up. I'd probably concentrate on multiple parameter support for now, and delegate identifier validation, typo correction for variadic functions

[PATCH] D101202: Thread safety analysis: Fix false negative on break

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. We weren't modifying the lock set when intersecting with one coming from a break-termina

[PATCH] D101202: Thread safety analysis: Fix false negative on break

aaronpuchert added inline comments. Comment at: clang/test/SemaCXX/warn-thread-safety-analysis.cpp:341 + sls_mu.Unlock(); // \ +// expected-warning{{releasing mutex 'sls_mu' that was not held}} } This is just a side effect, not the reason I did the change.

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

aaronpuchert created this revision. aaronpuchert added reviewers: dblaikie, rsmith. aaronpuchert requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Implicit instantiations of template classes with virtual methods might cause the vtable (and al

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

aaronpuchert added a comment. In D101566#2726820 , @dblaikie wrote: > Along time ago Clang had a fairly strong aversion to implementing "off by > default" warnings ([...]) because they would tend to go unused and > unmaintained. That was a valid reason

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

aaronpuchert added a comment. In D101566#2727312 , @dblaikie wrote: > I think it'd be good to gather that data first before committing it to Clang > - that's usually what we try to do to justify the warning. Well, except that this isn't a new warning. B

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

aaronpuchert updated this revision to Diff 341845. aaronpuchert added a comment. - Fix punctuation in expected warning message. - Fix test failure: the instantiated class might not be a template, it could also be the member of another template. Repository: rG LLVM Github Monorepo CHANGES SIN

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

aaronpuchert added a comment. So I tried this in two code bases, both of which don't have `-Wweak-vtables` enabled though. In the first (~500 TUs) there were a couple of interesting finds. But they are hard to fix, even where explicit instantiations were already there: the corresponding instan

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

aaronpuchert added a comment. In D101566#2730746 , @dblaikie wrote: > Out of curiosity - have you tried it & measured any significant > improvement/value in build times/sizes/etc? No, I fear that would take too much time. (Not so much the benchmarking,

[PATCH] D100801: Thread safety analysis: Replace flags in FactEntry by SourceKind [NFC]

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 rG530e074faafe: Thread safety analysis: Replace flags in FactEntry by SourceKind (NFC) (authored by aaro

[PATCH] D101202: Thread safety analysis: Fix false negative on break

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGdaca6edb31ef: Thread safety analysis: Fix false negative on break (authored by aaronpuchert). Repository: rG LLVM Github Monorepo CHANGES SINCE L

[PATCH] D101755: Thread safety analysis: Eliminate parameter from intersectAndWarn (NFC)

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. We were modifying precisely when intersecting the lock sets of multiple predecessors wit

[PATCH] D101755: Thread safety analysis: Eliminate parameter from intersectAndWarn (NFC)

This revision was automatically updated to reflect the committed changes. Closed by commit rGd21e1b79ff7d: Thread safety analysis: Eliminate parameter from intersectAndWarn (NFC) (authored by aaronpuchert). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D102025: Thread safety analysis: Factor out function for merging locks (NFC)

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. It's going to become a bit more complicated, so let's have it separate. Repository: rG LLVM G

[PATCH] D102026: Thread safety analysis: Allow exlusive/shared joins for managed and asserted capabilities

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. Similar to how we allow managed and asserted locks to be held and not held in joining br

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

aaronpuchert added a comment. In D76038#3219500 , @zahiraam wrote: > This change is generating this crash discussed here: > https://bugs.llvm.org/show_bug.cgi?id=49834 Now migrated to https://github.com/llvm/llvm-project/issues/49178. > @aaronpuchert do

[PATCH] D116190: Comment parsing: Don't recognize commands in single-line double quotation

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

[PATCH] D116966: [analyzer] Don't specify PLUGIN_TOOL for analyzer plugins

aaronpuchert added a subscriber: aaron.ballman. aaronpuchert added a comment. This is only observable on Windows, right? Then perhaps @aaron.ballman can comment on this. The change looks legitimate to me, but I don't work on Windows. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D116190: Comment parsing: Don't recognize commands in single-line double quotation

aaronpuchert marked an inline comment as done. aaronpuchert added inline comments. Comment at: clang/lib/AST/CommentLexer.cpp:278 + +again: + size_t End = gribozavr2 wrote: > I'd suggest refactoring to a `while (true)` if you don't mind. I was imitating `lexVerb

[PATCH] D116186: Comment parsing: Simplify Lexer::skipLineStartingDecorations (NFC)

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

[PATCH] D116186: Comment parsing: Simplify Lexer::skipLineStartingDecorations (NFC)

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGbd0a970f5341: Comment parsing: Simplify Lexer::skipLineStartingDecorations (NFC) (authored by aaronpuchert). Repository: rG LLVM Github Monorepo

[PATCH] D116190: Comment parsing: Don't recognize commands in single-line double quotation

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. aaronpuchert marked 3 inline comments as done. Closed by commit rG9f0fa6544012: Comment parsing: Don't recognize commands in single-line double quotation (authored by a

[PATCH] D113837: Sema: Let InitListExpr have dependent type instead of 'void'

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

[PATCH] D116186: Comment parsing: Simplify Lexer::skipLineStartingDecorations (NFC)

aaronpuchert created this revision. aaronpuchert added a reviewer: gribozavr2. aaronpuchert requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Inspection of the first character can just be handled by the loop as well, it does exactly the same

[PATCH] D116190: Comment parsing: Don't recognize commands in single-line double quotation

aaronpuchert created this revision. aaronpuchert added a reviewer: gribozavr2. aaronpuchert requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This is consistent with the behavior of Doxygen, and allows users to write strings with C escapes or

[PATCH] D98664: Fix crash on dumping AST containing constant initializer with ParenListExpr

aaronpuchert created this revision. aaronpuchert added reviewers: aaron.ballman, riccibruno, rsmith. Herald added a subscriber: kristof.beyls. aaronpuchert requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The crash was introduced by D83183 <

[PATCH] D98664: Fix crash on dumping AST containing constant initializer with ParenListExpr

aaronpuchert added inline comments. Comment at: clang/test/AST/ast-dump-templates.cpp:71-73 +template constexpr T var(0); +// DUMP: VarDecl {{.*}} col:35 var 'const T' constexpr callinit +// DUMP-NEXT: ParenListExpr {{.*}} 'void' This is what's currently cras

[PATCH] D98665: Correct Doxygen syntax for inline code

aaronpuchert created this revision. aaronpuchert added reviewers: george.karpenkov, gribozavr2. Herald added subscribers: dexonsmith, martong. Herald added a reviewer: aaron.ballman. aaronpuchert requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-c

[PATCH] D98664: Fix crash on dumping AST containing constant initializer with ParenListExpr

aaronpuchert added a comment. In D98664#2628591 , @aaron.ballman wrote: > As far as the changes go, these seem reasonable to me, though I find it a bit > odd that these are expressions without a type whereas a `ParenExpr` has a > type of the underlying

[PATCH] D98665: Correct Doxygen syntax for inline code

aaronpuchert added a comment. In D98665#2628851 , @aaron.ballman wrote: > LGTM! I wonder if this is something our `-Wdocumentation` should warn about? Interestingly we warn about the opposite problem: /// @endcode int x; produces "warning: '@endcod

[PATCH] D98665: Correct Doxygen syntax for inline code

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG1cb15b10ea37: Correct Doxygen syntax for inline code (authored by aaronpuchert). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D98724: Fix false negative in -Wthread-safety-attributes

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. The original implementation didn't fire on non-template classes when a base class was an instanti

[PATCH] D98747: Thread safety analysis: Don't warn about managed locks on join points

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. We already did so for scoped locks acquired in the constructor, this change extends the

[PATCH] D98747: Thread safety analysis: Don't warn about managed locks on join points

aaronpuchert updated this revision to Diff 331567. aaronpuchert added a comment. Negative capabilities don't need to be considered as managed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98747/new/ https://reviews.llvm.org/D98747 Files: clang/

[PATCH] D98971: [C++20] [P1825] Fix bugs with implicit-move from variables of reference type

aaronpuchert added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:3102 +// C++20 [class.copy.elision]p3: +// ...either a non-volatile object or an rvalue reference to a non-volatile object type... +if (!(CESK & CES_AllowRValueReferenceType)) ---

[PATCH] D98971: [C++20] [P1825] Fix bugs with implicit-move from variables of reference type

aaronpuchert added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:3095 - // ...non-volatile... - if (VD->getType().isVolatileQualified()) -return false; - - // C++20 [class.copy.elision]p3: - // ...rvalue reference to a non-volatile... - if (VD->getType()->is

[PATCH] D99005: [clang] WIP: Implement P2266

aaronpuchert added a comment. For coroutines I have D68845 . The current code is wrong, and not trivial to fix. We have never properly implemented P1825 and will probably just go with P2266 uncond

[PATCH] D99005: [clang] WIP: Implement P2266

aaronpuchert added a comment. With my previous comment I meant that it's better if you leave out the `co_return` bits for now because it's wrong anyway. We can't use `PerformMoveOrCopyInitialization`. It would just generate merge conflicts. I'll probably just update my change once the committee

[PATCH] D99005: [clang] WIP: Implement P2266

aaronpuchert added a comment. In D99005#2640415 , @mizvekov wrote: > We should expect the test above to work, by binding value to the rvalue > reference in task's promise, right? Precisely, we don't want to do any initialization at all. (There will be an

[PATCH] D98971: [C++20] [P1825] Fix bugs with implicit-move from variables of reference type

aaronpuchert accepted this revision. aaronpuchert added a comment. This revision is now accepted and ready to land. I thought maybe you wanted to follow @mizvekov's proposal to simplify, but I understand you want to stick close to the standard language. So LGTM. Repository: rG LLVM Github Mo

[PATCH] D98724: Fix false negative in -Wthread-safety-attributes

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

[PATCH] D98747: Thread safety analysis: Don't warn about managed locks on join points

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

[PATCH] D98724: Fix false negative in -Wthread-safety-attributes

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa6a1c3051dbd: Fix false negative in -Wthread-safety-attributes (authored by aaronpuchert). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D98747: Thread safety analysis: Don't warn about managed locks on join points

aaronpuchert added a comment. Not replying in order, I hope that's not confusing. Let's start with the motivation. In D98747#2648381 , @delesley wrote: > Is there a compelling reason for this change, other than consistency/symmetry > concerns? In other

[PATCH] D98747: Thread safety analysis: Don't warn about managed locks on join points

aaronpuchert added a comment. In D98747#2648943 , @aaronpuchert wrote: > We could probably address all false negatives by introducing a `FactEntry` > for "managed locks that might be held", which we introduce on discrepancies > at join points. We assume

[PATCH] D99455: [AST] Store regular ValueDecl* in BindingDecl

aaronpuchert created this revision. aaronpuchert added reviewers: riccibruno, rsmith. aaronpuchert requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. We were always storing a regular ValueDecl* as decomposition declaration and haven't been usi

[PATCH] D99466: Fix PR48889: assertion failure for void() in flattened ternary expression

aaronpuchert created this revision. aaronpuchert added a reviewer: rsmith. aaronpuchert requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. We treat the expression void() as CXXScalarValueInitExpr for historic reasons probably, but void isn't a

[PATCH] D99466: Fix PR48889: assertion failure for void() in flattened ternary expression

aaronpuchert updated this revision to Diff 333719. aaronpuchert added a comment. Replace `E->getType()` argument by `T`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99466/new/ https://reviews.llvm.org/D99466 Files: clang/lib/CodeGen/CGExprScal

[PATCH] D99466: Fix PR48889: assertion failure for void() in flattened ternary expression

aaronpuchert added inline comments. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:466-469 +// [expr.type.conv]: if the type is cv void and the initializer is () or {}, +// the expression is a prvalue of type void that performs no initialization. +QualType T = E->ge

[PATCH] D99455: [AST] Store regular ValueDecl* in BindingDecl

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

[PATCH] D98747: Thread safety analysis: Don't warn about managed locks on join points

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

[PATCH] D98747: Thread safety analysis: Don't warn about managed locks on join points

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGdfec26b186d2: Thread safety analysis: Don't warn about managed locks on join points (authored by aaronpuchert). Repository: rG LLVM Github Monorep

[PATCH] D99696: [clang] NRVO: Improvements and handling of more cases.

aaronpuchert added a comment. Didn't really check for correctness yet, just a superficial review. I like the idea, splitting the functionality up definitely helps understanding this. Comment at: clang/include/clang/Sema/Sema.h:4733 + +bool isMoveEligible() const { return

[PATCH] D99696: [clang] NRVO: Improvements and handling of more cases.

aaronpuchert added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:3030-3032 + Res.S = Res.S != Sema::NRVOResult::None && CanMove + ? Sema::NRVOResult::MoveEligible + : Sema::NRVOResult::None; mizvekov wrote: > aaronpuchert wro

[PATCH] D99696: [clang] NRVO: Improvements and handling of more cases.

aaronpuchert added inline comments. Comment at: clang/lib/Sema/SemaCoroutine.cpp:1585-1586 InitializedEntity Entity = InitializedEntity::InitializeVariable(GroDecl); - ExprResult Res = S.PerformMoveOrCopyInitialization(Entity, nullptr, GroType, -

[PATCH] D99696: [clang] NRVO: Improvements and handling of more cases.

aaronpuchert added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:3144-3145 +/// \param ReturnType This is the return type of the function. +/// \returns The copy elision candidate, in case the initial return expression +/// was copy elidable, or nullptr otherwise. +con

[PATCH] D99696: [clang] NRVO: Improvements and handling of more cases.

aaronpuchert added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:3066 - if (isCopyElisionCandidate(ReturnType, VD, CESK)) -return VD; - return nullptr; +static void downgradeNRVOResult(Sema::NRVOResult &Res, bool CanMove) { + Res.S = std::min(Res.S, CanMove ?

[PATCH] D100713: [clang] NFC: refactor usage of getDecltypeForParenthesizedExpr and getValueKind

aaronpuchert added a comment. It seems that using `is*Value` and the introduction of `getDecltypeForParenthesizedExpr` could be two separate changes. Comment at: clang/lib/Sema/SemaExprCXX.cpp:5845 + if (!To->isRValue()) { +QualType T = Self.Context.getDecltypeForParenthe

[PATCH] D100733: [clang] NFC: change uses of `Expr->getValueKind` into `is?Value`

aaronpuchert added a comment. The change seems to be correct, but I'm wondering if `x.getValueKind() == VK_*Value` doesn't have one advantage over `x.is*Value()`: it's obvious that this is exclusive with the other values. Especially with `isRValue()` it might not be so obvious, because Clang do

[PATCH] D100801: Thread safety analysis: Replace flags in FactEntry by SourceKind [NFC]

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. The motivation here is to make it available in the base class whether a fact is managed

[PATCH] D111190: Comment parsing: Complete list of Doxygen commands

aaronpuchert created this revision. aaronpuchert added reviewers: gribozavr2, Mordante. Herald added a subscriber: arphaman. aaronpuchert requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. These should be all the commands from [1] except those

[PATCH] D110216: [clang] retain type sugar in auto / template argument deduction

aaronpuchert added a comment. In D110216#3041117 , @mizvekov wrote: > In D110216#3040748 , @craig.topper > wrote: > >> Looks like this fixes PR51282. > > I guess it does fix it, but the underlying implementation

[PATCH] D111190: Comment parsing: Complete list of Doxygen commands

aaronpuchert updated this revision to Diff 377492. aaronpuchert added a comment. Also support `\ifnot` but give up on treating `\if`/`\endif` as `VerbatimBlockCommand`. That's not meaningful anyway if we don't understand `\else` or `\endif`. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D111262: Comment AST: Factor out function type extraction in DeclInfo::fill (NFC)

aaronpuchert created this revision. aaronpuchert added reviewers: gribozavr2, Mordante. aaronpuchert requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D111262 Files: clang/lib

[PATCH] D111264: Comment AST: Declare function pointer variables as functions

aaronpuchert created this revision. aaronpuchert added reviewers: gribozavr2, Mordante. aaronpuchert requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. We were doing this already for type aliases, and it deduplicates the code looking through a

[PATCH] D111266: Comment AST: Add support for variable templates

aaronpuchert created this revision. aaronpuchert added reviewers: gribozavr2, Mordante. aaronpuchert requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. We treat them as variables of course, though if they have function pointer type we treat th

[PATCH] D111190: Comment parsing: Complete list of Doxygen commands

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

[PATCH] D111262: Comment AST: Factor out function type extraction in DeclInfo::fill (NFC)

aaronpuchert added a comment. Ping @gribozavr2. (Ideally also the followups.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111262/new/ https://reviews.llvm.org/D111262 ___ cfe-commits mailing list cfe-c

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

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

<    2   3   4   5   6   7