[PATCH] D89765: [LibTooling][Clang-cast] A Clang LibTool to convert C-style casts to C++ style casts and more.

lebedev.ri added a comment. Reminder to upload patches with diff to master, not last patch/commit/upload. Wouldn't it be better for this to be a clang-tidy check, as opposed to a standalone tool? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89765

[PATCH] D89959: UBSAN: emit distinctive traps in trapping mode

lebedev.ri added a comment. Missing langref changes for new intrinsic. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89959/new/ https://reviews.llvm.org/D89959 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi

[PATCH] D89986: [AIX] do not emit visibility attribute into IR when there is -mignore-xcoff-visibility

lebedev.ri added inline comments. Comment at: clang/test/CodeGen/aix-visibility-inlines-hidden.cpp:30-34 +// COMMON-ASM: mflr 0 +// COMMON-ASM-NEXT:stw 0, 8(1) +// COMMON-ASM-NEXT:stwu 1, -64(1) +// COMMON-ASM-NEXT:bl ._Z1fv +// NOP-ASM-NEXT: nop ---

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

lebedev.ri requested changes to this revision. lebedev.ri added a comment. This revision now requires changes to proceed. Please can you point me where you've added the negative test for the false-positive issue that caused the revert last time? Repository: rG LLVM Github Monorepo CHANGES SI

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

lebedev.ri added inline comments. Comment at: clang/test/SemaCXX/warn-thread-safety-negative.cpp:87-89 + void test4() { +MutexLock lock(&mu); // expected-warning {{acquiring mutex 'mu' requires negative capability '!mu'}} + } aaronpuchert wrote: > @lebedev

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

lebedev.ri resigned from this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. Thanks. Note that i didn't check that this doesn't cause any new false-positives Comment at: clang/test/SemaCXX/warn-thread-safety-negative.cpp:87-89 + void tes

[PATCH] D90221: Include attribute details when dumping AST in JSON

lebedev.ri added a comment. Are there tests missing? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90221/new/ https://reviews.llvm.org/D90221 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https

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

lebedev.ri added a comment. In D90392#2362308 , @ckennelly wrote: > In D90392#2362118 , @njames93 wrote: > >> IIUC, this is handling the case where `Ptr.reset(new int)` which is >> different to `Ptr.reset(new int()

[PATCH] D100388: [BROKEN][clang] Try to fix thunk function types

lebedev.ri updated this revision to Diff 344877. lebedev.ri marked 2 inline comments as done. lebedev.ri added a comment. In D100388#2749115 , @efriedma wrote: > Made a couple suggestions to make this easier to review. > > The test changes you've made so

[PATCH] D100388: [BROKEN][clang] Try to fix thunk function types

lebedev.ri added a comment. In D100388#2754872 , @efriedma wrote: >> Would it be reasonable to instead start with a stopgap measure of not adding >> attributes for this/return of thunks? > > You mean, add align attributes to "this" on regular methods, bu

[PATCH] D100388: [BROKEN][clang] Try to fix thunk function types

lebedev.ri added inline comments. Comment at: clang/lib/CodeGen/CGVTables.cpp:467 + if (!CalleeMD->isDefined()) { +CGM.ErrorUnsupported(ThunkGD.getDecl(), "thunk for forward declaration"); +return; efriedma wrote: > "IsUnprototyped" means that we have to

[PATCH] D99790: [CGCall] Annotate `this` argument with alignment

lebedev.ri abandoned this revision. lebedev.ri added a comment. Oh, i forgot that this review was here. I've just relanded this in rG16d03818412415c56efcd482d18c0cbdf712524c , after workarounding the thunk issue in rGa624cec56

[PATCH] D102502: [clang] Fix ternary operator in second for loop argument

lebedev.ri added a comment. This should have codegen, and maybe AST, tests. Comment at: clang/test/Parser/cxx2a-init-statement.cpp:18 + for (int i = 0; int x = i < 2 ? 1 : 0; i++) {} + This isn't cxx2a-specific isn't it? Isn't this valid even in C99? Repos

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

lebedev.ri resigned from this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. No real comments from me. I assume, the errors are because `-ffp-contract=on` actually results in *less* error? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D96223: [clang-tidy] Simplify static assert check

lebedev.ri reopened this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. Herald added a project: clang-tools-extra. Reverted in be6b9e8ae71768d2e09ec14619ca4ecfdef553fa - https://godbolt.

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

lebedev.ri resigned from this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. Removing from queue - i don't expect to review this. Looks like this has been reverted twice now, presumably llvm stage 2/linux kernel/chrome should be enough of a coverage to be s

[PATCH] D94827: [SimplifyCFG] If provided, preserve Dominator Tree

lebedev.ri updated this revision to Diff 319624. lebedev.ri retitled this revision from "[SimplifyCFG] Require and preserve dominator tree" to "[SimplifyCFG] If provided, preserve Dominator Tree". lebedev.ri edited the summary of this revision. lebedev.ri added a comment. Let me resell this. This

[PATCH] D94827: [SimplifyCFG] If provided, preserve Dominator Tree

lebedev.ri added a comment. @kuhar @mkazantsev wanna get in on the hype train? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94827/new/ https://reviews.llvm.org/D94827 ___ cfe-commits mailing list cfe-co

[PATCH] D94827: [SimplifyCFG] If provided, preserve Dominator Tree

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG8cfa963463d0: [SimplifyCFG] If provided, preserve Dominator Tree (authored by lebedev.ri). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D94355: [SimplifyCFG] Add relative switch lookup tables

lebedev.ri added a comment. Can you please add an explanation to the patch's description as to why we don't want to instead convert non-relative/relative LUT's elsewhere, please. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94355/new/ https://revi

[PATCH] D94355: [SimplifyCFG] Add relative switch lookup tables

lebedev.ri added inline comments. Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:5512-5525 + // If not in x86 or aarch64 mode, do not generate a relative lookup table. + Triple TargetTriple(M.getTargetTriple()); + if (!(TargetTriple.getArch() == Triple::x86_64 || +

[PATCH] D94355: [SimplifyCFG] Add relative switch lookup tables

lebedev.ri added a comment. In D94355#2531504 , @gulfem wrote: > In D94355#2530225 , @lebedev.ri > wrote: > >> Can you please add an explanation to the patch's description as to why >> we don't want to instead conv

[PATCH] D110037: [X86] Always check the size of SourceTy before getting the next type

lebedev.ri added inline comments. Comment at: clang/test/CodeGen/X86/va-arg-sse.c:2 // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py // RUN: %clang_cc1 %s -O2 -emit-llvm -o - -triple x86_64-unknown-unknown | FileCheck %s Please d

[PATCH] D72566: [clang-tidy] Clang tidy is now alias aware

lebedev.ri added a comment. I think this should be as simple as having an opt-in "DisableAllAliaseeChecks" option in `.clang-tidy`, that force-disables all the checks that are actually aliases and not proper checks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

[PATCH] D110436: [WIP] Add %n format specifier warning

lebedev.ri added a comment. The patch description says what the change is, but not why it is the way it is. In particular, it might be helpful to be more verbose than just stating that something is unsafe. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

[PATCH] D110663: [Driver] Support Debian multiarch style lib/clang/14.0.0/x86_64-linux-gnu runtime path

lebedev.ri added a comment. This doesn't help the world-breaking D107799 on debian for me. I don't know if this stands on it's own. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110663/new/ https://reviews.llvm.o

[PATCH] D96281: [clang-tidy] Add options to flag individual core increments and to ignore macros to readability-function-cognitive-complexity check.

lebedev.ri added a comment. It would be better to split this into two parts. Comment at: clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:540 + if (!FlagBasicIncrements) { +return; Drop `{}` Comment at: c

[PATCH] D103958: [WIP] Support MustControl conditional control attribute

lebedev.ri added a comment. This is missing langref changes, and a RFC to llvm-dev. I'm rather skeptical of this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103958/new/ https://reviews.llvm.org/D103958 __

[PATCH] D90835: [clang-tidy] Ignore diagnostics due to macro expansion from not-interested headers

lebedev.ri added a comment. This needs to explain why the existing functionality isn't sufficient - if the header is really not from this project, then it should be included via `-isystem`, and that will naturally suppress all diagnostics from it. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D104099: [NewPM] Remove SpeculateAroundPHIs pass from pipeline

lebedev.ri created this revision. lebedev.ri added reviewers: thopre, bjope, nikic, chandlerc, aeubanks. lebedev.ri added a project: LLVM. Herald added subscribers: ormris, wenlei, steven_wu, hiraditya. lebedev.ri requested review of this revision. Herald added a project: clang. Herald added a subs

[PATCH] D104099: [NewPM] Remove SpeculateAroundPHIs pass from pipeline

lebedev.ri added inline comments. Comment at: llvm/lib/Passes/PassBuilder.cpp:1449 - // inserting redundancies into the program. This even includes SimplifyCFG. - OptimizePM.addPass(SpeculateAroundPHIsPass()); - nikic wrote: > As it has been in-tree for a while

[PATCH] D104099: [NewPM] Remove SpeculateAroundPHIs pass from pipeline

lebedev.ri updated this revision to Diff 351444. lebedev.ri added a comment. Herald added a subscriber: zzheng. Update a few more tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104099/new/ https://reviews.llvm.org/D104099 Files: clang/test

[PATCH] D104099: [NewPM] Remove SpeculateAroundPHIs pass from pipeline

lebedev.ri updated this revision to Diff 351445. lebedev.ri added a comment. ... and upload the right patch this time. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104099/new/ https://reviews.llvm.org/D104099 Files: clang/test/CodeGen/thinlto-d

[PATCH] D104099: [NewPM] Remove SpeculateAroundPHIs pass from pipeline

lebedev.ri added a comment. In D104099#2813713 , @aeubanks wrote: > Some backends don't run SimplifyCFG, e.g. X86. I believe the pass was > originally created specifically for X86 (the header has some X86 examples) > and may or may not extend to other t

[PATCH] D104099: [NewPM] Remove SpeculateAroundPHIs pass from pipeline

lebedev.ri added a comment. In D104099#2813743 , @aeubanks wrote: > Which pass that comes after SpeculateAroundPHIs in the X86 pipeline (either > in the optimization or codegen) would undo its effects? As i have wrote in some other review, the pass i sa

[PATCH] D104258: [OPENMP]Fix PR50699: capture locals in combine directrives for aligned clause.

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

[PATCH] D104099: [NewPM] Remove SpeculateAroundPHIs pass from pipeline

lebedev.ri added a subscriber: MaskRay. lebedev.ri added a comment. In D104099#2815531 , @wenlei wrote: > In D104099#2814167 , @davidxl wrote: > >> Adding Wei to help measure performance impact on our internal wor

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

lebedev.ri added a comment. The test is still intermittently failing on bots. Is there some non-determinism in the change? It may be a good idea to revert this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101868/new/ https://reviews.llvm.org/D101

[PATCH] D104099: [NewPM] Remove SpeculateAroundPHIs pass

lebedev.ri updated this revision to Diff 352176. lebedev.ri retitled this revision from "[NewPM] Remove SpeculateAroundPHIs pass from pipeline" to "[NewPM] Remove SpeculateAroundPHIs pass". lebedev.ri added a comment. Herald added a subscriber: mgorny. On Tue, Jun 15, 2021 at 8:14 PM Wei Mi wrot

[PATCH] D104099: [NewPM] Remove SpeculateAroundPHIs pass

lebedev.ri added a comment. I see. Going to land this now. Thanks for looking! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104099/new/ https://reviews.llvm.org/D104099 ___ cfe-commits mailing list cfe-

[PATCH] D104099: [NewPM] Remove SpeculateAroundPHIs pass

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe52364532afb: [NewPM] Remove SpeculateAroundPHIs pass (authored by lebedev.ri). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

[PATCH] D102107: [OpenMP] Codegen aggregate for outlined function captures

lebedev.ri added a comment. (This is not offload-specific, right?) This does not bring any compatibility issues, right? Does this bring any compatibility issues? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102107/new/ https://reviews.llvm.org/D10

[PATCH] D104500: DRAFT: [clang] Apply P1825 as DR for all modes below C++20.

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

[PATCH] D107420: [sema] Disallow __builtin_mul_overflow under special condition.

lebedev.ri added a comment. I don't personally care, but i think this diag doesn't make sense. What is "backend"? Which one? All of them? What happens when one, but not all of them supports it? What if i don't intend to codegen this into an assembly, but only want to produce the IR? Repository

[PATCH] D108138: [SimplifyCFG] Remove switch statements before vectorization

lebedev.ri added a comment. I'm not sure i'm sold on this, even though i'm aware that selects hurt vectorization. How does this Simplify the CFG? I think it would be best to teach LV selects, or at worst do this in LV itself. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

[PATCH] D108138: [SimplifyCFG] Remove switch statements before vectorization

lebedev.ri added a comment. The alternative to teaching looprotate/LV about switches is to make swiches non-canonical in the first half of the pipeline, before LV. That is, don't form them, and aggressively expand any and all existing switches. Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D104797: [WebAssembly] Implementation of global.get/set for reftypes in LLVM IR

lebedev.ri reopened this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. Reverted again FAIL: LLVM :: CodeGen/WebAssembly/funcref-call.ll (44466 of 44468) TEST 'LLVM :: CodeGen/WebAssembly/funcref-call.ll' FAI

[PATCH] D105269: [X86] AVX512FP16 instructions enabling 6/6

lebedev.ri added inline comments. Comment at: llvm/lib/Target/X86/X86ScheduleZnver3.td:64 - let CompleteModel = 1; + let CompleteModel = 0; } RKSimon wrote: > You could avoid this change if you add a scheduler class to whatever > instruction is complaining?

[PATCH] D105439: [clang] protects users from relying on libc++ detail headers

lebedev.ri added a comment. Why not solve this in the headers themselves? Just `#error` in implementation header if some macro isn't defined, and define said macro in parent header before including implementation detail headers? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

lebedev.ri created this revision. lebedev.ri added reviewers: rjmccall, erichkeane, rsmith, thakis, jdoerfert. lebedev.ri added a project: LLVM. Herald added a subscriber: jfb. lebedev.ri requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. We h

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

lebedev.ri updated this revision to Diff 357648. lebedev.ri marked 2 inline comments as done. lebedev.ri edited the summary of this revision. lebedev.ri added a comment. In D105728#2868281 , @erichkeane wrote: > This seems logical to me. The comment nee

[PATCH] D105439: [clang] protects users from relying on libc++ detail headers

lebedev.ri added a comment. In D105439#2874706 , @ldionne wrote: > I'm not entirely sure I understand the purpose of this patch. So the idea is > that let's say a tool suggests including `<__algorithm/find.h>` to get the > definition of `std::find` as a

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

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

[PATCH] D112720: [clang-tidy] Use globs in HeaderFilter

lebedev.ri added a comment. Not in favor of general directions like this. Does it not break the existing uses (existing `.clang-tidy`'s e.g.) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112720/new/ https://reviews.llvm.org/D112720 ___ cfe-c

[PATCH] D112720: [clang-tidy] Use globs in HeaderFilter

lebedev.ri requested changes to this revision. lebedev.ri added a comment. This revision now requires changes to proceed. As long as this blankedly breaks/regresses existing configs it's a non-starter i think. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112720/new/ https://reviews.ll

[PATCH] D112720: [clang-tidy] Use globs in HeaderFilter

lebedev.ri added a comment. Assuming this is the change we want, i would imagine the viable enablement path is to introduce some option "filters are regexes and not globs", that defaults to `false`, and use it. Then some time later (2 releases?), flip the default and remove said option. My main

[PATCH] D112926: run-clang-tidy.py analyze unique files only

lebedev.ri added a comment. In D112926#3108745 , @kuhar wrote: > I think this change is fine. > One could argue that having multiple files is an issue with tooling/build > system in the first place, No, why would that be a bug in itself? It may be a

[PATCH] D113211: [NFC][clang] Inclusive terms: replace uses of blacklist in clang/test/

lebedev.ri resigned from this revision. lebedev.ri added a comment. I will not be part of this language policing. -10 to any changes that break existing external interface (that is, it is not okay to just rename externally-visible command-line flags, config options, etc.) Repository: rG LLVM

[PATCH] D97512: [clang] removes check against integral-to-pointer conversion...

lebedev.ri added a comment. Patch is missing a description of the problem. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97512/new/ https://reviews.llvm.org/D97512 ___ cfe-commits mailing list cfe-commit

[PATCH] D97577: [clang-tidy] performance-for-range-copy: Don't trigger on implicit type conversions.

lebedev.ri added a comment. It is best not to change existing tests, but add new ones. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97577/new/ https://reviews.llvm.org/D97577 ___ cfe-commits mailing lis

[PATCH] D97577: [clang-tidy] performance-for-range-copy: Don't trigger on implicit type conversions.

lebedev.ri added a comment. In D97577#2596458 , @hokein wrote: > In D97577#2592827 , @flx wrote: > >> In D97577#2592093 , @lebedev.ri >> wrote: >> >>> It is best not to chan

[PATCH] D96281: [clang-tidy] Add options to flag individual core increments and to ignore macros to readability-function-cognitive-complexity check.

lebedev.ri added a comment. Once again, can we please split this into two patches? I'm quite happy with the `DescribeBasicIncrements` stuff, that can be merged right now. But i'm not quite sold on the macro stuff. We don't have that for readability-function-size, nor does the CC spec say anythi

[PATCH] D96281: [clang-tidy] Add options to describe individual core increments to readability-function-cognitive-complexity check.

lebedev.ri added a comment. Does the test start to fail if you undo the .cpp change? You may want to add some ` // CHECK-NOTES-NOT: warning` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96281/new/ https://reviews.llvm.org/D96281

[PATCH] D96281: [clang-tidy] Add options to describe individual core increments to readability-function-cognitive-complexity check.

lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. In D96281#2599812 , @lebedev.ri wrote: > Does the test start to fail if you undo the .cpp change? > You may want to add some ` // CHECK-NOTES-N

[PATCH] D96281: [clang-tidy] Add options to describe individual core increments to readability-function-cognitive-complexity check.

lebedev.ri added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/readability-function-cognitive-complexity-flags.cpp:16 + // CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'func_of_complexity_4' has cognitive complexity of 4 (threshold 0) [readability-

[PATCH] D96281: [clang-tidy] Add options to describe individual core increments to readability-function-cognitive-complexity check.

lebedev.ri added a comment. In D96281#2599951 , @lebedev.ri wrote: > If you need help committing this, please specify `name ` to be > used for git commit author field. Comment at: clang-tools-extra/test/clang-tidy/checkers/readabili

[PATCH] D94640: adds more checks to -Wfree-nonheap-object

lebedev.ri added a comment. > <...> Is the patch still not reverted? That should have happened almost a week ago. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94640/new/ https://reviews.llvm.org/D94640 ___

[PATCH] D97116: Reduce the number of attributes attached to each function

lebedev.ri added a comment. In D97116#2610652 , @crownyanguan wrote: > In D97116#2596409 , @xbolva00 wrote: > >> In D97116#2596275 , @crownyanguan >> wrote: >> >>> However,

[PATCH] D98070: [clang-tidy] Add option to ignore macros in readability-function-cognitive-complexity check.

lebedev.ri added a comment. Please also add a test with global `IgnoreMacros=1` and `readability-function-cognitive-complexity.IgnoreMacros` unset. (The code is correct as-is, global `IgnoreMacros` should not affect the check here.) I'm also somewhat worried about forward compatibility. If in f

[PATCH] D97563: [clang-tidy] Enable modernize-concat-nested-namespaces also on headers

lebedev.ri added a comment. I think the implicit question is: won't this regress headers that are meant to be compatible with earlier standards? Did the original review mention anything about this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D9756

[PATCH] D94355: [Passes] Add relative lookup table converter pass

lebedev.ri added inline comments. Comment at: llvm/include/llvm/CodeGen/BasicTTIImpl.h:390-391 + +if (!TM.getTargetTriple().isArch64Bit()) + return false; + 1. But all tests are using `x86_64` triple? 2. This is somewhat backwards. if the target wants to

[PATCH] D114025: [clang][NFC] Inclusive terms: replace some uses of sanity in clang

lebedev.ri added a comment. In D114025#3140161 , @martong wrote: > Do we have a comprehensive list of non-inclusive terms and their inclusive > correspondent somewhere available? > I mean `master` -> `main`, `white list` -> `inclusive list`, `sanity` ->

[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. Were you able to actually reproduce the problem that lead to revert of D107799 , or is this based on blind guesses? In D110663#3149364 , @MaskRay wrote: > In D110663#3148690

[PATCH] D114317: [clang-tidy][WIP] Do not run perfect alias checks

lebedev.ri added a comment. Thank you for looking into it! In D114317#3149693 , @carlosgalvezp wrote: > In D114317#3149504 , > @salman-javed-nz wrote: > >> What would you say are the key differences between thi

[PATCH] D114317: [clang-tidy][WIP] Do not run perfect alias checks

lebedev.ri added a comment. In D114317#3150615 , @carlosgalvezp wrote: >> it is likely that the options were tuned, but most likely only for a single >> check. > > Yes, if the alias check has a different configuration than the primary check > then they

[PATCH] D114533: LLVM IR should allow bitcast between address spaces with the same size.

lebedev.ri added a comment. Is there an RFC? This quite seems like asking for problems. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114533/new/ https://reviews.llvm.org/D114533 ___ cfe-commits mailing

[PATCH] D114533: LLVM IR should allow bitcast between address spaces with the same size.

lebedev.ri added a comment. In D114533#3151423 , @arsenm wrote: > Patch description should include this avoids a need to introduce > ptrtoint/inttoptr pairs That is a good point, but all the motivational cases seem to be about chaining the address spac

[PATCH] D114579: [clang-tidy] Exempt _MSVC_EXECUTION_CHARACTER_SET from cppcoreguidelines-macro-usage

lebedev.ri resigned from this revision. lebedev.ri added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp:40 const MacroDirective *MD) override { if (SM.isWrittenInBuiltinFile(MD->getLocation()) ||

[PATCH] D136156: [Clang][Diagnostic] Add hidden-reinterpret-cast diagnostic warning

lebedev.ri added a comment. Looks like this and more is already being caught by clang-tidy's google-readability-casting check . CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136156/new/ https://reviews.llvm.org/D136156 _

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

lebedev.ri added inline comments. Comment at: clang/lib/Driver/Driver.cpp:4531-4548 +// Get linking executable file's parent path as TracePath's parent path, +// default is ".". Filename may be determined and added into TracePath then. +// +// e.g. executable fil

[PATCH] D140281: [X86] Rename CMPCCXADD intrinsics.

lebedev.ri added a comment. This patch says what it does, but not why it does what it does. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140281/new/ https://reviews.llvm.org/D140281 ___ cfe-commits mail

[PATCH] D140281: [X86] Rename CMPCCXADD intrinsics.

lebedev.ri added a comment. In D140281#4006650 , @FreddyYe wrote: > In D140281#4004707 , @lebedev.ri > wrote: > >> This patch says what it does, but not why it does what it does. > > Sorry for not mention. This p

[PATCH] D140467: [X86][Reduce] Preserve fast math flags when change it. NFCI

lebedev.ri requested changes to this revision. lebedev.ri added a comment. In D140467#4010675 , @pengfei wrote: >> If it exists it must be tested. >> Every piece of code generation needs to be tested. > > Let me show you the number: > > $ grep -rho '__b

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

lebedev.ri added a comment. As someone who has no idea what this does, this seems to be missing docs. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136651/new/ https://reviews.llvm.org/D136651 ___ cfe-co

[PATCH] D138958: [clang] Better UX for Clang’s unwind-affecting attributes

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

[PATCH] D137381: [clang][compiler-rt] Exception escape out of an non-unwinding function is an undefined behaviour

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

[PATCH] D138958: [clang] Better UX for Clang’s unwind-affecting attributes

lebedev.ri updated this revision to Diff 487548. lebedev.ri marked 2 inline comments as done. lebedev.ri added a comment. In D138958#4037526 , @erichkeane wrote: > 2 quick nits, otherwise LFTM. @erichkeane thank you for the review! @aaron.ballman would

[PATCH] D137381: [clang][compiler-rt] Exception escape out of an non-unwinding function is an undefined behaviour

lebedev.ri added a comment. In D137381#4038716 , @MaskRay wrote: > I am unfamiliar with Clang codegen so I am just commenting about what a user > may feel about this feature. > > `compiler-rt/test/ubsan/TestCases/Misc/exception-escape.cpp` cannot > demo

[PATCH] D138958: [clang] Better UX for Clang’s unwind-affecting attributes

lebedev.ri added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:565 +returning a value. They can not write to memory, but may read memory that is +immutable between invocations of the function. + erichkeane wrote: > A quick sentence somewhere

[PATCH] D138958: [clang] Better UX for Clang’s unwind-affecting attributes

lebedev.ri added inline comments. Comment at: clang/include/clang/Basic/Attr.td:4133 +def NoUnwind : DeclOrStmtAttr { + let Spellings = [Clang<"nounwind", /*allowInC =*/0>]; + let Accessors = [Accessor<"isClangNoUnwind", [CXX11<"clang", "nounwind">]>]; aaron.ba

[PATCH] D138958: [clang] Better UX for Clang’s unwind-affecting attributes

lebedev.ri updated this revision to Diff 487933. lebedev.ri marked 10 inline comments as done. lebedev.ri added a comment. @aaron.ballman thank you for taking a look! Addressed all review notes other than the SEH question and the simplehandler one. (i'll deal with whitespace changes when committi

[PATCH] D138958: [clang] Better UX for Clang’s unwind-affecting attributes

lebedev.ri marked 2 inline comments as done. lebedev.ri added inline comments. Comment at: clang/include/clang/Basic/Attr.td:4138 + "functions and statements">; + let SimpleHandler = 1; +} aaron.ballman wrote: > lebedev.ri wrote: > >

[PATCH] D138958: [clang] Better UX for Clang’s unwind-affecting attributes

lebedev.ri marked 2 inline comments as done. lebedev.ri added inline comments. Comment at: clang/include/clang/Basic/Attr.td:4138 + "functions and statements">; + let SimpleHandler = 1; +} aaron.ballman wrote: > lebedev.ri wrote: > >

[PATCH] D137381: [clang][compiler-rt] Exception escape out of an non-unwinding function is an undefined behaviour

lebedev.ri created this revision. lebedev.ri added reviewers: rjmccall, morehouse, aaron.ballman, dvyukov, MaskRay, vsk, Sanitizers. lebedev.ri added a project: LLVM. Herald added subscribers: Enna1, StephenFan, dberris. Herald added a project: All. lebedev.ri requested review of this revision. He

[PATCH] D137381: [clang][compiler-rt] Exception escape out of an non-unwinding function is an undefined behaviour

lebedev.ri added a comment. (also, this needs a bit more work around irgen, will look in a bit.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137381/new/ https://reviews.llvm.org/D137381 ___ cfe-commits

[PATCH] D137381: [clang][compiler-rt] Exception escape out of an non-unwinding function is an undefined behaviour

lebedev.ri added a comment. In D137381#3907104 , @MaskRay wrote: > In your example, `clang++ a.cc; ./a.out` gives a libstdc++ error: > > terminate called after throwing an instance of 'int' > > libc++'s is similar. That's great, but just a symptom of m

[PATCH] D137381: [clang][compiler-rt] Exception escape out of an non-unwinding function is an undefined behaviour

lebedev.ri updated this revision to Diff 473529. lebedev.ri edited the summary of this revision. lebedev.ri added a comment. Herald added a subscriber: mstorsjo. In D137381#3907799 , @lebedev.ri wrote: > In D137381#3907104

[PATCH] D137901: [Clang] `nothrow`-implying attributes should actually manifest `nothrow` attribute (PR58798)

lebedev.ri created this revision. lebedev.ri added reviewers: aaron.ballman, erichkeane, rjmccall. lebedev.ri added a project: LLVM. Herald added a project: All. lebedev.ri requested review of this revision. Herald added a reviewer: jdoerfert. Herald added a subscriber: sstefan1. Herald added a pro

[PATCH] D137381: [clang][compiler-rt] Exception escape out of an non-unwinding function is an undefined behaviour

lebedev.ri added subscribers: vitalybuka, glider, rsmith. lebedev.ri added a comment. Can someone from @Sanitizers please comment? :) @vitalybuka ? @glider ? @rsmith ? In D137381#3923911 , @MaskRay wrote: > I think improving diagnostic is useful but `-

[PATCH] D136806: [Pipelines] Introduce SROA after (final, full) loop unrolling

lebedev.ri updated this revision to Diff 475311. lebedev.ri retitled this revision from "[Pipelines] Introduce SROA after (full) loop unrolling" to "[Pipelines] Introduce SROA after (final, full) loop unrolling". lebedev.ri added a reviewer: arsenm. lebedev.ri added a comment. Herald added subscr

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