[PATCH] D77461: [WIP][clang-tidy] Remove false positive in AvoidNonConstGlobalVariables

lebedev.ri added a comment. FWIW i've posted https://github.com/isocpp/CppCoreGuidelines/issues/1599 asking for clarification, but no replies so far.. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77461/new/ https://reviews.llvm.org/D77461

[PATCH] D78098: [CGExprAgg] Fix infinite loop in `findPeephole`

lebedev.ri added a comment. Can test be added here? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78098/new/ https://reviews.llvm.org/D78098 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http

[PATCH] D77461: [WIP][clang-tidy] Remove false positive in AvoidNonConstGlobalVariables

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

[PATCH] D79830: Add support of __builtin_expect_with_probability

lebedev.ri added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:1818-1819 +llvm::APFloat Probability = Eval.Val.getFloat(); +if (!(Probability >= llvm::APFloat(0.0) || + Probability <= llvm::APFloat(1.0))) { + Diag(ProbArg->getLocStart(), diag:

[PATCH] D80492: Avoid linking libdl unless needed

lebedev.ri added a comment. FYI as reported on IRC, this likely breaks standalone build of clang standalone builds of clang are still supported right ? commit 0073c293a401774ac96b4b3d27f05e13f379f98e seems to break them, since HAVE_LIBDL is not defined (it requires config-ix include from llv

[PATCH] D81336: [clang-tidy] simplify-bool-expr ignores template instantiations

lebedev.ri added a comment. Thank you for looking into it! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81336/new/ https://reviews.llvm.org/D81336 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D81385: Fix libdl linking for libclang in standalone mode

lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. Thank you! mceier: https://reviews.llvm.org/D81385 LebedevRI: clang built successfully with that patch But consider waiting for @beanz Repository: rG LLVM Github Monorepo CH

[PATCH] D81420: Fix size for _ExtInt types with builtins

lebedev.ri added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:1747-1748 break; case Builtin::BI__builtin_add_overflow: case Builtin::BI__builtin_sub_overflow: case Builtin::BI__builtin_mul_overflow: I don't think this applies to `add

[PATCH] D78442: Create a warning flag for 'warn_conv_*_not_used'

lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. Sorry for the delay, LG. Comment at: clang/test/Misc/warning-flags.c:21 -CHECK: Warnings without flags (72): +CHECK: Warnings without flags (69): +

[PATCH] D81786: [clang][utils] Modify make-ast-dump-check.sh to generate AST serialization dump tests

lebedev.ri added a comment. +1, thank you for looking into this! I suspect there's a few issues to be discovered with this :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81786/new/ https://reviews.llvm.org/D81786 __

[PATCH] D81336: [clang-tidy] simplify-bool-expr ignores template instantiations

lebedev.ri added a comment. In D81336#2091833 , @aaron.ballman wrote: > In D81336#2090708 , > @LegalizeAdulthood wrote: > > > Yeah, the restriction to the header file is a bug. It was a misconception > > on my p

[PATCH] D81786: [clang][utils] Modify make-ast-dump-check.sh to generate AST serialization dump tests

lebedev.ri added a comment. SGTM, thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81786/new/ https://reviews.llvm.org/D81786 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lis

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

lebedev.ri added a comment. In D36836#1426884 , @lebedev.ri wrote: > In D36836#1021863 , @chandlerc wrote: > > > In D36836#931995 , @lebedev.ri > > wrote: > > > > > - Rebased

[PATCH] D81336: [clang-tidy] simplify-bool-expr ignores template instantiations

lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. LG, unless @aaron.ballman has concerns. Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81336/new/ https://reviews.llvm.org/D81336

[PATCH] D81938: [SROA] Teach SROA to perform no-op pointer conversion.[InferAddressSpaces] Handle the pair of `ptrtoint`/`inttoptr`.

lebedev.ri added a comment. This should be two separate patches - inferaddressspace and SROA. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81938/new/ https://reviews.llvm.org/D81938 ___ cfe-commits ma

[PATCH] D81953: [clang-tidy] warnings-as-error no longer exits with ErrorCount

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

[PATCH] D82994: [RFC] Instrumenting Clang/LLVM with Perfetto

lebedev.ri added a comment. New files are missing standard license header blurb. Also probably missing some tests. Comment at: clang/lib/Driver/Driver.cpp:3754 + // We don't need to count the assembler as a job since it doesn't + // cause the memory issue that requires disa

[PATCH] D71739: [AssumeBundles] Use operand bundles to encode alignment assumptions

lebedev.ri reopened this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. Reverted in 7ea46aee3670981827c04df89b2c3a1cbdc7561b . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST A

[PATCH] D83013: [LPM] Port CGProfilePass from NPM to LPM

lebedev.ri added a comment. This seems to have broken the build http://45.33.8.238/linux/22500/step_7.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83013/new/ https://reviews.llvm.org/D83013 ___ cf

[PATCH] D82604: No nested namespaces in Clang-Tidy checkers

lebedev.ri added a comment. Isn't that C++17? LLVM is at C++14. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82604/new/ https://reviews.llvm.org/D82604 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D46317: [clang-tidy] New check bugprone-map-subscript-operator-lookup

lebedev.ri added a comment. Herald added subscribers: phosek, mgehre. IMHO we should proceed with this patch. There's been several patches recently that seem to not care about false-positive rate, and in this case the cost of true-positive can be humongous, as i/we've been finding lots of bugs e

[PATCH] D78491: Avoid relying on address space zero default parameter in llvm/IR

lebedev.ri added subscribers: llvm-commits, lebedev.ri. lebedev.ri added a comment. This should really be reviewed on llvm-commits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78491/new/ https://reviews.llvm.org/D78491

[PATCH] D78659: Add nomerge function attribute to supress tail merge optimization in simplifyCFG

lebedev.ri added a subscriber: llvm-commits. lebedev.ri added a comment. + llvm-commits Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78659/new/ https://reviews.llvm.org/D78659 ___ cfe-commits mailing

[PATCH] D78659: Add nomerge function attribute to supress tail merge optimization in simplifyCFG

lebedev.ri added a comment. 1. Tests missing 2. Why isn't `noinline` sufficient, why a whole new attribue? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78659/new/ https://reviews.llvm.org/D78659 ___ c

[PATCH] D74051: Move update_cc_test_checks.py tests to clang

lebedev.ri added a comment. In D74051#2016078 , @mgorny wrote: > This broke running clang tests stand-alone: > > Traceback (most recent call last): > File > "/var/tmp/portage/sys-devel/clang-11.0.0./work/x/y/clang-abi_x86_64.amd64/bin/../../llvm

[PATCH] D85097: [Sema] add warning for comparisons like 'x<=y<=z'

lebedev.ri added a comment. Thank you! I strongly prefer this path forward. Comment at: clang/lib/Sema/SemaExpr.cpp:14010 + << Bop->getSourceRange() << OpLoc; + SuggestParentheses(Self, Bop->getOperatorLoc(), + Self.PDiag(diag::note_precedence_silence)

[PATCH] D74436: Change clang option -ffp-model=precise to select ffp-contract=on

lebedev.ri requested changes to this revision. lebedev.ri added a comment. This revision now requires changes to proceed. In D74436#2190422 , @fhahn wrote: > IIUC the patch is currently reverted (in > 78654e8511cf16d49f6680d782f3771a767ba942 >

[PATCH] D74436: Change clang option -ffp-model=precise to select ffp-contract=on

lebedev.ri added a comment. In D74436#2190492 , @lebedev.ri wrote: > <...> And by codegen changes i mostly mean newly-set/now-unset fp fast-math instruction flags. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D67253: clang-misexpect: a standalone tool for verifying the use of __builtin_expect with PGO data

lebedev.ri requested changes to this revision. lebedev.ri added a comment. This revision now requires changes to proceed. Abandon this? D66324 landed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67253/new/ https:

[PATCH] D85787: [InstCombine] Aggregate reconstruction simplification (PR47060)

lebedev.ri updated this revision to Diff 285283. lebedev.ri added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fix (overstepping) clang test being affected by this change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D85787: [InstCombine] Aggregate reconstruction simplification (PR47060)

lebedev.ri updated this revision to Diff 285371. lebedev.ri added a comment. Drop unneeded header include. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85787/new/ https://reviews.llvm.org/D85787 Files: clang/test/CodeGenCXX/nrvo.cpp llvm/lib/

[PATCH] D85787: [InstCombine] Aggregate reconstruction simplification (PR47060)

lebedev.ri added a comment. @spatel thank you for taking a look! In D85787#2216518 , @spatel wrote: > In D85787#2214038 , @lebedev.ri > wrote: > >> @ reviewers - i'm not so much interested in deep code/algo review

[PATCH] D85938: [OpenMP][NFC] Update check lines after D85099

lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. Thanks. Seems to fix the test for me FWIW, but 1. why wasn't this caught by anyone and all of the bots? 2. what is the actual underlying problem, if any? Repository: rG LLVM Github

[PATCH] D85938: [OpenMP][OMPIRBuilder] Use the source (=directory + filename) for locations

lebedev.ri added a comment. still lg, i guess Comment at: clang/test/OpenMP/irbuilder_nested_parallel_for.c:258 +// CHECK-DEBUG-NEXT:call void @__kmpc_barrier(%struct.ident_t* [[GLOB83:@.*]], i32 [[OMP_GLOBAL_THREAD_NUM239]]), [[DBG107]] +// CHECK-DEBUG-NEXT:ret void,

[PATCH] D86000: Add an unsigned shift base sanitizer

lebedev.ri added a comment. What's next, a sanitizer that input to signed right-shift is always non-negative? :) FWIW "fixing" these "bugs" via `(x & ~(~1U << (32-shamt))) << shamt` is going to be fine, i've already taught instcombine to drop such pointless masking before left-shift in PR42563

[PATCH] D85787: [InstCombine] Aggregate reconstruction simplification (PR47060)

lebedev.ri added a comment. @spatel thank you for taking a look! In D85787#2220343 , @spatel wrote: > This seems similar in spirit to the recursive element tracking that we do in > collectShuffleElements() in this file or foldIdentityShuffles() in > Ins

[PATCH] D85787: [InstCombine] Aggregate reconstruction simplification (PR47060)

lebedev.ri updated this revision to Diff 285898. lebedev.ri marked 4 inline comments as done. lebedev.ri added a comment. Patch updated - addressed all nits other than the question about decoupling `enum` values from variable names. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D85787: [InstCombine] Aggregate reconstruction simplification (PR47060)

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGae7f08812e09: [InstCombine] Aggregate reconstruction simplification (PR47060) (authored by lebedev.ri). Repository: rG LLVM Github Monorepo CHANG

[PATCH] D78659: Add nomerge function attribute to supress tail merge optimization in simplifyCFG

lebedev.ri added inline comments. Comment at: llvm/include/llvm/IR/Attributes.td:106 +/// Disable tail merge for this function +def NoMerge : EnumAttr<"nomerge">; Elsewhere we seem to be preventing any kind of movement, not just disabling inlining of such funct

[PATCH] D79830: Add support of __builtin_expect_with_probability

lebedev.ri added a reviewer: erichkeane. lebedev.ri added a comment. Thanks for working on this. Please upload patch with full context. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79830/new/ https://reviews.llvm.org/D79830

[PATCH] D79830: Add support of __builtin_expect_with_probability

lebedev.ri added a comment. In D79830#2034779 , @LukeZhuang wrote: > In D79830#2033367 , @lebedev.ri > wrote: > > > Thanks for working on this. > > Please upload patch with full context. > > > Hi, sorry but I'm no

[PATCH] D40463: [analyzer] Fix false negative on post-increment of uninitialized variable.

lebedev.ri added inline comments. Comment at: docs/ReleaseNotes.rst:261 +- Static Analyzer can now properly detect and diagnose unary pre-/post- + increment/decrement on an uninitialized values. + JonasToth wrote: > The end of the sentence should be either 'on u

[PATCH] D33826: [clang-tidy] avoid pointer cast to more strict alignment check

lebedev.ri added a comment. Ping? Repository: rL LLVM https://reviews.llvm.org/D33826 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40505: [clang-tidy] Ignore ExprWithCleanups when looking for else-after-throw

lebedev.ri added a comment. How about also matching on call to functions with no-return attribute? i.e. [[noreturn]] my_die(); void do_stuff(); void fn(int x) { if(!x) my_die(); else // <- since `my_die()` will never return, `else` is not really needed. do_stuff();

[PATCH] D39462: [Sema] Implement -Wmaybe-tautological-constant-compare for when the tautologicalness is data model dependent

lebedev.ri added a comment. In https://reviews.llvm.org/D39462#936566, @bcain wrote: > I'd like to understand/resurrect this change, so I'll try to summarize. > Please correct this as appropriate: > > 1. We got here because libc++ has code that triggers a warning for some > targets (those whos

[PATCH] D40463: [analyzer] Fix false negative on post-increment of uninitialized variable.

lebedev.ri added inline comments. Comment at: lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp:64 +if (const UnaryOperator *U = dyn_cast(StoreE)) { + str = "The expression of the unary operator is an uninitialized value. " +"The computed value will

[PATCH] D40463: [analyzer] Fix false negative on post-increment of uninitialized variable.

lebedev.ri added inline comments. Comment at: lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp:64 +if (const UnaryOperator *U = dyn_cast(StoreE)) { + str = "The expression of the unary operator is an uninitialized value. " +"The computed value will

[PATCH] D40463: [analyzer] Fix false negative on post-increment of uninitialized variable.

lebedev.ri added a comment. In https://reviews.llvm.org/D40463#939772, @dcoughlin wrote: > Thanks, this looks good to me! Awesome, thank you! > Do you have commit access or do you need someone to commit it for you? I do have commit access. Repository: rC Clang https://reviews.llvm.org/D4

[PATCH] D40671: Support specific categories for NOLINT directive

lebedev.ri added a comment. Please upload patches with full context (`-U99`) Also, you probably want to add some reviewers, see CODE_OWNERS.txt e.g. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40671 ___ cfe-commits mailing li

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

lebedev.ri updated this revision to Diff 125178. lebedev.ri changed the repository for this revision from rL LLVM to rCTE Clang Tools Extra. lebedev.ri added a comment. Rebased. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D36836 Files: LICENSE.TXT clang-tidy/CMakeLists.t

[PATCH] D36892: [clang-tidy] check_clang_tidy.py: support CHECK-NOTES prefix

lebedev.ri added a comment. @alexfh any thoughts on this one? Repository: rL LLVM https://reviews.llvm.org/D36892 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

lebedev.ri added inline comments. Comment at: docs/ReleaseNotes.rst:259 +- The ``NOLINT`` and ``NOLINTNEXTLINE`` suppression comments were extended to support the list of checks + to disable in parentheses. This reads strange, maybe it can be reworded somehow

[PATCH] D41048: [libcxx] workaround PR 28385 in __find_exactly_one_checked

lebedev.ri added a comment. Test? https://reviews.llvm.org/D41048 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41048: [libcxx] workaround PR 28385 in __find_exactly_one_checked

lebedev.ri added a comment. In https://reviews.llvm.org/D41048#950606, @CaseyCarter wrote: > In https://reviews.llvm.org/D41048#950601, @mclow.lists wrote: > > > These tests don't fail for me. (using a clang I built two days ago) > > > What about the repro for #35578 >

[PATCH] D36892: [clang-tidy] check_clang_tidy.py: support CHECK-NOTES prefix

lebedev.ri updated this revision to Diff 126681. lebedev.ri changed the repository for this revision from rL LLVM to rCTE Clang Tools Extra. lebedev.ri added a comment. Rebased. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D36892 Files: test/clang-tidy/check_clang_tidy.py

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

lebedev.ri updated this revision to Diff 126682. lebedev.ri added a comment. Rebased. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D36836 Files: LICENSE.TXT clang-tidy/CMakeLists.txt clang-tidy/plugin/CMakeLists.txt clang-tidy/sonarsource/CMakeLists.txt clang-tidy/so

[PATCH] D7375: [clang-tidy] Assert related checkers

lebedev.ri added inline comments. Herald added a subscriber: mgorny. Comment at: test/clang-tidy/misc-static-assert.cpp:71 + + assert(ZERO_MACRO && "Report me!"); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: found assert() that could be Hmm. Are you sure about

[PATCH] D36892: [clang-tidy] check_clang_tidy.py: support CHECK-NOTES prefix

lebedev.ri added a comment. In https://reviews.llvm.org/D36892#953891, @aaron.ballman wrote: > I think you're set to commit this Should i commit it though? Until licensing concerns with https://reviews.llvm.org/D36836 are finally magically resolved and it is committed, there won't be any users

[PATCH] D72742: Don't assume promotable integers are zero/sign-extended already in x86-64 ABI.

lebedev.ri added a comment. Herald added subscribers: StephenFan, sstefan1. Herald added a project: All. Passing-by remark: i'm not sure it was noted already, but i believe that the function linkage should be differentiated here, the ABI mismatch can only exist if the function can actually be call

[PATCH] D105728: [clang][Codegen] Directly lower `(*((volatile int *)(0))) = 0;` into a `call void @llvm.trap()`

lebedev.ri added a subscriber: efriedma. lebedev.ri added a comment. @rsmith @efriedma ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105728/new/ https://reviews.llvm.org/D105728 ___ cfe-commits mail

[PATCH] D107004: Turn off fp reassociation in IA intrinsic header files.

lebedev.ri added a comment. Patch description states what the change is, but not why the change is what it is. I don't think this is a good direction to second-guess the user's intentions. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107004/new/

[PATCH] D121457: Clamp negative coverage counters to zero.

lebedev.ri added a comment. Wouldn't it be best to instead fix the non-thread-safety of the coverage counters? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121457/new/ https://reviews.llvm.org/D121457

[PATCH] D121629: clang: also check alloc_alignment claims in return

lebedev.ri added a reviewer: lebedev.ri. lebedev.ri added a comment. Please can you pose the next patch itself? I suspect that propagation should simply not be done when sanitizer is enabled. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121629/new/

[PATCH] D119271: clang: emit allocalign to LLVM for alloc_align attributes

lebedev.ri added a reviewer: lebedev.ri. lebedev.ri added inline comments. Comment at: clang/test/CodeGen/catch-alignment-assumption-attribute-alloc_align-on-function-variable.cpp:55 // CHECK-NEXT:%[[ALIGNMENT_RELOADED:.*]] = load i64, i64* %[[ALIGNME

[PATCH] D119271: clang: emit allocalign to LLVM for alloc_align attributes

lebedev.ri added inline comments. Comment at: clang/test/CodeGen/catch-alignment-assumption-attribute-alloc_align-on-function-variable.cpp:55 // CHECK-NEXT:%[[ALIGNMENT_RELOADED:.*]] = load i64, i64* %[[ALIGNMENT_ADDR]], align 8 - // CHECK-NEXT:

[PATCH] D119271: clang: emit allocalign to LLVM for alloc_align attributes

lebedev.ri added inline comments. Comment at: clang/test/CodeGen/catch-alignment-assumption-attribute-alloc_align-on-function-variable.cpp:55 // CHECK-NEXT:%[[ALIGNMENT_RELOADED:.*]] = load i64, i64* %[[ALIGNMENT_ADDR]], align 8 - // CHECK-NEXT:

[PATCH] D119271: clang: emit allocalign to LLVM for alloc_align attributes

lebedev.ri added inline comments. Comment at: clang/test/CodeGen/catch-alignment-assumption-attribute-alloc_align-on-function-variable.cpp:55 // CHECK-NEXT:%[[ALIGNMENT_RELOADED:.*]] = load i64, i64* %[[ALIGNMENT_ADDR]], align 8 - // CHECK-NEXT:

[PATCH] D122147: Remove the clang/INPUTS directory; NFC

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

[PATCH] D123544: [randstruct] Automatically randomize a structure of function pointers

lebedev.ri added a comment. Does this not lead to non-deterministic/non-reproducible builds? I do not understand why this feature must be inflicted onto everyone. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123544/new/ https://reviews.llvm.org/D1

[PATCH] D84225: [CFE] Add nomerge function attribute to inline assembly.

lebedev.ri added a comment. In D84225#3303771 , @pengfei wrote: > In D84225#3302142 , @rnk wrote: > >> I think LLVM already doesn't do some tail merging optimizations on inline >> asm, but allowing the use of the a

[PATCH] D110663: [Driver] Support Debian multiarch style lib/clang/14.0.0/x86_64-linux-gnu runtime path and include/x86_64-linux-gnu/c++/v1 libc++ path

lebedev.ri added a comment. I'm personally ignoring this review because quite honestly i was horrified of just how nonchalantly dismissive/accusative the reaction was to the problem i raised during the revert. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D109239: Add support for floating-option `-ffp-eval-method` and for new `pragma clang fp eval-method`

lebedev.ri added subscribers: cfe-commits, lebedev.ri. lebedev.ri added a comment. Looks like the lists weren't subscribed for the review CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109239/new/ https://reviews.llvm.org/D109239 ___ cfe-commi

[PATCH] D66481: [C++20] Support for lambdas in unevaluated context

lebedev.ri added a comment. Doesn't this inadvertently allow them in every standard? That seems wrong. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66481/new/ https://reviews.llvm.org/D66481 ___ cfe-commits mailing

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

lebedev.ri added a comment. I also strongly -1 to disabling the warning for macros. It's just a disservice to everyone, and the fact there's precedent doesn't mean it's the right solution. That being said, does using C++14 binary literals also silence the diag? It should. CHANGES SINCE LAST A

[PATCH] D64695: [clang-format] Modified SortIncludes and IncludeCategories to priority for sorting #includes within the Group Category.

lebedev.ri added a comment. In D64695#1614386 , @MyDeveloperDay wrote: > Assuming this works and the other unit tests don't show issues then this LGTM. > Please consider running this on your NetBSD code base before committing, if > possible please al

[PATCH] D66559: [OPENMP] Update the diagnosis message for canonical loop form

lebedev.ri added a comment. Shouldn't this be OpenMP-version-dependent? `!=` is only allowed in 4.5+ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66559/new/ https://reviews.llvm.org/D66559 ___ cfe-com

[PATCH] D66559: [OPENMP] Update the diagnosis message for canonical loop form

lebedev.ri added a comment. Or more specifically, why `!=` is now silently accepted in 4.0 mode? https://godbolt.org/z/M6iL4H Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66559/new/ https://reviews.llvm.org/D66559 __

[PATCH] D66186: [Sema] Don't warn on printf('%hd', [char]) (PR41467)

lebedev.ri added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:8080-8083 + analyze_printf::ArgType::MatchKind Match = AT.matchesType(S.Context, ExprTy); bool Pedantic = Match == analyze_printf::ArgType::NoMatchPedantic; if (Match == analyze_printf::ArgType::

[PATCH] D66186: [Sema] Don't warn on printf('%hd', [char]) (PR41467)

lebedev.ri added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:8100-8107 +// All further checking is done on the subexpression +Match = AT.matchesType(S.Context, ExprTy); +if (Match) { + if (Match == analyze_printf::ArgType::NoMatch

[PATCH] D66186: [Sema] Don't warn on printf('%hd', [char]) (PR41467)

lebedev.ri requested changes to this revision. lebedev.ri added a comment. This revision now requires changes to proceed. Please can you explain why the snippet i posted in-line does not work for you? Comment at: clang/lib/Sema/SemaChecking.cpp:8105-8107 + if (Implicit

[PATCH] D66186: [Sema] Don't warn on printf('%hd', [char]) (PR41467)

lebedev.ri added a comment. Looks about right Comment at: clang/lib/Sema/SemaChecking.cpp:8105-8106 +if (ImplicitMatch == analyze_printf::ArgType::Match) return true; +else if (ImplicitMatch == analyze_printf::ArgType::NoMatchPedantic) + Peda

[PATCH] D66186: [Sema] Don't warn on printf('%hd', [char]) (PR41467)

lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. In D66186#1642607 , @aaron.ballman wrote: > LGTM aside from the else after return nit. Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D66646: Ensure that statements, expressions and types are trivially destructible with a static_assert

lebedev.ri added a comment. SGTM, but i wonder if this should be done one level up, in `BumpPtrAllocator` itself? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66646/new/ https://reviews.llvm.org/D66646 ___ cfe-comm

[PATCH] D66186: [Sema] Don't warn on printf('%hd', [char]) (PR41467)

lebedev.ri requested changes to this revision. lebedev.ri added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Sema/SemaChecking.cpp:8106 return true; +Pedantic = true; } `if (ImplicitMatch == analy

[PATCH] D66186: [Sema] Don't warn on printf('%hd', [char]) (PR41467)

lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. LG, thank you. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66186/new/ https://reviews.llvm.org/D66186 _

[PATCH] D66646: Ensure that statements, expressions and types are trivially destructible with a static_assert

lebedev.ri added a comment. In D66646#1642730 , @riccibruno wrote: > In D66646#1642708 , @lebedev.ri > wrote: > > > SGTM, but i wonder if this should be done one level up, in > > `BumpPtrAllocator` itself? > > > I

[PATCH] D66695: msan, codegen, instcombine: Keep more lifetime markers used for msan

lebedev.ri added inline comments. Comment at: llvm/test/Transforms/InstCombine/lifetime-sanitizer.ll:37-50 +define void @msan() sanitize_memory { +entry: + ; CHECK-LABEL: @msan( + %text = alloca i8, align 1 + + call void @llvm.lifetime.start.p0i8(i64 1, i8* %text) + call void

[PATCH] D66067: Push lambda scope earlier when transforming lambda expression

lebedev.ri added a reviewer: aaron.ballman. lebedev.ri added inline comments. Comment at: clang/test/SemaTemplate/default-arguments-cxx0x.cpp:1 -// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify %s +// RUN: %clang_cc1 -fsyntax-only -std=c++14 -verify %s // expected-no-diagnost

[PATCH] D64671: [clang-tidy] New check: misc-init-local-variables

lebedev.ri added inline comments. Comment at: clang-tools-extra/test/clang-tidy/cppcoreguidelines-init-variables.cpp:3 + +#include + Eugene.Zelenko wrote: > It'll be better to include cstdint. I'll correct that comment: tests normally shouldn't depend on system

[PATCH] D64671: [clang-tidy] New check: misc-init-local-variables

lebedev.ri added inline comments. Comment at: clang-tools-extra/test/clang-tidy/cppcoreguidelines-init-variables.cpp:3 + +#include + aaron.ballman wrote: > Eugene.Zelenko wrote: > > lebedev.ri wrote: > > > Eugene.Zelenko wrote: > > > > It'll be better to includ

[PATCH] D66646: Ensure that statements, expressions and types are trivially destructible with a static_assert

lebedev.ri accepted this revision. lebedev.ri added a subscriber: rsmith. lebedev.ri added a comment. This revision is now accepted and ready to land. SGTM but please wait a bit in case @aaron.ballman / @rsmith wants to comment. Thanks. Repository: rC Clang CHANGES SINCE LAST ACTION https:/

[PATCH] D66726: [Bitfield] Disable -ffine-grained-bitfield-accesses only when thread sanitizer is enabled

lebedev.ri added a comment. I suppose this is fine for ubsan/lsan/asan, but does msan track things on smaller-than-byte level? Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66726/new/ https://reviews.llvm.org/D66726

[PATCH] D66695: msan, codegen, instcombine: Keep more lifetime markers used for msan

lebedev.ri added inline comments. Comment at: llvm/test/Transforms/InstCombine/lifetime-sanitizer.ll:37-50 +define void @msan() sanitize_memory { +entry: + ; CHECK-LABEL: @msan( + %text = alloca i8, align 1 + + call void @llvm.lifetime.start.p0i8(i64 1, i8* %text) + call void

[PATCH] D66822: Hardware cache line size builtins

lebedev.ri added a comment. > My implementation switches on the target triple to get the max cache line > size for that target. I am not sure if this is the best way to implement > these builtins, but it will ensure that there is not an ABI break. Passing-by remark: i'm not sure it is possible

[PATCH] D65300: [clang] [CodeGen] clang-misexpect prototype for compiler warnings

lebedev.ri added a comment. Please can you clarify hat's the current layout of these patches? Is this patch required, or is it superseded by D66324 (and thus should be abandoned)? I'd like to begin reviewing, but i don't understand where to start. Repository:

[PATCH] D65300: [clang] [CodeGen] clang-misexpect prototype for compiler warnings

lebedev.ri added a comment. In D65300#1647775 , @paulkirth wrote: > In D65300#1647746 , @lebedev.ri > wrote: > > > Please can you clarify hat's the current layout of these patches? > > Is this patch required, or i

[PATCH] D66850: [AST][JSON] Avoid crash when dumping NULL Type as JSON

lebedev.ri added a comment. Test coverage missing. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66850/new/ https://reviews.llvm.org/D66850 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listi

[PATCH] D66559: [OPENMP] Update the diagnosis message for canonical loop form

lebedev.ri added a comment. `!=` is still not being diagnosed in pre-5.0 mode though? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66559/new/ https://reviews.llvm.org/D66559 ___ cfe-commits mailing li

[PATCH] D43779: [Tooling] [0/1] Refactor FrontendActionFactory::create() to return std::unique_ptr<>

lebedev.ri abandoned this revision. lebedev.ri added a comment. Herald added a project: LLVM. Superseded by D66960 Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D43779/new/ https://reviews.llvm.org/D43779 ___

[PATCH] D66960: [Tooling] Migrated APIs that take ownership of objects to unique_ptr

lebedev.ri added a comment. This is missing documentation changes. And this likely would be good to mention in releasenotes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66960/new/ https://reviews.llvm.org/D66960 ___

<    9   10   11   12   13   14   15   16   17   18   >