[PATCH] D57280: [clang][OpenMP] OMPFlushClause is synthetic, no such clause exists

lebedev.ri added inline comments. Comment at: lib/Basic/OpenMPKinds.cpp:56 #define OPENMP_CLAUSE(Name, Class) .Case(#Name, OMPC_##Name) + OPENMP_CLAUSE(flush, OMPFlushClause) #include "clang/Basic/OpenMPKinds.def" ABataev wrote: > Just `.Case("flush", OMPC

[PATCH] D57280: [clang][OpenMP] OMPFlushClause is synthetic, no such clause exists

lebedev.ri marked an inline comment as done. lebedev.ri added inline comments. Comment at: lib/Basic/OpenMPKinds.cpp:56 #define OPENMP_CLAUSE(Name, Class) .Case(#Name, OMPC_##Name) + OPENMP_CLAUSE(flush, OMPFlushClause) #include "clang/Basic/OpenMPKinds.def" --

[PATCH] D57112: [ASTTypeTraits] OMPClause handling

lebedev.ri updated this revision to Diff 183902. lebedev.ri marked an inline comment as done. lebedev.ri added a comment. - Rebased - Go back to storing the actual AST class type as string in `ASTNodeKind::KindInfo ASTNodeKind::AllKindInfo[]`. - Add `OpenMPClauseKind ASTNodeKind::getOMPClauseKind

[PATCH] D57113: [clang-tidy] openmp-use-default-none - a new module and a check

lebedev.ri updated this revision to Diff 183903. lebedev.ri added a comment. NFC. - Rebased - Switched to `OpenMPClauseKind ASTNodeKind::getOMPClauseKind()`. This is better anyway, since it avoids string comparisons, and is a simple switch over an enum. Repository: rCTE Clang Tools Extra C

[PATCH] D44823: [libcxx] Improving std::vector and std::deque perfomance

lebedev.ri added a comment. In D44823#1375654 , @mclow.lists wrote: > In D44823#1375590 , @mclow.lists > wrote: > > > I just tried this (on Compiler Explorer) using LLVM 7, and the code for my > > original test in

[PATCH] D57435: [clang-tidy] created wrap-unique check

lebedev.ri added a comment. `.DS_Store` files clearly should not be there. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57435/new/ https://reviews.llvm.org/D57435 ___ cfe-commits mailing list cfe-commi

[PATCH] D57452: [ASTDumper][OpenMP] CapturedDecl has a 'nothrow' bit

lebedev.ri created this revision. lebedev.ri added reviewers: aaron.ballman, steveire. lebedev.ri added projects: OpenMP, clang. Herald added a subscriber: guansong. Was trying to understand how complicated it would be to write a clang-tidy `openmp-exception-escape`-ish check once D57100

[PATCH] D57452: [ASTDumper][OpenMP] CapturedDecl has a 'nothrow' bit

lebedev.ri added a comment. In D57452#1377140 , @ABataev wrote: > LG Thank you for the review. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57452/new/ https://reviews.llvm.org/D57452

[PATCH] D57419: [ASTDump] Move Decl node dumping to TextNodeDumper

lebedev.ri added a comment. Needs rebasing for rL352631 . Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57419/new/ https://reviews.llvm.org/D57419 ___ cfe-commits mailing list cfe-c

[PATCH] D57100: [clang-tidy] refactor bugprone-exception-escape analysis into class

lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. Looked this over once more. To me this looks pretty straight-forward. The caching seems ok, too. As long as `ExceptionAnalyzer` does not outlive it's `TU`, everything should be fine. @b

[PATCH] D57108: [clang-tidy] diagnose possibiltiy to add 'noexcept' in modernize-use-noexcept

lebedev.ri added a comment. Please add tests with lambdas, Comment at: test/clang-tidy/modernize-use-noexcept-new-noexcept.cpp:5-6 + +void undefined(); +void undefinedNoexcept() noexcept; + Unused functions. I suspect if the first one called, we can't be optimi

[PATCH] D53935: Delete dependency on config.h

lebedev.ri added a comment. How does this play with `LLVM_ENABLE_THREADS` cmake option? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53935 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailm

[PATCH] D53949: [clang][CodeGen] Implicit Conversion Sanitizer: discover the world of CompoundAssign operators

lebedev.ri created this revision. lebedev.ri added reviewers: rsmith, regehr, vsk, rjmccall, Sanitizers. lebedev.ri added projects: clang, Sanitizers. Not yet for an //actual// review, needs tests. As reported by @regehr (thanks!) on twitter (https://twitter.com/johnregehr/status/105768149625581

[PATCH] D53949: [clang][CodeGen] Implicit Conversion Sanitizer: discover the world of CompoundAssign operators

lebedev.ri added a comment. In https://reviews.llvm.org/D53949#1282861, @regehr wrote: > I can test this and write a few test cases. I'll write the tests tomorrow, i just wanted to post the initial code diff. (it is a shame that there isn't any working analog of llvm/utils/update_test_checks.p

[PATCH] D53949: [clang][CodeGen] Implicit Conversion Sanitizer: discover the world of CompoundAssign operators

lebedev.ri added a comment. In https://reviews.llvm.org/D53949#1282866, @regehr wrote: > This patch doesn't appear to yet fix the "x++" or "x--" cases. It won't, the increment/decrement happens on the original type, there is no cast there. https://godbolt.org/z/WuWA62 Those cases are for norma

[PATCH] D53949: [clang][CodeGen] Implicit Conversion Sanitizer: discover the world of CompoundAssign operators

lebedev.ri added a comment. In https://reviews.llvm.org/D53949#1282870, @lebedev.ri wrote: > In https://reviews.llvm.org/D53949#1282866, @regehr wrote: > > > This patch doesn't appear to yet fix the "x++" or "x--" cases. > > > It won't, the increment/decrement happens on the original type, there

[PATCH] D53949: [clang][CodeGen] Implicit Conversion Sanitizer: discover the world of CompoundAssign operators

lebedev.ri added a comment. In https://reviews.llvm.org/D53949#1282884, @regehr wrote: > I do not agree that ++ is performed on the original type. I was only talking about the IR. In https://reviews.llvm.org/D53949#1282884, @regehr wrote: > The C99 standard (6.5.3.1.2) appears to be very clea

[PATCH] D53949: [clang][CodeGen] Implicit Conversion Sanitizer: discover the world of CompoundAssign operators

lebedev.ri added a comment. In https://reviews.llvm.org/D53949#1282884, @regehr wrote: > I do not agree that ++ is performed on the original type. The C99 standard > (6.5.3.1.2) appears to be very clear on this point: "The expression ++E is > equivalent to (E+=1)." In https://reviews.llvm.o

[PATCH] D53936: [clang-tidy] More clearly separate public, check-facing APIs from internal ones.

lebedev.ri added inline comments. Comment at: clang-tidy/ClangTidy.h:11 +// +// It should remain as stable as possible, as many out-of-tree checks exist. +//===--===// steveire wrote: > alexfh wr

[PATCH] D53949: [clang][CodeGen] Implicit Conversion Sanitizer: discover the world of CompoundAssign operators

lebedev.ri updated this revision to Diff 172115. lebedev.ri edited the summary of this revision. lebedev.ri added a subscriber: mclow.lists. lebedev.ri added a comment. Added test coverage. Please review :) Repository: rC Clang https://reviews.llvm.org/D53949 Files: lib/CodeGen/CGExprScala

[PATCH] D53949: [clang][CodeGen] Implicit Conversion Sanitizer: discover the world of CompoundAssign operators

lebedev.ri added a comment. In https://reviews.llvm.org/D53949#1283872, @regehr wrote: > Regarding ++ and --, I think for now it's fine to just document that these > aren't instrumented. Indeed, that is a different issue, i don't want to solve it in this differential. Though, there currently

[PATCH] D53949: [clang][CodeGen] Implicit Conversion Sanitizer: discover the world of CompoundAssign operators

lebedev.ri updated this revision to Diff 172335. lebedev.ri added a comment. And finish the test coverage. (sidenote: will these follow-ups again be stuck for two months? let's see!) Repository: rC Clang https://reviews.llvm.org/D53949 Files: lib/CodeGen/CGExprScalar.cpp test/CodeGen/ca

[PATCH] D33672: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker

lebedev.ri added inline comments. Comment at: lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:96 + // Get the value of the expression to cast. + const auto ValueToCastOptional = + C.getSVal(CE->getSubExpr()).getAs(); ZaMaZaN4iK wrote: > aaron.bal

[PATCH] D33672: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker

lebedev.ri added inline comments. Comment at: lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:96 + // Get the value of the expression to cast. + const auto ValueToCastOptional = + C.getSVal(CE->getSubExpr()).getAs(); ZaMaZaN4iK wrote: > lebedev.r

[PATCH] D33672: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker

lebedev.ri added inline comments. Comment at: lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:96 + // Get the value of the expression to cast. + const auto ValueToCastOptional = + C.getSVal(CE->getSubExpr()).getAs(); ZaMaZaN4iK wrote: > lebedev.r

[PATCH] D53771: [clang-tidy] Avoid C arrays check

lebedev.ri marked 4 inline comments as done. lebedev.ri added a comment. (only comments, patch to follow) Comment at: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp:84 "cppcoreguidelines-c-copy-assignment-signature"); +CheckFactories.registerCheck( +

[PATCH] D53771: [clang-tidy] Avoid C arrays check

lebedev.ri updated this revision to Diff 172520. lebedev.ri marked 6 inline comments as done. lebedev.ri added a comment. Better diag for VLA arrays. https://reviews.llvm.org/D53771 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.c

[PATCH] D53771: [clang-tidy] Avoid C arrays check

lebedev.ri added inline comments. Comment at: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp:84 "cppcoreguidelines-c-copy-assignment-signature"); +CheckFactories.registerCheck( +"cppcoreguidelines-avoid-c-arrays"); lebedev.ri wr

[PATCH] D53488: [clang-tidy] Catching narrowing from double to float.

lebedev.ri added inline comments. Comment at: test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp:164 + while (i) { +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: narrowing conversion from 'int' to 'bool' [cppcoreguidelines-narrowing-conversions] + } ---

[PATCH] D50050: [AST] CastExpr: BasePathSize is not large enough.

lebedev.ri added a comment. In https://reviews.llvm.org/D50050#1287780, @george.karpenkov wrote: > @lebedev.ri @erichkeane The test fails for me on macOS whenever asan and > ubsan are both enabled. > The failure is stack overflow at stack frame 943 > (? maybe asan usage enforces lower stack

[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable

lebedev.ri added inline comments. Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:20 + +static const char LoopName[] = "forLoopName"; +static const char loopVarName[] = "loopVar"; Szelethus wrote: > JonasToth wrote: > > JonasToth wrote: > > > ztamas

[PATCH] D52421: [Sema] Diagnose parameter names that shadow inherited field names

lebedev.ri added a comment. And now i'm having concerns. struct Base { void* f; }; struct Inherit : Base { static void func(void* f) { // <- does 'f' *actually* shadow the 'f' in the 'Base'? You can't access that non-static member variable from static function. } };

[PATCH] D52421: [Sema] Diagnose parameter names that shadow inherited field names

lebedev.ri added a comment. In https://reviews.llvm.org/D52421#1288917, @aaron.ballman wrote: > In https://reviews.llvm.org/D52421#1288910, @lebedev.ri wrote: > > > ... > > > ... Ok, thank you for reassuring me that it is working as intended. https://reviews.llvm.org/D52421 ___

[PATCH] D52695: [clang][Parse] Diagnose useless null statements (PR39111)

lebedev.ri added a comment. ping. I wonder if @aaron.ballman has any opinion on this. Repository: rC Clang https://reviews.llvm.org/D52695 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-

[PATCH] D54262: [clang-tidy] Add `delete this` bugprone check (PR38741)

lebedev.ri added a comment. It's best to use `clang-tidy/add_new_check.py` tool. You also need tests, and a note in releasenotes. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54262 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-pp'

lebedev.ri added a comment. No one will know for sure what "pp" in "readability-redundant-pp" means. I'd highly recommend to fully spell it out. Also, i'd like to see some analysis of the false-positives. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54349 _

[PATCH] D54344: [Clang][CodeGen][CXX]: Workaround __attribute((no_destroy)) crash/incorrect code generation.

lebedev.ri added inline comments. Comment at: test/SemaCXX/attr-no-destroy-d54344.cpp:93 +static some_class s_ctor(1); \ No newline at end of file Please add new line. Repository: rC Clang https://reviews.llvm.org/D54344 _

[PATCH] D53771: [clang-tidy] Avoid C arrays check

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

[PATCH] D53771: [clang-tidy] Avoid C arrays check

lebedev.ri added inline comments. Comment at: clang-tidy/modernize/AvoidCArraysCheck.cpp:64 + diag(ArrayType->getBeginLoc(), + isa(ArrayType->getTypePtr()) ? UseVector : UseArray); +} JonasToth wrote: > Why `isa<>` and not `isVariableArrayType()` (does it

[PATCH] D53771: [clang-tidy] Avoid C arrays check

lebedev.ri updated this revision to Diff 173514. lebedev.ri marked 2 inline comments as done. lebedev.ri added a comment. Use `isVariableArrayType()` Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53771 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcoregu

[PATCH] D53771: [clang-tidy] Avoid C arrays check

lebedev.ri added inline comments. Comment at: test/clang-tidy/modernize-avoid-c-arrays.cpp:32 + +using k = int[4]; +// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not declare C-style arrays, use std::array<> instead JonasToth wrote: > What happens for a variabl

[PATCH] D53771: [clang-tidy] Avoid C arrays check

lebedev.ri added a comment. *Maybe* we should be actually diagnosing the each actual declaration, not just the `typeloc`. Else if you use one `typedef`, and `// NOLINT` it, you will silence it for all the decls that use it. In https://reviews.llvm.org/D53771#1294241, @JonasToth wrote: > LGTM.

[PATCH] D53949: [clang][CodeGen] Implicit Conversion Sanitizer: discover the world of CompoundAssign operators

lebedev.ri added a comment. Ping. Repository: rC Clang https://reviews.llvm.org/D53949 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54473: [sanitizers] Initial implementation for -fsanitize=init-locals

lebedev.ri added inline comments. Comment at: include/clang/Basic/Sanitizers.def:163 +// Initialize local variables. +SANITIZER("init-locals", InitLocals) + Unless i'm mistaken, I suspect you may see some surprising behavior here, unfortunately. [[ https://bugs.

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

lebedev.ri added a comment. Are you sure `check-clang` passes with this patch? It looks like some bots weren't happy before the revert. (Also, is this warning enabled when building llvm? If yes, you might want to double-check stage2 build.) Repository: rC Clang https://reviews.llvm.org/D5283

[PATCH] D54565: Introduce `-Wc++14-compat-ctad` as a subgroup of `-Wc++14-compat`

lebedev.ri added a comment. Looks about right, but needs tests. Repository: rC Clang https://reviews.llvm.org/D54565 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54589: [clang][UBSan] Sanitization for alignment assumptions.

lebedev.ri created this revision. lebedev.ri added reviewers: ABataev, craig.topper, vsk, rsmith, rnk, Sanitizers, erichkeane, filcab, rjmccall. lebedev.ri added a project: Sanitizers. lebedev.ri added a dependency: D54588: [llvm][IRBuilder] Introspection for CreateAlignmentAssumption*() function

[PATCH] D54604: Automatic variable initialization

lebedev.ri added a comment. There is also https://reviews.llvm.org/D54473 `[sanitizers] Initial implementation for -fsanitize=init-locals`. Repository: rC Clang https://reviews.llvm.org/D54604 ___ cfe-commits mailing list cfe-commits@lists.llvm.

[PATCH] D54589: [clang][UBSan] Sanitization for alignment assumptions.

lebedev.ri updated this revision to Diff 174451. lebedev.ri added a comment. Adapt to https://reviews.llvm.org/D54588 no longer providing the actual pointer. The handled itself will have to deal with subtracting the offset. Repository: rC Clang https://reviews.llvm.org/D54589 Files: docs/R

[PATCH] D54654: Correct CreateAlignmentAssumption to check sign and power-of-2 (Clang Part)

lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. LG. I hope that extra is-power-of-two won't confuse the optimizer. Repository: rC Clang https://reviews.llvm.org/D54654 ___ cfe-commit

[PATCH] D53949: [clang][CodeGen] Implicit Conversion Sanitizer: discover the world of CompoundAssign operators

lebedev.ri added a comment. In https://reviews.llvm.org/D53949#1296232, @rjmccall wrote: > Seems reasonable to me. I'm very happy that it seems reasonable. I'm waiting for either review comments, or formal approval :) Ping. Repository: rC Clang https://reviews.llvm.org/D53949 _

[PATCH] D52695: [clang][Parse] Diagnose useless null statements (PR39111)

lebedev.ri added a comment. Ping. Is even the `-Wempty-init-stmt` is not interesting? Repository: rC Clang https://reviews.llvm.org/D52695 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-

[PATCH] D52695: [clang][Parse] Diagnose useless null statements (PR39111)

lebedev.ri updated this revision to Diff 174624. lebedev.ri marked 12 inline comments as done. lebedev.ri added a comment. Address @aaron.ballman review notes. Repository: rC Clang https://reviews.llvm.org/D52695 Files: docs/ReleaseNotes.rst include/clang/Basic/DiagnosticGroups.td incl

[PATCH] D52695: [clang][Parse] Diagnose useless null statements (PR39111)

lebedev.ri added inline comments. Comment at: test/Parser/extra-semi-resulting-in-nullstmt.cpp:81 + } +} aaron.ballman wrote: > There are four additional tests I'd like to see: 1) a semicolon at global > scope, 2) a semicolon after a namespace declaration (e.g.

[PATCH] D53949: [clang][CodeGen] Implicit Conversion Sanitizer: discover the world of CompoundAssign operators

lebedev.ri added a comment. In https://reviews.llvm.org/D53949#1303154, @rjmccall wrote: > Heh, okay. Okay, great, thank you for the review! Repository: rC Clang https://reviews.llvm.org/D53949 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D52695: [clang][Parse] Diagnose useless null statements (PR39111)

lebedev.ri added a comment. In https://reviews.llvm.org/D52695#1304225, @aaron.ballman wrote: > Aside from some small nits, LGTM! Thank you for the review! Will address nits and land. Repository: rC Clang https://reviews.llvm.org/D52695 ___ cf

[PATCH] D54757: [clang-tidy] new check: bugprone-branch-clone

lebedev.ri added inline comments. Comment at: clang-tidy/bugprone/BranchCloneCheck.cpp:43 + + for (unsigned i = 0; i < LHS.size(); i++) { +if (!areStatementsIdentical(LHS[i]->stripLabelLikeStatements(), Eugene.Zelenko wrote: > Please use size_t. Also ``` for

[PATCH] D48866: [clang-tidy] Add incorrect-pointer-cast checker

lebedev.ri added a comment. `Warn for cases when the pointed to type is wider than the allocated type.` What does that mean? Is this talking about the size of the allocation, or the alignment? Are you aware of the previous attempts, namely https://reviews.llvm.org/D33826? Please incorporate the

[PATCH] D54589: [clang][UBSan] Sanitization for alignment assumptions.

lebedev.ri added a comment. ping Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54589/new/ https://reviews.llvm.org/D54589 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/

[PATCH] D54589: [clang][UBSan] Sanitization for alignment assumptions.

lebedev.ri added inline comments. Comment at: docs/UndefinedBehaviorSanitizer.rst:198 +assume-aligned-like attributes), `object-size``, and ``vptr`` checks do not +apply to pointers to types with the ``volatile`` qualifier rjmccall wrote: > Is there a reason fo

[PATCH] D54589: [clang][UBSan] Sanitization for alignment assumptions.

lebedev.ri marked an inline comment as done. lebedev.ri added inline comments. Comment at: docs/UndefinedBehaviorSanitizer.rst:198 +assume-aligned-like attributes), `object-size``, and ``vptr`` checks do not +apply to pointers to types with the ``volatile`` qualifier --

[PATCH] D54589: [clang][UBSan] Sanitization for alignment assumptions.

lebedev.ri marked an inline comment as not done. lebedev.ri added inline comments. Comment at: docs/UndefinedBehaviorSanitizer.rst:198 +assume-aligned-like attributes), `object-size``, and ``vptr`` checks do not +apply to pointers to types with the ``volatile`` qualifier --

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

lebedev.ri added a comment. Looks reasonable to me (or at least all these tests make this look good :)), but i'm not sure i can fully review this. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54395/new/ https://reviews.llvm.org/D54395 _

[PATCH] D54589: [clang][UBSan] Sanitization for alignment assumptions.

lebedev.ri updated this revision to Diff 175407. lebedev.ri marked 11 inline comments as done. lebedev.ri added a comment. Address @rjmccall review notes. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54589/new/ https://reviews.llvm.org/D54589 Files: docs/Rele

[PATCH] D54589: [clang][UBSan] Sanitization for alignment assumptions.

lebedev.ri updated this revision to Diff 175494. lebedev.ri marked 4 inline comments as done. lebedev.ri added a comment. Address @rjmccall review notes. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54589/new/ https://reviews.llvm.org/D54589 Files: docs/Relea

[PATCH] D54589: [clang][UBSan] Sanitization for alignment assumptions.

lebedev.ri added a comment. In D54589#1309929 , @rjmccall wrote: > LGTM. Thank you for the review! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54589/new/ https://reviews.llvm.org/D54589

[PATCH] D54986: Make CodeGen choose when to emit vtables.

lebedev.ri added a comment. Will `-fforce-emit-vtables` still work? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54986/new/ https://reviews.llvm.org/D54986 ___ cfe-commits mailing list cfe-commits@lists.llvm.org htt

[PATCH] D55044: [cfe-dev] clang-tidy check for Abseil make_unique

lebedev.ri added a comment. Thanks for working on this. Semi-duplicate of D50852 Please see discussion there. It should not be an abseil-specific check, the prefix (`std`,`abseil`), and the function (`make_unique`) should be a config option, and it should likely

[PATCH] D54995: [MemoryBuffer] By default assume that all files are volatile to prevent unintended file locks

lebedev.ri added a comment. Passing-by thought, feel free to ignore: this seems to so far only affect windows only? So the fix shouldn't probably pessimize all other arches? (and maybe even non-clangd) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54995/new/ https://reviews.llvm.org/D

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

lebedev.ri requested changes to this revision. lebedev.ri added a comment. This revision now requires changes to proceed. Marking both of these as "changes requested" to highlight the similarity of issues in them. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews

[PATCH] D50852: [clang-tidy] abseil-auto-make-unique

lebedev.ri requested changes to this revision. lebedev.ri added a comment. This revision now requires changes to proceed. Marking both of these as "changes requested" to highlight the similarity of issues in them. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews

[PATCH] D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap

lebedev.ri added inline comments. Comment at: lib/Support/MemoryBuffer.cpp:42 +static bool MemoryMappingEnabled = true; + Such global flags are a bad idea in general, and really not great in LLVM's case. The editor would set it for "it's" llvm, but that will a

[PATCH] D55321: Do not check for parameters shadowing fields in function declarations

lebedev.ri added inline comments. Comment at: test/SemaCXX/warn-shadow.cpp:236 } + void F(int B); // Ok, declaration; not definition. }; Can you please also add one function with out-of-line definition? CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D55245: [clang-tidy] Add the abseil-duration-subtraction check

lebedev.ri added a comment. Please always upload all patches with the full context (`-U9`) Comment at: clang-tidy/abseil/DurationRewriter.cpp:35 +getInverseForScale(DurationScale Scale) { + static const std::unordered_map> https://llvm.org/docs/Programmers

[PATCH] D55428: [Docs] Expand -fstack-protector and -fstack-protector-all info

lebedev.ri added a comment. Please linewrap to 80 col. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55428/new/ https://reviews.llvm.org/D55428 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llv

[PATCH] D55410: [clang-tidy] check for flagging using declarations in headers

lebedev.ri added inline comments. Comment at: clang-tidy/abseil/AliasFreeHeadersCheck.cpp:22 + // Match all using declarations in header files. + Finder->addMatcher(usingDecl(isExpansionInFileMatching(".*\\.h.*")).bind("x"), +this); Relevant: ``` $ clang-t

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

lebedev.ri added a comment. Have you evaluated this on some major C++ projects yet? I suspect this will have pretty low SNR. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55433/new/ https://reviews.llvm.org/D55433

[PATCH] D34796: upporting -f(no)-reorder-functions flag, clang side change

lebedev.ri added inline comments. Comment at: docs/ClangCommandLineReference.rst:1953 +.. option:: -freorder-functions, -fno-reorder-functions + Isn't this autogenerated from `include/clang/Driver/Options.td`? Repository: rC Clang CHANGES SINCE LAST ACTION

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

lebedev.ri added a comment. In D55433#1323145 , @MyDeveloperDay wrote: > In D55433#1323106 , @lebedev.ri > wrote: > > > Have you evaluated this on some major C++ projects yet? > > I suspect this may have pretty l

[PATCH] D55245: [clang-tidy] Add the abseil-duration-subtraction check

lebedev.ri added inline comments. Comment at: clang-tidy/abseil/DurationRewriter.cpp:35 +getInverseForScale(DurationScale Scale) { + static const std::unordered_map> hwright wrote: > lebedev.ri wrote: > > https://llvm.org/docs/ProgrammersManual.html#other-map-li

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

lebedev.ri added inline comments. Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:28-30 +bool isOperator(const FunctionDecl *D) { + return D->getNameAsString().find("operator") != std::string::npos; +} Can't you use `isOverloadedOperator()`? No way going

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

lebedev.ri added a comment. In D55433#1323757 , @MyDeveloperDay wrote: > a lot of projects aren't setup for c++17 yet which is needed for > [[nodiscard]] to be allowed, You can use `[[clang::warn_unused_result]]` for this evaluation, that does not req

[PATCH] D54450: Get the correct range of tokens for preprocessor conditions

lebedev.ri added a comment. This seems to be correct, but the test coverage could be improved. There are no tests with `()` e.g. Comment at: lib/Lex/PPExpressions.cpp:154-156 // Consume the ). -Result.setEnd(PeekTok.getLocation()); PP.LexNonComment(PeekTok); +

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

lebedev.ri added a comment. In D55433#1323779 , @lebedev.ri wrote: > In D55433#1323757 , @MyDeveloperDay > wrote: > > > a lot of projects aren't setup for c++17 yet which is needed for > > [[nodiscard]] to be allo

[PATCH] D55245: [clang-tidy] Add the abseil-duration-subtraction check

lebedev.ri added inline comments. Comment at: clang-tidy/abseil/DurationRewriter.cpp:20-39 +struct DurationScale2IndexFunctor { + using argument_type = DurationScale; + unsigned operator()(DurationScale Scale) const { +switch (Scale) { +case DurationScale::Hours: +

[PATCH] D56160: [clang-tidy] modernize-use-trailing-return check

lebedev.ri added a comment. I just noticed, i don't like the naming choice in the check. `UseTrailingReturn` what is the "trailing return"? This is about "trailing return *type*", no? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56160/new/ https://reviews.llvm.org/D56160 ___

[PATCH] D56160: [clang-tidy] modernize-use-trailing-return-type check

lebedev.ri added a comment. In D56160#1412701 , @bernhardmgruber wrote: > - renamed the check to modernize-use-trailing-return-type Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56160/new/ https://reviews.llvm.org/D56160 __

[PATCH] D58743: Handle built-in when importing SourceLocation and FileID

lebedev.ri added a comment. Is there a test missing here? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58743/new/ https://reviews.llvm.org/D58743 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailma

[PATCH] D58764: [clang-tidy] ignore predefined expressions in cppcoreguidelines-pro-bounds-array-to-pointer-decay check

lebedev.ri requested changes to this revision. lebedev.ri added a comment. This revision now requires changes to proceed. See D22196 . Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58764/new/ https://reviews.llvm.org

[PATCH] D58782: Use ArrayRef::copy, instead of copying data manually

lebedev.ri added a comment. Would it please be possible to write actual commit / DR titles, i.e. include appropriate `[tag]`s into them, and ideally proper commit messages, too? It really clutters -commit list listing otherwise. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D57112: [ASTTypeTraits] OMPClause handling

lebedev.ri added a comment. ping @klimek @hokein thanks. The fun of of the code that is so well separated that no one touches it for years and thus can't review patches for it :) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57112/new/ https://reviews.llvm.org/D5

[PATCH] D58811: [clang] Fix misuses of char width to char align

lebedev.ri added a comment. Can you be more specific? Is there some bugreport this fixes? Is there some test? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58811/new/ https://reviews.llvm.org/D58811 ___ cfe-commits m

[PATCH] D58811: [clang] Fix misuses of char width to char align

lebedev.ri added a comment. In D58811#1414784 , @amalykh wrote: > Currently getCharWidth and getCharAlign functions are hard coded to return 8 > in clang with no way to change it for target. > This can be seen in TargetInfo.h file: > > unsigned getChar

[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check

lebedev.ri added a comment. +1 i was just going to comment - this needs **much** more tests. Also, it would be really great if you could supply the differential's description.T Comment at: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.cpp:25 + const auto MutexDecl = type(has

[PATCH] D58841: [Diagnostics] Support -Wtype-limits for GCC compatibility

lebedev.ri requested changes to this revision. lebedev.ri added a comment. This revision now requires changes to proceed. See D41512 , rC322901 , D51545 . Comment at: clang/inc

[PATCH] D58841: [Diagnostics] Support -Wtype-limits for GCC compatibility

lebedev.ri added a reviewer: thakis. lebedev.ri resigned from this revision. lebedev.ri added inline comments. Comment at: clang/include/clang/Basic/DiagnosticGroups.td:485 +// For compatibility with GCC; -Wtype-limits = -Wtautological-constant-in-range-compare +def TypeLimits :

[PATCH] D58841: [Diagnostics] Support -Wtype-limits for GCC compatibility

lebedev.ri added inline comments. Comment at: clang/include/clang/Basic/DiagnosticGroups.td:485 +// For compatibility with GCC; -Wtype-limits = -Wtautological-constant-in-range-compare +def TypeLimits : DiagGroup<"type-limits", [TautologicalInRangeCompare]>; def TautologicalOut

[PATCH] D58894: [analyzer] Handle modification of vars inside an expr with comma operator

lebedev.ri added a comment. Thank you for working on this! Is there any way to model this more generically? I.e don't duplicate every naive matcher (ignoring possible presence of `,` op) with an variant that does not ignore `,`. E.g. will this handle `(a,b)+=1` ? What about `(a,b).callNonConstMe

[PATCH] D57112: [ASTTypeTraits] OMPClause handling

lebedev.ri marked 6 inline comments as not done. lebedev.ri added inline comments. Comment at: lib/AST/ASTTypeTraits.cpp:114 +#define OPENMP_CLAUSE(Name, Class) \ +case OMPC_##Name: return ASTNodeKind(NKI_##Class); +#include "clang

[PATCH] D58811: [clang] Fix misuses of char width to char align

lebedev.ri requested changes to this revision. lebedev.ri added a comment. This revision now requires changes to proceed. In D58811#1417959 , @amalykh wrote: > Thank you for your comments, Roman. > Despite the fact, that there are no official LLVM targets

<    1   2   3   4   5   6   7   8   9   10   >