[PATCH] D46281: [clang-doc] Attaching a name to reference data

lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. I can't really comment on the BC change since i don't think anything here makes use of that yet, but the code changes look ok to me. LGTM. Comment at: clang-doc/Bitc

[PATCH] D46323: [compiler-rt][X86][AMD][Bulldozer] Fix Bulldozer Model 2 detection.

lebedev.ri updated this revision to Diff 144749. lebedev.ri marked an inline comment as done. lebedev.ri added a comment. Update comment, too. Repository: rL LLVM https://reviews.llvm.org/D46323 Files: lib/builtins/cpu_model.c Index: lib/builtins/cpu_model.c =

[PATCH] D46323: [compiler-rt][X86][AMD][Bulldozer] Fix Bulldozer Model 2 detection.

lebedev.ri added a comment. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85591 Repository: rL LLVM https://reviews.llvm.org/D46323 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45931: [ASTMatchers] Don't garble the profiling output when multiple TU's are processed

lebedev.ri added a comment. In https://reviews.llvm.org/D45931#1083192, @lebedev.ri wrote: > Thank you for looking at this. > > In https://reviews.llvm.org/D45931#1083184, @alexfh wrote: > > > From a user's perspective I'd probably prefer a different behavior of > > checks profiling with multipl

[PATCH] D46281: [clang-doc] Attaching a name to reference data

lebedev.ri added inline comments. Comment at: clang-doc/BitcodeWriter.cpp:382 + emitRecord(R.USR, REFERENCE_USR); + emitRecord(R.Name, REFERENCE_NAME); + emitRecord((unsigned)R.RefType, REFERENCE_TYPE); lebedev.ri wrote: > juliehockett wrote: > > lebedev.ri wr

[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers

lebedev.ri added a comment. In https://reviews.llvm.org/D46159#1085400, @pfultz2 wrote: > Do you want the flag to be renamed? I personally would //like// a bit better name, but i'm not sure what would be better, so not a requirement. Do you have some better choices? Repository: rCTE Clang

[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers

lebedev.ri added a comment. In https://reviews.llvm.org/D46159#1085684, @dcoughlin wrote: > In https://reviews.llvm.org/D46159#1085548, @pfultz2 wrote: > > > > Do you have some better choices? > > > > I could do `-allow-alpha-checks`. What do you think? > > > That seems reasonable to me. > Al

[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers

lebedev.ri added a comment. In https://reviews.llvm.org/D46159#1086322, @alexfh wrote: > How about `-enable-alpha-checks=yes-i-know-they-are-broken` ? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46159 ___ cfe-commits mailing l

[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers

lebedev.ri added a comment. In https://reviews.llvm.org/D46159#1086463, @pfultz2 wrote: > > As Devin says (and as we discussed this with Anna Zaks) alpha checkers are > > still in development, so we don't want to expose them to the users, even > > very curious ones. > > Then why do we make them

[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers

lebedev.ri added inline comments. Comment at: clang-tidy/tool/ClangTidyMain.cpp:195 +/// This option Enables alpha checkers from the static analyzer, that are +/// experimental. This option is set to false and not visible in help, because xbolva00 wrote: > This

[PATCH] D45931: [ASTMatchers] Don't garble the profiling output when multiple TU's are processed

lebedev.ri added a comment. In https://reviews.llvm.org/D45931#1086665, @alexfh wrote: > In https://reviews.llvm.org/D45931#1084503, @lebedev.ri wrote: > > > In https://reviews.llvm.org/D45931#1083192, @lebedev.ri wrote: > > > > > Thank you for looking at this. > > > > > > In https://reviews.llvm

[PATCH] D46403: [CFI] Force LLVM to die if the implicit blacklist files cannot be found.

lebedev.ri added a comment. Also, please upload the correct patch, from the root of the repo, not from the directory of the file. Repository: rC Clang https://reviews.llvm.org/D46403 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:/

[PATCH] D46403: [CFI] Force LLVM to die if the implicit blacklist files cannot be found.

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

[PATCH] D45931: [ASTMatchers] Don't garble the profiling output when multiple TU's are processed

lebedev.ri added a comment. In https://reviews.llvm.org/D45931#1086665, @alexfh wrote: > In https://reviews.llvm.org/D45931#1084503, @lebedev.ri wrote: > > > In https://reviews.llvm.org/D45931#1083192, @lebedev.ri wrote: > > > > > Thank you for looking at this. > > > > > > In https://reviews.llvm

[PATCH] D46439: Fix incorrect packed aligned structure layout

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

[PATCH] D46441: [clang][CodeGenCXX] Noalias attr for copy/move constructor arguments

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

[PATCH] D46155: Add warning flag -Wordered-compare-function-pointers.

lebedev.ri added a comment. Ideally there should be a test that verifies that `-Wordered-compare-function-pointers` / `-Wno-ordered-compare-function-pointers` / the default is what you expect it to be. Repository: rC Clang https://reviews.llvm.org/D46155

[PATCH] D46187: [Analyzer] getRegisteredCheckers(): handle debug checkers too.

lebedev.ri added a comment. In https://reviews.llvm.org/D46187#1088722, @NoQ wrote: > It seems that you're using debug checkers of the analyzer to gain some free > tools for exploring the source code (such as displaying a call graph) for > free, right? Yes. I was interested to dump the callgr

[PATCH] D46504: [clang-tidy] Profile is a per-AST (per-TU) data.

lebedev.ri created this revision. lebedev.ri added reviewers: alexfh, sbenza. lebedev.ri added a project: clang-tools-extra. Herald added subscribers: mgrang, xazax.hun, mgorny. As discussed in https://reviews.llvm.org/D45931, currently, profiling output of clang-tidy is somewhat not great. It ou

[PATCH] D46504: [clang-tidy] Profile is a per-AST (per-TU) data.

lebedev.ri added inline comments. Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:181 std::unique_ptr OptionsProvider) -: DiagEngine(nullptr), OptionsProvider(std::move(OptionsProvider)), - Profile(nullptr) { +: DiagEngine(nullptr), OptionsProvider(std::m

[PATCH] D33537: [clang-tidy] Exception Escape Checker

lebedev.ri added inline comments. Comment at: test/clang-tidy/bugprone-exception-escape.cpp:178 +void indirect_implicit() noexcept { + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'indirect_implicit' throws + implicit_int_thrower(); baloghadamsoftware

[PATCH] D46188: [clang-tidy] Add a flag to enable debug checkers

lebedev.ri added subscribers: george.karpenkov, NoQ, dcoughlin. lebedev.ri added a comment. In https://reviews.llvm.org/D46188#1091221, @alexfh wrote: > I don't think debug CSA checkers belong to clang-tidy. If one needs to dump > CFG, they are probably doing quite involved stuff already, so goi

[PATCH] D46504: [clang-tidy] Profile is a per-AST (per-TU) data.

lebedev.ri added a comment. In https://reviews.llvm.org/D46504#1091210, @alexfh wrote: > LG with a couple of nits. Thank you for the review, faster than ever :) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46504 ___ cfe-commits

[PATCH] D46951: [clang-tidy] misc-unused-parameters - retain old behavior under StrictMode

lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. I would like to see some test in non-strict mode that shows that it is the child existence is what matters, e.g. with empty lines, and maybe another one with all the content commented ou

[PATCH] D48721: Patch to fix pragma metadata for do-while loops

lebedev.ri added a comment. Test? (Or was this meant to contain `[Private]` in title?) Repository: rC Clang https://reviews.llvm.org/D48721 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe

[PATCH] D48721: Patch to fix pragma metadata for do-while loops

lebedev.ri added a comment. In https://reviews.llvm.org/D48721#1146662, @deepak2427 wrote: > It's a patch for a bug in clang. > I have requested for a Bugzilla account, however thought of putting up the > patch in the meantime. > Do I need to mark it '[Private]'? Phab is the correct way to s

[PATCH] D48721: Patch to fix pragma metadata for do-while loops

lebedev.ri added a comment. In https://reviews.llvm.org/D48721#1146681, @deepak2427 wrote: > > Phab is the correct way to submit patches. > > But having a bugreport in bugzilla is good too. > > But the test will be needed regardless of the patch submission method. > > And yes, please do always

[PATCH] D48721: Patch to fix pragma metadata for do-while loops

lebedev.ri added a comment. I'm not sure we can use `-O` in tests at all, and i'm not sure it is even needed here since you are only testing codegen. Repository: rC Clang https://reviews.llvm.org/D48721 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D45179: [libc++] Add _LIBCPP_FORCE_NODISCARD define to force-enable nodiscard in pre-C++17

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

[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning

lebedev.ri added a comment. Some drive-by nits. General observation: this is kinda large. I would //personally// split split off the codegen part into a second patch. Comment at: include/clang/AST/Decl.h:2212-2213 + bool isCPUDispatchMultiVersion() const; + bool isCPUSpecif

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

lebedev.ri updated this revision to Diff 154490. lebedev.ri added a comment. Rebased, just to control bitrot, no changes. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D36892 Files: test/clang-tidy/check_clang_tidy.py Index: test/clang-tidy/check_clang_tidy.py =

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

lebedev.ri updated this revision to Diff 154491. lebedev.ri added a comment. Rebased, just to control bitrot, no changes. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D36836 Files: LICENSE.TXT clang-tidy/CMakeLists.txt clang-tidy/plugin/CMakeLists.txt clang-tidy/sonars

[PATCH] D48958: [clang][ubsan] Implicit Cast Sanitizer - integer truncation - clang part

lebedev.ri updated this revision to Diff 154591. lebedev.ri retitled this revision from "[private][clang] Implicit Cast Sanitizer - integer truncation - clang part" to "[clang][ubsan] Implicit Cast Sanitizer - integer truncation - clang part". lebedev.ri edited the summary of this revision. leb

[PATCH] D48958: [clang][ubsan] Implicit Cast Sanitizer - integer truncation - clang part

lebedev.ri added a comment. Finished running it on a normal testset of my pet project . - It fired ~18 times. - There were no obvious false-positives (e.g. when an explicit cast was involved). - At least 3 of those appear to be a true bugs. - 4-5 more

[PATCH] D48958: [clang][ubsan] Implicit Cast Sanitizer - integer truncation - clang part

lebedev.ri updated this revision to Diff 154863. lebedev.ri added a comment. - Check that sanitizer is actually enabled before doing the AST upwalk. I didn't measure, but it would be logical for this to be better. Repository: rC Clang https://reviews.llvm.org/D48958 Files: docs/ReleaseNot

[PATCH] D48958: [clang][ubsan] Implicit Cast Sanitizer - integer truncation - clang part

lebedev.ri updated this revision to Diff 154877. lebedev.ri marked 2 inline comments as done. lebedev.ri added a comment. @vsk thank you for taking a look! Addressed the trivial part of nits. Repository: rC Clang https://reviews.llvm.org/D48958 Files: docs/ReleaseNotes.rst docs/Undefine

[PATCH] D48958: [clang][ubsan] Implicit Cast Sanitizer - integer truncation - clang part

lebedev.ri added inline comments. Comment at: lib/CodeGen/CGExprScalar.cpp:318 - Value *EmitScalarConversion(Value *Src, QualType SrcTy, QualType DstTy, + Value *EmitScalarConversion(Value *Src, QualType SrcType, QualType DstType, SourceLocation

[PATCH] D48958: [clang][ubsan] Implicit Cast Sanitizer - integer truncation - clang part

lebedev.ri added inline comments. Comment at: lib/CodeGen/CGExprScalar.cpp:1694 // handle things like function to ptr-to-function decay etc. Value *ScalarExprEmitter::VisitCastExpr(CastExpr *CE) { Expr *E = CE->getSubExpr(); vsk wrote: > lebedev.ri wrote: >

[PATCH] D48958: [clang][ubsan] Implicit Cast Sanitizer - integer truncation - clang part

lebedev.ri updated this revision to Diff 155064. lebedev.ri added a comment. Add some more tricky tests where maintaining just the `CastExpr` part of AST stack would break them. Repository: rC Clang https://reviews.llvm.org/D48958 Files: docs/ReleaseNotes.rst docs/UndefinedBehaviorSanit

[PATCH] D48958: [clang][ubsan] Implicit Cast Sanitizer - integer truncation - clang part

lebedev.ri updated this revision to Diff 155149. lebedev.ri marked 7 inline comments as done. lebedev.ri added a comment. Address @vsk's review notes. - Maintain the stack of currently-being-visited `CastExpr`'s - Use that stack to check whether we are in a `ExplicitCastExpr` - Move logic for dec

[PATCH] D48958: [clang][ubsan] Implicit Cast Sanitizer - integer truncation - clang part

lebedev.ri added inline comments. Comment at: lib/CodeGen/CGExprScalar.cpp:1694 // handle things like function to ptr-to-function decay etc. Value *ScalarExprEmitter::VisitCastExpr(CastExpr *CE) { Expr *E = CE->getSubExpr(); vsk wrote: > lebedev.ri wrote: >

[PATCH] D48958: [clang][ubsan] Implicit Cast Sanitizer - integer truncation - clang part

lebedev.ri added a comment. In https://reviews.llvm.org/D48958#1160479, @vsk wrote: > In https://reviews.llvm.org/D48958#1160435, @lebedev.ri wrote: > > > Thank you for taking a look! > > > > In https://reviews.llvm.org/D48958#1160381, @vsk wrote: > > > > > I have some minor comments but overall

[PATCH] D48958: [clang][ubsan] Implicit Cast Sanitizer - integer truncation - clang part

lebedev.ri added a comment. Thank you for taking a look! In https://reviews.llvm.org/D48958#1160381, @vsk wrote: > I have some minor comments but overall I think this is in good shape. It > would be great to see some compile-time numbers just to make sure this is > tractable. I'm pretty sure -

[PATCH] D49268: [clang-doc] Create a script to generate tests

lebedev.ri added a comment. Nice! Some comments. Sorry about lack of the review, this kinda fell off my radar. Did you meant to commit `clang-tools-extra/clang-doc/test_cases/` ? I can't tell whether it is a temporary directory or not. Comment at: clang-tools-extra/clang-doc/g

[PATCH] D48958: [clang][ubsan] Implicit Cast Sanitizer - integer truncation - clang part

lebedev.ri added a comment. In https://reviews.llvm.org/D48958#1160848, @vsk wrote: > In https://reviews.llvm.org/D48958#1160494, @lebedev.ri wrote: > > > In https://reviews.llvm.org/D48958#1160479, @vsk wrote: > > > > > In https://reviews.llvm.org/D48958#1160435, @lebedev.ri wrote: > > > > > > >

[PATCH] D48958: [clang][ubsan] Implicit Cast Sanitizer - integer truncation - clang part

lebedev.ri added a comment. In https://reviews.llvm.org/D48958#1160853, @lebedev.ri wrote: > In https://reviews.llvm.org/D48958#1160848, @vsk wrote: > > > <...> > > The stage2 build traps before it finishes: > > > > FAILED: lib/IR/AttributesCompatFunc.inc.tmp > > cd > > /Users/vsk/src/build

[PATCH] D48958: [clang][ubsan] Implicit Cast Sanitizer - integer truncation - clang part

lebedev.ri marked 5 inline comments as done. lebedev.ri added a comment. @vsk so yeah, no wonder that doesn't work. Somehow in that test case `ScalarExprEmitter::VisitExplicitCastExpr()` **never** gets called. (I'm pretty sure this worked with the naive implementation, so worst case i'll just re

[PATCH] D48958: [clang][ubsan] Implicit Cast Sanitizer - integer truncation - clang part

lebedev.ri updated this revision to Diff 155363. lebedev.ri marked 3 inline comments as done. lebedev.ri added a comment. Address @vsk review notes, although this will be revered by the next update dropping the faulty 'stack' optimization. Repository: rC Clang https://reviews.llvm.org/D48958

[PATCH] D48958: [clang][ubsan] Implicit Cast Sanitizer - integer truncation - clang part

lebedev.ri added a comment. Well, that's just great, with `isCastPartOfExplictCast()`, the `ASTContext::getParents()` also does not return `CXXStaticCastExpr` as parent for such cases. I don't know how to proceed. Repository: rC Clang https://reviews.llvm.org/D48958 __

[PATCH] D49268: [clang-doc] Create a script to generate tests

lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. I have no further comments here. I assume this works as intended; if not, since it is mainly a developer-only tool, further issues could be addressed later on. Maybe wait a bit just in

[PATCH] D45444: [clang-tidy] implement new check for const-correctness

lebedev.ri added a comment. Ping, i guess :) Anything needed to get this going? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45444 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listi

[PATCH] D49348: Harden/relax clang/test/CodeGen/opt-record-MIR.c test

lebedev.ri created this revision. lebedev.ri added reviewers: anemet, aaron.ballman, hfinkel. Herald added a subscriber: cfe-commits. If the build path is short, `Line` field can end up fitting on the same line as `File`, but the `{{.*}}` would consume it. Keeping in mind https://reviews.llvm.or

[PATCH] D49348: Harden/relax clang/test/CodeGen/opt-record-MIR.c test

lebedev.ri updated this revision to Diff 155591. lebedev.ri added a comment. Simplify regex even more. Repository: rC Clang https://reviews.llvm.org/D49348 Files: test/CodeGen/opt-record-MIR.c Index: test/CodeGen/opt-record-MIR.c ==

[PATCH] D49348: Harden/relax clang/test/CodeGen/opt-record-MIR.c test

lebedev.ri added a comment. In https://reviews.llvm.org/D49348#1164579, @anemet wrote: > LGTM, thanks! Thank you for the review. (I'll watch the bots, obviously) Repository: rC Clang https://reviews.llvm.org/D49348 ___ cfe-commits mailing list

[PATCH] D49508: [CodeGen] VisitMaterializeTemporaryExpr(): don't skip NoOp Casts.

lebedev.ri created this revision. lebedev.ri added reviewers: rsmith, rjmccall, majnemer, efriedma. Herald added a subscriber: cfe-commits. As discussed in PR38166 , we need to be able to distinqush whether the cast we are visiting is actually a cast,

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

lebedev.ri added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:23 + +const char DefaultIgnoredValues[] = "0;1;2;10;100;"; + aaron.ballman wrote: > Why 2, 10, and 100? These really should be a config option. Repository: rCTE Clang T

[PATCH] D49508: [CodeGen] VisitMaterializeTemporaryExpr(): don't skip NoOp Casts.

lebedev.ri added a comment. In https://reviews.llvm.org/D49508#1167177, @efriedma wrote: > skipRValueSubobjectAdjustments has to match the rules in `[class.temporary]` > in the standard, which includes skipping over certain explicit casts. I'm this approach because this is what @rsmith suggest

[PATCH] D49508: [Sema] Expr::skipRValueSubobjectAdjustments(): record skipped NoOp casts.

lebedev.ri updated this revision to Diff 156235. lebedev.ri retitled this revision from "[CodeGen] VisitMaterializeTemporaryExpr(): don't skip NoOp Casts." to "[Sema] Expr::skipRValueSubobjectAdjustments(): record skipped NoOp casts.". lebedev.ri edited the summary of this revision. lebedev.ri ad

[PATCH] D68526: [Diagnostics] Silence -Wsizeof-array-div for character buffers

lebedev.ri added a comment. There's not a single word in the patch's description. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68526/new/ https://reviews.llvm.org/D68526 ___ cfe-commits mailing list c

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

lebedev.ri added a comment. Ping. Friendly remainder that the unsanitized UB is still being miscompiled. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67122/new/ https://reviews.llvm.org/D67122 ___ cfe

[PATCH] D68569: [clang-format] Also look for .{ext}.clang-format file

lebedev.ri added a comment. I'm not seeing any merrit this brings over existing https://clang.llvm.org/docs/ClangFormatStyleOptions.html `Language` option. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68569/new/ https://reviews.llvm.org/D68569

[PATCH] D68569: [clang-format] Also look for .{ext}.clang-format file

lebedev.ri added a comment. In D68569#1697248 , @wanders wrote: > The "Language" option can not distinguish between C and C++. > > We have projects which contains both C and C++ code. Using different style > (yes..) for C and C++. Our C++ headers are nam

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

lebedev.ri added inline comments. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:4657 + Builder.GetInsertBlock()->getParent(), PtrTy->getPointerAddressSpace()); + // Check for overflows unless the GEP got constant-folded, + // and only in the default address space

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

lebedev.ri added a comment. In D67122#1703482 , @russell.gallop wrote: > This is failing the sanitizer lint check: > > http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/23819/steps/64-bit%20check-sanitizer/logs/stdio > > /compiler-rt/lib/u

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

lebedev.ri added a comment. Not unexpectedly, this broke sanitizer-x86_64-linux-bootstrap-ubsan. I'm going to attempt to address uncovered issues Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67122/new/ https://reviews.llvm.org/D67122 __

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

lebedev.ri added a comment. http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap-ubsan/builds/15329/steps/annotate/logs/stdio appears to contain "only" 3 distinct issues. I've pushed those 3 fixes, but maybe that is not enough, we'll see. Repository: rG LLVM Github Monorepo CHA

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

lebedev.ri added a comment. In D67122#1703616 , @lebedev.ri wrote: > http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap-ubsan/builds/15329/steps/annotate/logs/stdio > appears to contain "only" 3 distinct issues. > I've pushed those 3 fixe

[PATCH] D68824: Fix __builtin_assume_aligned with too large values.

lebedev.ri added a comment. Thanks, i like it. Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:2058-2073 +/* void CodeGenFunction::EmitAlignmentAssumption(llvm::Value *PtrValue, QualType Ty, SourceLocation Loc,

[PATCH] D68824: Fix __builtin_assume_aligned with too large values.

lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. LG, thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68824/new/ https://reviews.llvm.org/D68824 ___ cfe-commits mailing lis

[PATCH] D68807: [ClangTidy] Separate tests for infrastructure and checkers

lebedev.ri added a comment. That's a lot of moved files. How about *only* moving the infrastructure tests? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68807/new/ https://reviews.llvm.org/D68807 ___ c

[PATCH] D68807: [ClangTidy] Separate tests for infrastructure and checkers

lebedev.ri added a comment. Description says: > making it easier to find infrastructure tests > Tests for infrastructure were difficult to find because they were > outnumbered by tests for checkers. So presumably there far fewer infra tests as compared to checker tests, and the issue is specif

[PATCH] D68694: [clang-tidy] hicpp-signed-bitwise: Do not show "use of a signed integer operand with a binary bitwise operator" for positive integer operands

lebedev.ri added inline comments. Comment at: clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.cpp:23-24 +: ClangTidyCheck(Name, Context), + IgnorePositiveIntegerLiterals( + Options.getLocalOrGlobal("IgnorePositiveIntegerLiterals", false)) {} +

[PATCH] D68694: [clang-tidy] hicpp-signed-bitwise: Do not show "use of a signed integer operand with a binary bitwise operator" for positive integer operands

lebedev.ri added inline comments. Comment at: clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.cpp:23-24 +: ClangTidyCheck(Name, Context), + IgnorePositiveIntegerLiterals( + Options.getLocalOrGlobal("IgnorePositiveIntegerLiterals", false)) {} +

[PATCH] D68410: [AttrDocs] document always_inline

lebedev.ri requested changes to this revision. lebedev.ri added a comment. This revision now requires changes to proceed. I think partial (but not wrong!) docs is better than no docs whatsoever, so i'd be inclined to proceed with this. I, too, not really convinced that actual explicit rules shoul

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

lebedev.ri requested changes to this revision. lebedev.ri added a comment. This revision now requires changes to proceed. (outstanding unaddressed review notes) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67545/new/ https://reviews.llvm.org/D6754

[PATCH] D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros

lebedev.ri added a comment. The description only says what the patch does, not why this is the desired behavior. Also, this is now under a wrong license. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D31130/new/ https://reviews.llvm.org/D31130

[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

lebedev.ri added a comment. So https://godbolt.org/z/qzjU-C This feels like gcc being overly zealous, i'm not sure what it says with that warning. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69145/new/ https://reviews.llvm.org/D69145 _

[PATCH] D69218: [clang][AST] Add `CXXBaseSpecifier` matcher support

lebedev.ri added a comment. Missing docs regeneration (`clang/docs/tools/dump_ast_matchers.py` i think) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69218/new/ https://reviews.llvm.org/D69218 ___ cfe-

[PATCH] D69218: [clang][AST] Add `CXXBaseSpecifier` matcher support

lebedev.ri added a comment. Needs more tests - what happens with multiple inheritance, what about inheriting from inherited? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69218/new/ https://reviews.llvm.org/D69218 __

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

lebedev.ri added inline comments. Comment at: clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp:22 + +bool CheckPotentiallyTriviallyDestructible(const CXXDestructorDecl *Dtor) { + if (Dtor->isFirstDecl() || !Dtor->isExplicitlyDefaulted())

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

lebedev.ri added inline comments. Comment at: clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp:22 + +bool CheckPotentiallyTriviallyDestructible(const CXXDestructorDecl *Dtor) { + if (Dtor->isFirstDecl() || !Dtor->isExplicitlyDefaulted())

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

lebedev.ri added a comment. This should likely be at least 3 patches: llvm middle-end, llvm codegen, clang. Langref changes missing for new intrinsics. Please post all patches with full context (`-U9`) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

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

lebedev.ri resigned from this revision. lebedev.ri added a comment. In D80344#2051697 , @tentzen wrote: > In D80344#2051127 , @lebedev.ri > wrote: > > > This should likely be at least 3 patches: llvm middle-end, ll

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

lebedev.ri added a comment. In D80344#2051804 , @tentzen wrote: > > It may be helpful (even for the reviewers) to first specify their behavior, > > instead of writing that after-the-fact "backwardly" based on the > > implementation. > > For reviewers, th

[PATCH] D80941: [PowerPC][Power10] Implement Count Leading/Trailing Zeroes Builtins in LLVM/Clang

lebedev.ri added a comment. Why not lower it to `@llvm.cttz(and(a, b))`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80941/new/ https://reviews.llvm.org/D80941 ___ cfe-commits mailing list cfe-commit

[PATCH] D80947: Add to the Coding Standard our that single-line bodies omit braces

lebedev.ri added a comment. SGTM Comment at: llvm/docs/CodingStandards.rst:1573 +Don't Use Braces on Simple Single-Statement Bodies of if/else/loop Statements +^ `case` too Reposito

[PATCH] D83360: [InstSimplify] Remove select ?, undef, X -> X and select ?, X, undef -> X

lebedev.ri added a comment. In D83360#2154540 , @echristo wrote: > This seems like something that we should then revert until we know that > instsimplify can be updated with a fix? Possibility for a miscompile sounds much worse to me than a false-positi

[PATCH] D83360: [InstSimplify] Remove select ?, undef, X -> X and select ?, X, undef -> X

lebedev.ri added a comment. In D83360#2154545 , @echristo wrote: > We're starting to see miscompiles as we do more testing as well, just nothing > smaller at the moment. Could you clarify, do you mean that this fix is causing (new) miscompiles? Reposi

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

lebedev.ri added inline comments. Comment at: llvm/cmake/modules/AddPerfetto.cmake:9 + ExternalProject_Add(perfetto_git + GIT_REPOSITORY https://github.com/google/perfetto.git + GIT_TAG releases/v4.x nickdesaulniers wrote: > lebedev.ri wrote: > > I hav

[PATCH] D84176: [CMake][CTE] Add "check-clang-tools-..." targets to test only a particular Clang extra tool

lebedev.ri added a comment. Thank you, this would be very helpful. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84176/new/ https://reviews.llvm.org/D84176 ___ cfe-commits mailing list cfe-commits@list

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

lebedev.ri added inline comments. Comment at: llvm/cmake/modules/AddPerfetto.cmake:9 + ExternalProject_Add(perfetto_git + GIT_REPOSITORY https://github.com/google/perfetto.git + GIT_TAG releases/v4.x nickdesaulniers wrote: > lebedev.ri wrote: > > nickd

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

lebedev.ri added a comment. In D81385#2164235 , @hans wrote: > In D81385#2162911 , @mstorsjo wrote: > > > Ping @beanz > > > > @hans - I think this is something that would be wanted to have fixed in the > > release

[PATCH] D84898: [clang-tidy] Add new checker for complex conditions with no meaning

lebedev.ri added a comment. I really think this should be in clang proper. I have seen such patterns written in the wild, they should be caught by just the compiler. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84898/new/ https://reviews.llvm.org/

[PATCH] D70366: Add new 'flatten' LLVM attribute to fix clang's 'flatten' function attribute

lebedev.ri added a comment. In D70366#1960300 , @jdoerfert wrote: > I'm fine with this. I would hope a C/C++/Clang person will also take a look > though. This is missing clang codegen test[s]. Seems to look fine to me otherwise. CHANGES SINCE LAST ACT

[PATCH] D76572: Replace `T(x)` with `reinterpret_cast(x)` everywhere it means reinterpret_cast. No functional change

lebedev.ri added a comment. All these changes are rather NFCI, i'm not sure each one needs to be reviewed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76572/new/ https://reviews.llvm.org/D76572 ___ c

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

lebedev.ri added a comment. What's the status here? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71739/new/ https://reviews.llvm.org/D71739 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http

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

lebedev.ri added a comment. https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Ri-global is in "Interfaces" section, it only covers inter-procedural stuff. Diagnosing function-local static non-const variable is a plain false-positive diagnostic. Repository: rG LLVM

[PATCH] D77574: [OpenMP] Fix layering problem with FrontendOpenMP

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

[PATCH] D75591: [OpenMP] Add firstprivate as a default data-sharing attribute to clang

lebedev.ri resigned from this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. No further comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75591/new/ https://reviews.llvm.org/D75591 __

[PATCH] D77984: Make IRBuilder automatically set alignment on load/store/alloca.

lebedev.ri added a comment. Thanks, this is indeed much better and the direction again in-line with what i've raised in some attributor patch. LG to me in general, but please wait for someone else to take a look, too. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://re

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