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

courbet added a comment. I've updated https://github.com/llvm/llvm-project/pull/68572 to do as suggested. To sum up: we have a new flag `-Wthread-safety-reference-return`, which is on by default under `-Wthread-safety-reference`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

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

courbet added a comment. In D153131#4653664 , @aaronpuchert wrote: > In D153131#4653564 , @courbet wrote: > >> We have a large number of users of `-Werror -Wthread-safety-analysis` >> internally. When we make th

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

courbet added a comment. In D153131#4653456 , @aaronpuchert wrote: > In D153131#4653412 , @courbet wrote: > >> I also had some push back internally on adding this to the existing flag. >> I'm going to add `-Wthr

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

courbet added a comment. In D153131#4653362 , @aaronpuchert wrote: > In D153131#4653345 , @aeubanks > wrote: > >> This is finding lots of real issues in code, which is awesome, but could I >> request that this

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

courbet added inline comments. Comment at: clang/lib/Analysis/ThreadSafety.cpp:1604-1606 +Handler.handleFunExcludesLock(Cp.getKind(), D->getNameAsString(), + (!Cp).toString(), Loc); +return; aaronpuchert wrote:

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

courbet updated this revision to Diff 557475. courbet marked 2 inline comments as done. courbet added a comment. Address comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153132/new/ https://reviews.llvm.org/D153132 Files: clang/lib/Analysis/ThreadSafety.cpp Index: clang/lib/An

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

courbet added a comment. In D153132#4651204 , @courbet wrote: > In D153132#4647557 , @aaronpuchert > wrote: > >> In D153132#4431627 , @courbet >> wrote: >> Is this

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

courbet added a comment. In D153131#4651198 , @courbet wrote: > In D153131#4651074 , @aaronpuchert > wrote: > >> Looks still good to me. As I wrote on D153132 >> , I don't think

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

courbet added a comment. In D153132#4647557 , @aaronpuchert wrote: > In D153132#4431627 , @courbet wrote: > >>> Is this actually required for the subsequent change? I don't see the >>> connection. >> >> In the f

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

courbet added a comment. In D153131#4651074 , @aaronpuchert wrote: > 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. Sorry, I

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

courbet updated this revision to Diff 557354. courbet added a comment. rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153131/new/ https://reviews.llvm.org/D153131 Files: clang/include/clang/Analysis/Analyses/ThreadSafety.h clang/include/

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

courbet added a comment. Thanks for the review. Comment at: clang/lib/Analysis/ThreadSafety.cpp:2304-2305 + CFGBlockInfo &Initial = BlockInfo[CFGraph->getEntry().getBlockID()]; + CFGBlockInfo &Final = BlockInfo[CFGraph->getExit().getBlockID()]; + aaronpuche

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

courbet updated this revision to Diff 557015. courbet marked an inline comment as done. courbet added a comment. Rebase on NFC changes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153131/new/ https://reviews.llvm.org/D153131 Files: clang/includ

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

courbet added a comment. > I think we should use the expected exit set instead of the entry set. Indeed, I've added a few tests to check this. Let me know if you see any other tests that might be valuable. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

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

courbet updated this revision to Diff 556940. courbet marked 7 inline comments as done. courbet added a comment. - Check return values against the exit set rather than the entry set, add unit tests. - Address other cosmetic review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LA

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

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

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

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

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

courbet updated this revision to Diff 535418. courbet added a comment. Add a specific message for return-by-ref of `pt_guarded_by` variable. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153131/new/ https://reviews.llvm.org/D153131 Files: clang/

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

courbet updated this revision to Diff 534539. courbet added a comment. Put the new warnings in `-Wtread-safety-reference` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153131/new/ https://reviews.llvm.org/D153131 Files: clang/include/clang/Analy

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

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

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

courbet updated this revision to Diff 532559. courbet added a comment. format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153131/new/ https://reviews.llvm.org/D153131 Files: clang/include/clang/Analysis/Analyses/ThreadSafety.h clang/include/

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

courbet added a comment. Thanks. 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] D153131: [clang analysis][thread-safety] Handle return-by-reference...

courbet updated this revision to Diff 532558. courbet marked an inline comment as done. courbet added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153131/new/ https://reviews.llvm.org/D153131 Files: clang/incl

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

courbet updated this revision to Diff 532534. courbet added a comment. remove type alias Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153132/new/ https://reviews.llvm.org/D153132 Files: clang/lib/Analysis/ThreadSafety.cpp Index: clang/lib/Anal

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

courbet added a comment. > 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 computed. We can't analyze the returns as they are seen because , because what matters for t

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

courbet created this revision. courbet added a reviewer: delesley. Herald added a reviewer: NoQ. Herald added a project: All. courbet requested review of this revision. Herald added a project: clang. Refactoring in preparation for D153131 Repository: rG LLVM

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

courbet created this revision. courbet added a reviewer: delesley. Herald added a reviewer: NoQ. Herald added a project: All. courbet requested review of this revision. Herald added a project: clang. ...of guarded variables, when the function is not marked as requiring locks: class Return {

[PATCH] D151092: [clang-tidy]performance-no-automatic-move: fix false negative on `const T&&` ctors.

courbet added a comment. In D151092#4368151 , @aaron.ballman wrote: > The release note references documentation that doesn't exist, so Sphinx is > failing: https://lab.llvm.org/buildbot/#/builders/115/builds/46942 Sorry, fixed in rG62dc3ba8442f

[PATCH] D151092: [clang-tidy]performance-no-automatic-move: fix false negative on `const T&&` ctors.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG9182c679dde7: [clang-tidy]performance-no-automatic-move: fix false negative on `const T&&`… (authored by courbet). Repository: rG LLVM Github Mono

[PATCH] D151092: [clang-tidy]performance-no-automatic-move: fix false negative on `const T&&` ctors.

courbet updated this revision to Diff 524288. courbet marked 2 inline comments as done. courbet added a comment. address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151092/new/ https://reviews.llvm.org/D151092 Files: clang-tool

[PATCH] D151092: [clang-tidy]performance-no-automatic-move: fix false negative on `const T&&` ctors.

courbet updated this revision to Diff 524276. courbet added a comment. fix release note placemenfix release note placement Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151092/new/ https://reviews.llvm.org/D151092 Files: clang-tools-extra/clang-

[PATCH] D151092: [clang-tidy]performance-no-automatic-move: fix false negative on `const T&&` ctors.

courbet created this revision. courbet added a reviewer: gnanabit. Herald added subscribers: PiotrZSL, carlosgalvezp, xazax.hun. Herald added a reviewer: njames93. Herald added a project: All. courbet requested review of this revision. Herald added a project: clang-tools-extra. We were only handli

[PATCH] D137782: [clang-tidy]bugprone-fold-init-type

courbet marked an inline comment as done. courbet added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:294 +- Improved :doc:`bugprone-fold-init-type + ` to handle iterators that do not Eugene.Zelenko wrote: > Please keep alphabetical orde

[PATCH] D137782: [clang-tidy]bugprone-fold-init-type

This revision was automatically updated to reflect the committed changes. Closed by commit rG1012677284b8: [clang-tidy]bugprone-fold-init-type (authored by courbet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137782/new/ https://reviews.llvm.org/

[PATCH] D137782: [clang-tidy]bugprone-fold-init-type

courbet added a comment. Thanks for the review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137782/new/ https://reviews.llvm.org/D137782 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://

[PATCH] D137782: [clang-tidy]bugprone-fold-init-type

courbet updated this revision to Diff 511634. courbet added a comment. Fix release note placement and phrase it more consistenly with other notes in that section. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137782/new/ https://reviews.llvm.org/D

[PATCH] D137782: [clang-tidy]bugprone-fold-init-type

courbet updated this revision to Diff 511424. courbet marked 2 inline comments as done. courbet added a comment. - Address review comments - Add release notes. Thanks ! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137782/new/ https://reviews.llvm

[PATCH] D137782: [clang-tidy]bugprone-fold-init-type

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

[PATCH] D147419: [clang-tidy] ignore NRVO const variables in performance-no-automatic-move.

courbet accepted this revision. courbet added a comment. This revision is now accepted and ready to land. Thanks for the patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147419/new/ https://reviews.llvm.org/D147419

[PATCH] D142459: [clang] Deprecate uses of GlobalObject::getAlignment

courbet added inline comments. Comment at: clang/include/clang/AST/CharUnits.h:73 + /// fromQuantity - Construct a CharUnits quantity from an llvm::Align + /// quantity. `Maybe`* Comment at: clang/lib/CodeGen/CGObjCMac.cpp:1991 +

[PATCH] D141925: [ASTMatchers] Add `isDirectInit` matcher.

courbet updated this revision to Diff 491348. courbet added a comment. add matcher to registry and add release note Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141925/new/ https://reviews.llvm.org/D141925 Files: clang/docs/LibASTMatchersRefere

[PATCH] D141925: [ASTMatchers] Add `isDirectInit` matcher.

courbet added a comment. In D141925#4073385 , @aaron.ballman wrote: > You also need to add the new matcher to Registry.cpp so it's usable from the > dynamic matchers, and you should add a release note for the new matcher. Thanks, will do. > (We don't

[PATCH] D141925: [ASTMatchers] Add `isDirectInit` matcher.

courbet added a comment. Herald added a subscriber: ChuanqiXu. ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141925/new/ https://reviews.llvm.org/D141925 ___ cfe-commits mailing list cfe-commits@lis

[PATCH] D141925: [ASTMatchers] Add `isDirectInit` matcher.

courbet updated this revision to Diff 489796. courbet added a comment. Regenerate doc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141925/new/ https://reviews.llvm.org/D141925 Files: clang/docs/LibASTMatchersReference.html clang/include/clang

[PATCH] D141925: [ASTMatchers] Add `isDirectInit` matcher.

courbet updated this revision to Diff 489794. courbet added a comment. fix doc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141925/new/ https://reviews.llvm.org/D141925 Files: clang/include/clang/ASTMatchers/ASTMatchers.h clang/unittests/ASTM

[PATCH] D141925: [ASTMatchers] Add `isDirectInit` matcher.

courbet updated this revision to Diff 489790. courbet added a comment. fix ddoc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141925/new/ https://reviews.llvm.org/D141925 Files: clang/include/clang/ASTMatchers/ASTMatchers.h clang/unittests/AST

[PATCH] D141925: [ASTMatchers] Add `isDirectInit` matcher.

courbet created this revision. courbet added reviewers: hokein, alexfh. Herald added a project: All. courbet requested review of this revision. Herald added a project: clang. To match variables that are direct-initialized. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D141925

[PATCH] D137782: [clang-tidy]bugprone-fold-init-type

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

[PATCH] D97854: [RFC][nsan] A Floating-point numerical sanitizer.

courbet added a subscriber: titeup. courbet added a comment. In D97854#3944804 , @olologin wrote: > @courbet > Hi, sorry for pinging you. > Is this review stalled? Is there a reason for this? Judging by paper this > sanitizer looks promising. I was recen

[PATCH] D137782: [clang-tidy]bugprone-fold-init-type

courbet updated this revision to Diff 474548. courbet added a comment. remove extra training newline Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137782/new/ https://reviews.llvm.org/D137782 Files: clang-tools-extra/clang-tidy/bugprone/FoldInit

[PATCH] D137782: [clang-tidy]bugprone-fold-init-type

courbet updated this revision to Diff 474520. courbet added a comment. fix typo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137782/new/ https://reviews.llvm.org/D137782 Files: clang-tools-extra/clang-tidy/bugprone/FoldInitTypeCheck.cpp clang

[PATCH] D137782: [clang-tidy]bugprone-fold-init-type

courbet created this revision. courbet added a reviewer: aaron.ballman. Herald added subscribers: carlosgalvezp, xazax.hun. Herald added a reviewer: njames93. Herald added a project: All. courbet requested review of this revision. Herald added a project: clang-tools-extra. Handle iterators that do

[PATCH] D126903: [clang] Add support for __builtin_memset_inline

courbet added a comment. I think there's a bug in the code though: // If this is memset, we just need to see if the offset is valid in the size // of the memset.. if (MI->getIntrinsicID() == Intrinsic::memset) should really be: // If this is memset, we just need to see if the offset

[PATCH] D121365: [CFG] Fix crash on CFG building when deriving from a template.

This revision was automatically updated to reflect the committed changes. Closed by commit rG672311bd77c5: [CFG] Fix crash on CFG building when deriving from a template. (authored by courbet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121365/new/

[PATCH] D121365: [CFG] Fix crash on CFG building when deriving from a template.

courbet added a comment. Thanks ! In D121365#3720171 , @NoQ wrote: > Hi, interesting, the code looks great but at a glance I don't see why would > you even want a CFG for an uninstantiated template. Every time you want to > analyze the actual runtime b

[PATCH] D121365: [CFG] Fix crash on CFG building when deriving from a template.

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

[PATCH] D128807: [clang][transformer] Finish plumbing `Note` all the way to the output.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG156c0754bc2e: [clang][transformer] Finish plumbing `Note` all the way to the output. (authored by courbet). Repository: rG LLVM Github Monorepo C

[PATCH] D128807: [clang][transformer] Finish plumbing `Note` all the way to the output.

courbet added a comment. Thanks for the comments. Comment at: clang/include/clang/Tooling/Transformer/RewriteRule.h:250-251 +// Adds a note to the given edit or edits. If there are several edits, the note +// is added to each one of them. +// \code ymandel wr

[PATCH] D128807: [clang][transformer] Finish plumbing `Note` all the way to the output.

courbet updated this revision to Diff 451425. courbet added a comment. Replace both versions of `withNote()` with a single `ASTEdit note()` generator. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128807/new/ https://reviews.llvm.org/D128807 Files

[PATCH] D128887: [clang][transformer] Fix crash on replacement-less ASTEdit.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5331e1229aa6: [clang][transformer] Fix crash on replacement-less ASTEdit. (authored by courbet). Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D128807: [clang][transformer] Finish plumbing `Note` all the way to the output.

courbet added a comment. In D128807#3625676 , @ymandel wrote: >> For my particular case, I just need multiple notes per rule. I don't need >> them to be associated to a particular edit (and in that very particular >> case, I don't even need a source loc

[PATCH] D128807: [clang][transformer] Finish plumbing `Note` all the way to the output.

courbet added a comment. In D128807#3622779 , @ymandel wrote: > In D128807#3622727 , @li.zhe.hua > wrote: > >> - A note being a part of an edit seems weird at best. An `ASTEdit` and >> `Edit` are fragments of a

[PATCH] D128807: [clang][transformer] Finish plumbing `Note` all the way to the output.

courbet added inline comments. Comment at: clang/lib/Tooling/Transformer/RewriteRule.cpp:56 T.Range = *EditRange; -T.Replacement = std::move(*Replacement); -T.Metadata = std::move(*Metadata); +if (E.Replacement) { + auto Replacement = E.Replacement->eval(Res

[PATCH] D128887: [clang][transformer] Fix crash on replacement-less ASTEdit.

courbet created this revision. courbet added a reviewer: ymandel. Herald added a project: All. courbet requested review of this revision. Herald added projects: clang, clang-tools-extra. Given that we provide an EditGenerator edit(ASTEdit), we can't ever be sure that the user won't give us an empt

[PATCH] D128807: [clang][transformer] Finish plumbing `Note` all the way to the output.

courbet updated this revision to Diff 440948. courbet added a comment. Format patch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128807/new/ https://reviews.llvm.org/D128807 Files: clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cp

[PATCH] D128807: [clang][transformer] Finish plumbing `Note` all the way to the output.

courbet created this revision. courbet added a reviewer: ymandel. Herald added a subscriber: carlosgalvezp. Herald added a project: All. courbet requested review of this revision. Herald added projects: clang, clang-tools-extra. Right now we can only add a single warning, notes are not possible.

[PATCH] D126903: [clang] Add support for __builtin_memset_inline

courbet accepted this revision. courbet added inline comments. This revision is now accepted and ready to land. Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:7355 + isVol, true, DstPtrInfo, AAInfo); +assert(Result && "getMemset

[PATCH] D126903: [clang] Add support for __builtin_memset_inline

courbet added inline comments. Comment at: llvm/include/llvm/CodeGen/SelectionDAGTargetInfo.h:79 /// SDValue if the target declines to use custom code and a different /// lowering strategy should be used. virtual SDValue EmitTargetCodeForMemset(SelectionDAG &DAG, const S

[PATCH] D126903: [clang] Add support for __builtin_memset_inline

courbet added inline comments. Comment at: clang/test/Sema/builtins-memcpy-inline.cpp:11 +void test_memcpy_inline_invalid_arg_types() { + __builtin_memcpy_inline(1, 2, 3); // expected-error {{cannot initialize a parameter of type 'void *' with an rvalue of type 'int'}} +} -

[PATCH] D121365: [CFG] Fix crash on CFG building when deriving from a template.

courbet created this revision. courbet added reviewers: gribozavr, alexfh. Herald added a project: All. courbet requested review of this revision. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D121365 Files: clang/lib/Analysis/CFG.cpp clang/uni

[PATCH] D116535: [clang-tidy] Fix false positive in modernize-pass-by-value

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGed8ff29aa683: [clang-tidy] Fix false positive in modernize-pass-by-value (authored by courbet). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D116535: [clang-tidy] Fix false positive in modernize-pass-by-value

courbet updated this revision to Diff 397543. courbet added a comment. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116535/new/ https://reviews.llvm.org/D116535 Files: clang-tools-extra/clang-tidy/modernize/PassByValueC

[PATCH] D116535: [clang-tidy] Fix false positive in modernize-pass-by-value

courbet marked 2 inline comments as done. courbet added a comment. Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116535/new/ https://reviews.llvm.org/D116535 ___ cfe-commits mailing list cfe-comm

[PATCH] D116535: [clang-tidy] Fix false positive in modernize-pass-by-value

courbet created this revision. courbet added reviewers: alexfh, flx, aaron.ballman. Herald added subscribers: carlosgalvezp, xazax.hun. courbet requested review of this revision. Herald added a project: clang-tools-extra. The check should not trigger on lvalue/rvalue overload pairs: struct S {

[PATCH] D112453: [Sema] When dereferencing a pointer of dependent type, infer the result type.

courbet added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:13824 SourceLocation OpLoc) { - if (Op->isTypeDependent()) + if (Op->isTypeDependent() && !Op->getType()->isPointerType()) return S.Context.DependentTy;

[PATCH] D112453: [Sema] When dereferencing a pointer of dependent type, infer the result type.

courbet updated this revision to Diff 395410. courbet added a comment. Try to get rid of "dependent and not a pointer" checks checks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112453/new/ https://reviews.llvm.org/D112453 Files: clang/include

[PATCH] D112453: [Sema] When dereferencing a pointer of dependent type, infer the result type.

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

[PATCH] D114105: [clang-tidy] Ignore narrowing conversions in case of bitfields

courbet accepted this revision. courbet added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-bitfields.cpp:62 +void take(T); +void test_parameter_passing(NoBitfield x) { + take(x.id); [nit] newlin

[PATCH] D114105: [clang-tidy] Ignore narrowing conversions in case of bitfields

courbet added a comment. Yes, I think the new approach is what we want. The comments also make it much clearer. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-bitfields.cpp:1 +// RUN: %check_clang_tidy %s cppcoreguidelines-narro

[PATCH] D114539: [clang-tidy] performance-unnecessary-copy-initialization: handle pointer containers.

courbet updated this revision to Diff 389690. courbet marked an inline comment as done. courbet added a comment. address comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114539/new/ https://reviews.llvm.org/D114539 Files: clang-tools-extra/

[PATCH] D114539: [clang-tidy] performance-unnecessary-copy-initialization: handle pointer containers.

courbet updated this revision to Diff 389667. courbet added a comment. Add more comments on the approach. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114539/new/ https://reviews.llvm.org/D114539 Files: clang-tools-extra/clang-tidy/performance/

[PATCH] D114539: [clang-tidy] performance-unnecessary-copy-initialization: handle pointer containers.

courbet updated this revision to Diff 389504. courbet added a comment. Rebase on submitted unit tests so that we can see the changes better. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114539/new/ https://reviews.llvm.org/D114539 Files: clang-

[PATCH] D114539: [clang-tidy] performance-unnecessary-copy-initialization: handle pointer containers.

courbet created this revision. courbet added reviewers: flx, aaron.ballman. Herald added subscribers: carlosgalvezp, xazax.hun, mgorny. courbet requested review of this revision. Herald added a project: clang-tools-extra. This includes modifying `DeclRefExprUtils` to handle more cases. Repositor

[PATCH] D114249: [clang-tidy] performance-unnecessary-copy-initialization: Fix false negative.

This revision was automatically updated to reflect the committed changes. Closed by commit rGba4411e7c6a5: [clang-tidy] performance-unnecessary-copy-initialization: Fix false negative. (authored by courbet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D114249: [clang-tidy] performance-unnecessary-copy-initialization: Fix false negative.

courbet updated this revision to Diff 389197. courbet added a comment. add container exclusion tests for operators. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114249/new/ https://reviews.llvm.org/D114249 Files: clang-tools-extra/clang-tidy/pe

[PATCH] D114249: [clang-tidy] performance-unnecessary-copy-initialization: Fix false negative.

courbet updated this revision to Diff 389184. courbet added a comment. Canonicalize more types and add more container tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114249/new/ https://reviews.llvm.org/D114249 Files: clang-tools-extra/clan

[PATCH] D114249: [clang-tidy] performance-unnecessary-copy-initialization: Fix false negative.

courbet updated this revision to Diff 388821. courbet added a comment. rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114249/new/ https://reviews.llvm.org/D114249 Files: clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitializatio

[PATCH] D114249: [clang-tidy] performance-unnecessary-copy-initialization: Fix false negative.

courbet added inline comments. Comment at: clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp:97 + cxxOperatorCallExpr(callee(MethodDecl), hasArgument(0, ReceiverExpr), + hasArgument(0, hasType(recordType(hasDeclaration( +

[PATCH] D114249: [clang-tidy] performance-unnecessary-copy-initialization: Fix false negative.

courbet created this revision. courbet added reviewers: flx, aaron.ballman. Herald added subscribers: carlosgalvezp, xazax.hun. courbet requested review of this revision. Herald added a project: clang-tools-extra. `isConstRefReturningMethodCall` should be considering `CXXOperatorCallExpr` in addit

[PATCH] D114105: [clang-tidy] Ignore narrowing conversions in case of bitfields

courbet added a comment. Thanks for the details, this explains the motivation well. I think the key point is the combination of: > It turns out doing anything useful with a bitfield will eventually result in > an integral promotion, which mandates that the type must be 'int' if it fits > conv.p

[PATCH] D114105: [clang-tidy] Ignore narrowing conversions in case of bitfields

courbet added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:104 + castExpr(hasCastKind(CK_LValueToRValue), + has(memberExpr(hasDeclaration(fieldDecl(isBitField(, + hasParent(

[PATCH] D112453: [Sema] When dereferencing a pointer of dependent type, infer the result type.

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

[PATCH] D112453: [Sema] When dereferencing a pointer of dependent type, infer the result type.

courbet updated this revision to Diff 385768. courbet added a comment. one more spaceship operator fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112453/new/ https://reviews.llvm.org/D112453 Files: clang/include/clang/AST/Type.h clang/lib/

[PATCH] D112453: [Sema] When dereferencing a pointer of dependent type, infer the result type.

courbet added a comment. > Is this change observable in some way? With the new changes, we are now catching more typing errors before instantiation. I've added more tests to show that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112453/new/ htt

[PATCH] D112453: [Sema] When dereferencing a pointer of dependent type, infer the result type.

courbet updated this revision to Diff 384771. courbet added a comment. Implement the proposed changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112453/new/ https://reviews.llvm.org/D112453 Files: clang/include/clang/AST/Type.h clang/lib/S

[PATCH] D112453: [Sema] When dereferencing a pointer of dependent type, infer the result type.

courbet added a comment. In D112453#3103515 , @rsmith wrote: > I think this change is being made in an imperfect place, though. Instead of > putting a special case in here, how would we feel about making > `Type::isOverloadableType()` smarter, to return

[PATCH] D112453: [Sema] When dereferencing a pointer of dependent type, infer the result type.

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

[PATCH] D112722: [clang-tidy]performance-unnecessary-copy-initialization: fix false negative

This revision was automatically updated to reflect the committed changes. Closed by commit rGfc1b24d7360f: [clang-tidy]performance-unnecessary-copy-initialization: fix false negative (authored by courbet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D112722: [clang-tidy]performance-unnecessary-copy-initialization: fix false negative

courbet created this revision. courbet added reviewers: flx, ymandel, aaron.ballman. Herald added subscribers: carlosgalvezp, xazax.hun. courbet requested review of this revision. Herald added a project: clang-tools-extra. We're missing all cases where the return value is a type alias. Unfortunat

[PATCH] D112453: [Sema] When dereferencing a pointer of dependent type, infer the result type.

courbet added a comment. As per the comment in BuiltinTypes.def (see below), `Dependent` is allowed in context where the type is deducible, but is there any reason **not** to deduce the type if we can do it cheaply in some cases ? // This represents the type of an expression whose type is //

  1   2   3   >