[PATCH] D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1

lebedev.ri added a comment. In D80344#2255090 , @asmith wrote: > Yes there was an RFC and discussion and several requests for comments. > http://lists.llvm.org/pipermail/llvm-dev/2020-March/140541.html It is probably good to state so in the patch's descri

[PATCH] D87243: [cmake] Centralize LLVM_ENABLE_WARNINGS option

lebedev.ri added a comment. This is not nessesairy correct, at least the clang can be built in standalone mode, not as part of monorepo checkout (i.e. it's `clang/cmakelists.txt` is the root cmakelists, not `llvm/cmakelists.txt`'s) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

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

lebedev.ri added a comment. I'm not sure this is the problematic patch, but i'm now seeing some weird warnings: :304:15: warning: acquiring mutex 'guard' requires negative capability '!guard' [-Wthread-safety-negative] MutexLocker guard(&mutex); ^ :309:15: warning: acqui

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

lebedev.ri added a comment. In D84604#2262745 , @lebedev.ri wrote: > I'm not sure this is the problematic patch, but i'm now seeing some weird > warnings: > > :304:15: warning: acquiring mutex 'guard' requires negative > capability '!guard' [-Wthread-s

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

lebedev.ri reopened this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. So i've just reverted this. @aaron.ballman @baloghadamsoftware how's QC going on nowadays here? Was this evaluated on anything other than it's tests? It appears to be either unacceptab

[PATCH] D87451: add new clang option -fnovisibility.

lebedev.ri added a comment. Bad commit message - links will go dead eventually, all the relevant stuff needs to be specified in the commit message itself. Comment at: clang/docs/ClangCommandLineReference.rst:2310-2320 .. option:: -fvisibility-ms-compat +Ignore all the visib

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

lebedev.ri added a comment. In D71199#2265692 , @baloghadamsoftware wrote: > In D71199#2265594 , @lebedev.ri > wrote: > >> So i've just reverted this in rGebf496d805521b53022a351f35854de977fee844 >>

[PATCH] D91055: [clang-tidy] Introduce misc No Integer To Pointer Cast check

lebedev.ri added a comment. In D91055#2440344 , @aaron.ballman wrote: > LGTM! Thank you for the review! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91055/new/ https://reviews.llvm.org/D91055 __

[PATCH] D91055: [clang-tidy] Introduce misc No Integer To Pointer Cast check

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG39431e479ffd: [clang-tidy] Introduce misc No Integer To Pointer Cast check (authored by lebedev.ri). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D87188: [InstCombine] Canonicalize SPF to abs intrinc

lebedev.ri commandeered this revision. lebedev.ri edited reviewers, added: nikic; removed: lebedev.ri. lebedev.ri added a comment. @mstorsjo please can you be more specific what kind of tests are starting to fail? Do those tests just check that the code compiled into an identical assembly? I red

[PATCH] D87188: [InstCombine] Canonicalize SPF to abs intrinc

lebedev.ri updated this revision to Diff 310855. lebedev.ri added a comment. This revision is now accepted and ready to land. Partial rebase (without updating test coverage) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87188/new/ https://reviews.l

[PATCH] D87188: [InstCombine] Canonicalize SPF to abs intrinc

lebedev.ri added a subscriber: t.p.northover. lebedev.ri added a comment. In D87188#2447025 , @craig.topper wrote: > In D87188#2446096 , @spatel wrote: > >> In D87188#2445506

[PATCH] D93182: [clang-tidy] Add linux kernel log functions checker

lebedev.ri added a comment. Out of curiocity, are all checkpatch diagnostics (and something else)? are planned to be ported into being clang-tidy checks? How many will checks will that be? I'll be slightly worried if `linuxkernel` module ends up having more checks than all other modules combined..

[PATCH] D91444: [InstCombine] Preserve !annotation metadata for memory combines.

lebedev.ri accepted this revision. lebedev.ri added a comment. In D91444#2458024 , @fhahn wrote: > In D91444#2394512 , @lebedev.ri > wrote: > >> What about teaching IRBuilder to deal with it like it deals with debu

[PATCH] D68410: [AttrDocs] document always_inline

lebedev.ri resigned from this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. Seems fine to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68410/new/ https://reviews.llvm.org/D68410 ___

[PATCH] D87188: [InstCombine] Canonicalize SPF to abs intrinc

lebedev.ri added a subscriber: dmajor. lebedev.ri added a comment. @mstorsjo i've bothered @dmajor to give this a try on some of mozilla's CI, and unfortunately it came back green.. I don't recall if i can reproduce it locally (on vanilla test suite, i'll recheck), but right now it would seem th

[PATCH] D87188: [InstCombine] Canonicalize SPF to abs intrinc

lebedev.ri added a comment. In D87188#2463285 , @dmgreen wrote: > Thanks for the reproducer. I verified that it does indeed fail with this > patch. > > It seems to be doing this as a knock-on effect: https://godbolt.org/z/Y4z3je, > which does not verify:

[PATCH] D87188: [InstCombine] Canonicalize SPF to abs intrinc

lebedev.ri added a comment. In D87188#2463447 , @dmgreen wrote: > Yeah. The reproducer seems to work OK with a patch something like this: > > diff --git a/llvm/lib/Analysis/InstructionSimplify.cpp > b/llvm/lib/Analysis/InstructionSimplify.cpp > index

[PATCH] D87188: [InstCombine] Canonicalize SPF to abs intrinc

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG897c985e1e21: [InstCombine] Canonicalize SPF to abs intrinsic (authored by lebedev.ri). Changed prior to commit: https://reviews.llvm.org/D87188?v

[PATCH] D87188: [InstCombine] Canonicalize SPF to abs intrinc

lebedev.ri added a comment. Once again, thank you @mstorsjo and @dmgreen! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87188/new/ https://reviews.llvm.org/D87188 ___ cfe-commits mailing list cfe-commits

[PATCH] D51650: Implement target_clones multiversioning

lebedev.ri added a comment. In D51650#2454830 , @erichkeane wrote: > Since there seems to be at least a little renewed interest in this from a > handful of people lately, I spend the time to rebase this and do some minor > cleanup tasks. FWIW yes, i'm b

[PATCH] D91000: [clang-tidy] CERT MSC24-C Obsolescent Functions check

lebedev.ri added a comment. In D91000#2469880 , @steakhal wrote: > In D91000#2465514 , @ktomi996 wrote: > >> In D91000#2382562 , @steakhal wrote: >> >>> Quoting the revision

[PATCH] D91000: [clang-tidy] CERT MSC24-C Obsolescent Functions check

lebedev.ri added a comment. In D91000#2469894 , @steakhal wrote: > In D91000#2469884 , @lebedev.ri > wrote: > >> In D91000#2469880 , @steakhal wrote: >> >>> In D91000#246551

[PATCH] D87188: [InstCombine] Canonicalize SPF to abs intrinc

lebedev.ri added a comment. In D87188#2470392 , @thakis wrote: > Heads up: Breaks a test for us: > https://bugs.chromium.org/p/chromium/issues/detail?id=1161542 > > (No reduced repro yet, might be UB, just fyi at this point.) Thanks for headsup. For now

[PATCH] D87188: [InstCombine] Canonicalize SPF to abs intrinc

lebedev.ri added a comment. In D87188#2467915 , @nlopes wrote: > This patch regressed the following test of Transforms/InstCombine/abs-1.ll: > (need to drop NSW in this case). > > define i8 @nabs_canonical_3(i8 %x) { > %0: > %cmp = icmp slt i8 %x,

[PATCH] D93822: [clang][Sema] Add diagnostics for implicit widening of multiplication result

lebedev.ri created this revision. lebedev.ri added reviewers: rsmith, aaron.ballman, rjmccall, erichkeane, dblaikie. lebedev.ri added a project: LLVM. lebedev.ri requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Overflows are never fun. In m

[PATCH] D90392: [clang-tidy] Omit std::make_unique/make_shared for default initialization.

lebedev.ri added a comment. I think for starters this patch needs a better description, explaining not *what* it does, but *why* it does what it does. Also, it is probably not good to change (break) existing test coverage. Instead, it is best to add new tests, and adjust existing check lines. R

[PATCH] D90944: [clang-tidy] implement misc-mt-unsafe

lebedev.ri added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/misc-mt-unsafe.cpp:3-4 + +#include +#include + Tests should be hermetic, they can not use headers from system Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D90944: [clang-tidy] implement misc-mt-unsafe

lebedev.ri added a comment. While i share concern about false-positives, it is literally impossible to avoid them here, and this should be viewed more as an enforcement tool (don't use thread-unsafe fns), not bug-detection check. What i would however like to be improved, is better docs. I'm gue

[PATCH] D91055: [clang-tidy] Introduce misc No Integer To Pointer Cast check

lebedev.ri created this revision. lebedev.ri added reviewers: aaron.ballman, njames93, JonasToth. lebedev.ri added a project: LLVM. Herald added subscribers: cfe-commits, xazax.hun, mgorny. Herald added a project: clang. lebedev.ri requested review of this revision. While casting an (integral) poi

[PATCH] D91055: [clang-tidy] Introduce misc No Integer To Pointer Cast check

lebedev.ri added a comment. In D91055#2382112 , @nlopes wrote: > Nice! Why thank you. > BTW, another popular idiom is to store data in the last few bits of the > pointer (e.g., LLVM's own PointerIntPair). I guess that one can also be > implement by cas

[PATCH] D91055: [clang-tidy] Introduce misc No Integer To Pointer Cast check

lebedev.ri added a subscriber: rsmith. lebedev.ri added a comment. CC'ing @rsmith regarding the suggestion/question of also having a clang diagnostic for this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91055/new/ https://reviews.llvm.org/D9105

[PATCH] D90392: [clang-tidy] Omit std::make_unique/make_shared for default initialization.

lebedev.ri added a comment. In D90392#2362118 , @njames93 wrote: > IIUC, this is handling the case where `Ptr.reset(new int)` which is different > to `Ptr.reset(new int())` because the former doesn't initialise the int while > the latter default(zero) in

[PATCH] D87188: [InstCombine] Canonicalize SPF to abs intrinc

lebedev.ri added inline comments. Comment at: llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp:1070 InstCombinerImpl &IC) { if (!Cmp.hasOneUse() || !isa(Cmp.getOperand(1))) return nullptr; nikic wrote: > nikic

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

lebedev.ri added a comment. In D66324#2336555 , @lebedev.ri wrote: > In D66324#2336186 , @phosek wrote: > >> I apologize for the late response, I somehow missed the earlier responses. >> We have successfully used t

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

lebedev.ri added a comment. In D66324#2395230 , @haowei wrote: > In D66324#2394687 , @lebedev.ri > wrote: > >> In D66324#2336555 , @lebedev.ri >> wrote: >> >>> In D66324#23

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

lebedev.ri reopened this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. Reverted in rG6861d938e5c946cc7079d9849ef7560d07aa2d80 , looking forward to the rework! Repository: rL LLVM CH

[PATCH] D91055: [clang-tidy] Introduce misc No Integer To Pointer Cast check

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

[PATCH] D90944: [clang-tidy] implement misc-mt-unsafe

lebedev.ri added a comment. `Libc` option name doesn't really make sense to me, maybe `FunctionSet` would fit better. Comment at: clang-tools-extra/clang-tidy/misc/MtUnsafeCheck.cpp:32-33 + +// Initial list was extracted from gcc documentation +static const StringRef glibcFunc

[PATCH] D90944: [clang-tidy] implement misc-mt-unsafe

lebedev.ri added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/MtUnsafeCheck.cpp:279-282 + llvm::DenseSet result; + result.insert(std::begin(posixFunctions), std::end(posixFunctions)); + result.insert(std::begin(glibcFunctions), std::end(glibcFunctions)); + r

[PATCH] D91656: [clang-tidy] add concurrent module

lebedev.ri added a comment. I'm not a native english speaker, but i think the module name be `concurrency`, not `concurrent`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91656/new/ https://reviews.llvm.org/D91656 ___

[PATCH] D78938: Make LLVM build in C++20 mode

lebedev.ri added inline comments. Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h:167-171 - -inline bool operator!=(const DWARFExpression::iterator &LHS, - const DWARFExpression::iterator &RHS) { - return !(LHS == RHS); -} B

[PATCH] D87972: [OldPM] Pass manager: run SROA after (simple) loop unrolling

lebedev.ri updated this revision to Diff 295817. lebedev.ri added a comment. Herald added a subscriber: pengfei. Rebased, NFC Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87972/new/ https://reviews.llvm.org/D87972 Files: llvm/lib/Target/AMDGPU/

[PATCH] D87972: [OldPM] Pass manager: run SROA after (simple) loop unrolling

lebedev.ri added a comment. In D87972#2294614 , @lebedev.ri wrote: > In D87972#2294488 , @xbolva00 wrote: > Does that sound reasonable? >> >> Yes IMHO. >> What are the next suggested steps? >> >> It would

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

lebedev.ri updated this revision to Diff 295879. lebedev.ri retitled this revision from "[clang-tidy] Implement sonarsource-function-cognitive-complexity check" to "[clang-tidy] Implement readability-function-cognitive-complexity check". lebedev.ri added a comment. @aaron.ballman @alexfh it is m

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

lebedev.ri updated this revision to Diff 295902. lebedev.ri marked 10 inline comments as done. lebedev.ri added a comment. In D36836#2309243 , @aaron.ballman wrote: > In D36836#2308981 , @lattner wrote: > >> Hi all

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

lebedev.ri added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:367-368 + bool TraverseStmt(Stmt *Node) { +if (!Node) + return Base::TraverseStmt(Node); + aaron.ballman wrote: > If there's not

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

lebedev.ri updated this revision to Diff 295904. lebedev.ri added a comment. Maybe fine tune comments - we count block decls, not block statements/expressions. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D36836/new/ https://reviews.llvm.org/D3683

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

lebedev.ri marked 4 inline comments as done. lebedev.ri added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:381 +// FIXME: "each method in a recursion cycle" Increment is not implemented. +case Stmt::Conditiona

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

lebedev.ri marked 3 inline comments as done. lebedev.ri added a comment. @aaron.ballman @JonasToth thank you for the review! @tonic @lattner thank you for helping resolve the misunderstading :) > ! In D36836#2309432 , @aaron.ballman > wrote: > LGTM, than

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGace644030e67: [clang-tidy] Implement readability-function-cognitiv

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

lebedev.ri added a comment. In D36836#2310168 , @RKSimon wrote: > @lebedev.ri This is failing on the windows arm build bots - please can you > take a look? > > http://lab.llvm.org:8011/builders/llvm-clang-win-x-armv7l/builds/1482 > http://lab.llvm.org:801

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

lebedev.ri added a comment. In D36836#2310169 , @lebedev.ri wrote: > In D36836#2310168 , @RKSimon wrote: > >> @lebedev.ri This is failing on the windows arm build bots - please can you >> take a look? >> >> http://

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

lebedev.ri added a comment. In D36836#2310170 , @lebedev.ri wrote: > In D36836#2310169 , @lebedev.ri > wrote: > >> In D36836#2310168 , @RKSimon wrote: >> >>> @lebedev.ri Thi

[PATCH] D87972: [OldPM] Pass manager: run SROA after (simple) loop unrolling

lebedev.ri updated this revision to Diff 296005. lebedev.ri added a comment. Re-fix clang tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87972/new/ https://reviews.llvm.org/D87972 Files: clang/test/CodeGenCXX/union-tbaa2.cpp clang/test/Mi

[PATCH] D87972: [OldPM] Pass manager: run SROA after (simple) loop unrolling

lebedev.ri added a comment. In D87972#2310603 , @xbolva00 wrote: >> ! In D87972#2310595 , @nikic wrote: >> >>> I'll just say this LGTM as it establishes parity with what NewPM has been >>> doing for a while already

[PATCH] D87972: [OldPM] Pass manager: run SROA after (simple) loop unrolling

This revision was automatically updated to reflect the committed changes. Closed by commit rG03bd5198b6f7: [OldPM] Pass manager: run SROA after (simple) loop unrolling (authored by lebedev.ri). Changed prior to commit: https://reviews.llvm.org/D87972?vs=296005&id=296028#toc Repository: rG LL

[PATCH] D88789: [InstCombine] Revert rL226781 "Teach InstCombine to canonicalize loads which are only ever stored to always use a legal integer type if one is available." (PR47592)

lebedev.ri updated this revision to Diff 296055. lebedev.ri added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. Rebase/fix remaining tests. In D88789#2310441 , @nlopes wrote: > Not introducing inttoptr during optimizati

[PATCH] D88789: [InstCombine] Revert rL226781 "Teach InstCombine to canonicalize loads which are only ever stored to always use a legal integer type if one is available." (PR47592)

lebedev.ri added a comment. In D88789#2310967 , @efriedma wrote: > To address the issue at hand, I think changing the isNonIntegralPointerType() > check to just isPtrOrPtrVectorTy() would be enough. I think that might make > sense? I briefly considered

[PATCH] D88789: [InstCombine] Revert rL226781 "Teach InstCombine to canonicalize loads which are only ever stored to always use a legal integer type if one is available." (PR47592)

lebedev.ri updated this revision to Diff 296173. lebedev.ri added a comment. NFC Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88789/new/ https://reviews.llvm.org/D88789 Files: clang/test/CodeGen/attr-arm-sve-vector-bits-bitcast.c clang/test/C

[PATCH] D88831: [clang-tidy] Remove obsolete checker google-runtime-references

lebedev.ri added a comment. In D88831#2311800 , @Eugene.Zelenko wrote: > Does this check make sense in content of other style guides? I'd +1 to moving it to `readability`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D88789: [InstCombine] Revert rL226781 "Teach InstCombine to canonicalize loads which are only ever stored to always use a legal integer type if one is available." (PR47592)

lebedev.ri added a comment. So, we can't really teach SCEV about this: D88788 (not without the https://bugs.llvm.org/show_bug.cgi?id=47592 at lease) And we can't recover the situation post-inlining in instcombine: D88842 . It r

[PATCH] D88789: [InstCombine] Revert rL226781 "Teach InstCombine to canonicalize loads which are only ever stored to always use a legal integer type if one is available." (PR47592)

lebedev.ri added a comment. In D88789#2312510 , @lebedev.ri wrote: > So, we can't really teach SCEV about this: D88788 > (not without the > https://bugs.llvm.org/show_bug.cgi?id=47592 at least) > And we can't recover the

[PATCH] D88789: [InstCombine] Revert rL226781 "Teach InstCombine to canonicalize loads which are only ever stored to always use a legal integer type if one is available." (PR47592)

This revision was automatically updated to reflect the committed changes. Closed by commit rGe00f189d392d: [InstCombine] Revert rL226781 "Teach InstCombine to canonicalize loads which… (authored by lebedev.ri). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D88831: [clang-tidy] Remove obsolete checker google-runtime-references

lebedev.ri added a subscriber: hans. lebedev.ri added a comment. @hans it would be nice if this could make it into the release, removing stuff is usually a pretty safe thing to do :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88831/new/ https://

[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

lebedev.ri added a comment. I think the current behavior should instead be configurable, with a way to opt-in into weaker guarantees. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88833/new/ https://reviews.llvm.org/D88833 ___

[PATCH] D88979: [InstCombine] combineLoadToOperationType(): don't fold int<->ptr cast into load

lebedev.ri updated this revision to Diff 297217. lebedev.ri added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. Rebased, NFC. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88979/new/ https://reviews.llvm.org/D889

[PATCH] D89194: [clang-tidy] Fix crash in readability-function-cognitive-complexity on weak refs

lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. Looks good to me, @aaron.ballman ? Comment at: clang-tools-extra/test/clang-tidy/checkers/readability-function-cognitive-complexity.cpp:1017 + +static void dont_cras

[PATCH] D89194: [clang-tidy] Fix crash in readability-function-cognitive-complexity on weak refs

lebedev.ri accepted this revision. lebedev.ri added a comment. Since this is touching more than just a clang-tidy check i still want someone else to sign off, but this looks ok to me in principle. Thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89194/new/ https://reviews.llvm.org/

[PATCH] D88979: [InstCombine] combineLoadToOperationType(): don't fold int<->ptr cast into load

lebedev.ri added inline comments. Comment at: llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:563 if (auto* CI = dyn_cast(LI.user_back())) - if (CI->isNoopCast(DL)) + if (CI->isNoopCast(DL) && LI.getType()->isPtrOrPtrVectorTy() == +

[PATCH] D89194: [clang-tidy] Fix crash in readability-function-cognitive-complexity on weak refs

lebedev.ri added a comment. In D89194#2323920 , @JonasToth wrote: > You can land it. Worst case would be post-commit comments, but this is not a > complicated patch and should be fine! +1 @JonasToth Thanks! CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D88979: [InstCombine] combineLoadToOperationType(): don't fold int<->ptr cast into load

lebedev.ri added inline comments. Comment at: llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:563 if (auto* CI = dyn_cast(LI.user_back())) - if (CI->isNoopCast(DL)) + if (CI->isNoopCast(DL) && LI.getType()->isPtrOrPtrVectorTy() == +

[PATCH] D88979: [InstCombine] combineLoadToOperationType(): don't fold int<->ptr cast into load

lebedev.ri added a comment. In D88979#2323935 , @nikic wrote: > LGTM @nlopes does this look good to you? > Looking through other uses of isNoopCast(), I don't think it makes sense to > push this change into it, as many other usages do need it to work wi

[PATCH] D88979: [InstCombine] combineLoadToOperationType(): don't fold int<->ptr cast into load

lebedev.ri added a comment. @nlopes @nikic thank you for the review! In D88979#2323948 , @nlopes wrote: > In D88979#2323940 , @lebedev.ri > wrote: > >> In D88979#2323935 ,

[PATCH] D88979: [InstCombine] combineLoadToOperationType(): don't fold int<->ptr cast into load

This revision was automatically updated to reflect the committed changes. Closed by commit rG544a6aa2674e: [InstCombine] combineLoadToOperationType(): don't fold int<->ptr cast into load (authored by lebedev.ri). Changed prior to commit: https://reviews.llvm.org/D88979?vs=297217&id=297475#toc

[PATCH] D89332: [clang-tidy] performance-unnecessary-copy-initialization: Always allow std::function to be copied.

lebedev.ri added inline comments. Comment at: clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp:52 AllowedTypes( utils::options::parseStringList(Options.get("AllowedTypes", ""))) {} Just put it here? Repository: r

[PATCH] D89464: Revert "[clang-format] Fix AlignConsecutive on PP blocks"

lebedev.ri added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:12255 Alignment); - - // Bug 25167 Passing-by comment: you probably want to keep the tests though? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[PATCH] D89332: [clang-tidy] performance-unnecessary-copy-initialization: Always allow std::function to be copied.

lebedev.ri added inline comments. Comment at: clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp:52 AllowedTypes( utils::options::parseStringList(Options.get("AllowedTypes", ""))) {} flx wrote: > lebedev.ri wrote: > >

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

lebedev.ri added a subscriber: thakis. lebedev.ri added a comment. Herald added a subscriber: wenlei. @paulkirth @phosek @leonardchan @thakis @nickdesaulniers It seems to me that this feature is still broken an unusable - https://bugs.llvm.org/show_bug.cgi?id=45073 -and the bug was reported more

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

lebedev.ri added a comment. > The associated comment suggested that GCC keeps this extension enabled in > C90/C++03 standard-conforming mode, but it actually does not, so rather than > adding a check for C++ language version, this change simply removes the check > for C language version. Then

[PATCH] D91055: [clang-tidy] Introduce misc No Integer To Pointer Cast check

lebedev.ri added a comment. Ping. If there's no pushback within next 24h i will commit this and wait for post-commit review (if any). Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91055/new/ https://reviews.llvm.org/D91055 ___

[PATCH] D91656: [clang-tidy] add concurrency module

lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang-tools-extra/clang-tidy/CMakeLists.txt:57-58 add_subdirectory(bugprone) add_subdirectory(cert) add_subdirectory(cppcoreguidelines) add_subdi

[PATCH] D91975: clang-tidy: detect narrowing conversions involving typedefs

lebedev.ri added subscribers: gchatelet, courbet, lebedev.ri. lebedev.ri edited reviewers, added: aaron.ballman, njames93; removed: gchatelet, courbet. lebedev.ri added a comment. This revision now requires review to proceed. Whoa, that's a fun bug :) Repository: rG LLVM Github Monorepo CHAN

[PATCH] D91656: [clang-tidy] add concurrency module

lebedev.ri added a comment. Not really, i think this should be landed together with it's dependee, which is not ready yet. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91656/new/ https://reviews.llvm.org/D91656 ___ cfe-commits mailing list

[PATCH] D90944: [clang-tidy] implement concurrency-mt-unsafe

lebedev.ri added a comment. Looks good to me, but i'm still uneasy about doing the checking in inverted sense. Comment at: clang-tools-extra/clang-tidy/concurrency/MtUnsafeCheck.cpp:31-32 + +// Initial list was extracted from gcc documentation +static const StringRef GlibcFunc

[PATCH] D51650: Implement target_clones multiversioning

lebedev.ri added a comment. Herald added a subscriber: jdoerfert. In D51650#1305509 , @erichkeane wrote: > Fix @rsmith s comments, rebase on the big CPUDispatch refactor. Ping. What's the status here? CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D90944: [clang-tidy] implement concurrency-mt-unsafe

lebedev.ri added a comment. In D90944#2418792 , @segoon wrote: > - move static vectors out of namespace Nice. > - mark mt-unsafe decls and check for marks in exprs Eeeh. I was thinking of either some smart matcher "match any function declaration with na

[PATCH] D91656: [clang-tidy] add concurrency module

lebedev.ri added a comment. In D91656#2418790 , @segoon wrote: > - fix sort order Please always upload all patches with full context (`-U`) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91656/new/ https://reviews.llvm.org/D91656 __

[PATCH] D90944: [clang-tidy] implement concurrency-mt-unsafe

lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. Please upload all patches with full context (`-U`) I'm guessing you'll need help committing this, in which case please specify `Author ` to be used for `git commit --author="<>"` T

[PATCH] D91656: [clang-tidy] add concurrency module

This revision was automatically updated to reflect the committed changes. Closed by commit rG8da7efbb0d5e: [clang-tidy] add concurrency module (authored by segoon, committed by lebedev.ri). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91656/new/ ht

[PATCH] D90944: [clang-tidy] implement concurrency-mt-unsafe

This revision was automatically updated to reflect the committed changes. Closed by commit rGcac5be495ed8: [clang-tidy] implement concurrency-mt-unsafe (authored by segoon, committed by lebedev.ri). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90944

[PATCH] D91055: [clang-tidy] Introduce misc No Integer To Pointer Cast check

lebedev.ri updated this revision to Diff 308683. lebedev.ri marked 6 inline comments as done. lebedev.ri added a comment. Herald added a subscriber: phosek. @aaron.ballman @aqjune thank you for taking a look! Rebased, mostly addressed review notes: - Moved into `performance` module - Don't diagn

[PATCH] D92001: [ubsan] Fix crash on __builtin_assume_aligned

lebedev.ri added inline comments. Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:2524 // defined. - if (Ty->getPointeeType().isVolatileQualified()) + if (!Ty->getPointeeType().isNull() && Ty->getPointeeType().isVolatileQualified()) return; vsk wrote

[PATCH] D92001: [ubsan] Fix crash on __builtin_assume_aligned

lebedev.ri requested changes to this revision. lebedev.ri added a comment. This revision now requires changes to proceed. @orivej please can you look where we create AST nodes for these builtins and ensure that the "pointer" argument is actually converted into a pointer beforehand? I'm afraid i

[PATCH] D92257: [clang-format] Add option to control the space at the front of a line comment

lebedev.ri added a comment. In D92257#2435902 , @MyDeveloperDay wrote: > In D92257#2435899 , > @HazardyKnusperkeks wrote: > >> In D92257#2435701 , @MyDeveloperDay >> wrote

[PATCH] D90392: [clang-tidy] Omit std::make_unique/make_shared for default initialization.

lebedev.ri added a comment. I think the patches description still could use some more words, in particular it still only repeats what the patch does, but not why it does that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90392/new/ https://reviews

[PATCH] D91055: [clang-tidy] Introduce misc No Integer To Pointer Cast check

lebedev.ri added a comment. Ugh, i really hate that inline comments aren't submitted upon uploading the new diff, it catches me off-guard each time... Comment at: clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp:40 "misc-new-delete-overloads"); +CheckFactories

[PATCH] D91055: [clang-tidy] Introduce misc No Integer To Pointer Cast check

lebedev.ri updated this revision to Diff 309976. lebedev.ri marked an inline comment as done. lebedev.ri added a comment. @aaron.ballman thank you for taking a look! Addressing review notes: - s/inttoptr/int-to-ptr/ - Adjust diagnostic to be more descriptive of the problem Repository: rG LLVM

[PATCH] D92257: [clang-format] Add option to control the space at the front of a line comment

lebedev.ri added a comment. In D92257#2437918 , @HazardyKnusperkeks wrote: > In D92257#2435906 , @lebedev.ri > wrote: > >> In D92257#2435902 , @MyDeveloperDay >> wrote: >>

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

lebedev.ri added a comment. In D66324#2336186 , @phosek wrote: > I apologize for the late response, I somehow missed the earlier responses. We > have successfully used this feature in Fuchsia and found it useful, but I > agree that the issues raised need

<    11   12   13   14   15   16   17   18   19   20   >