[PATCH] D56090: Add a matcher for members of an initializer list expression

lebedev.ri added a comment. 1. Tests 2. The docs file will need to be regenerated Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56090/new/ https://reviews.llvm.org/D56090 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D54408: [ASTMatchers] Add matchers available through casting to derived

lebedev.ri added a comment. Differential lacks description. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54408/new/ https://reviews.llvm.org/D54408 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://list

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

lebedev.ri added a comment. Please always upload all patches with full context (`-U9`). Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56160/new/ https://reviews.llvm.org/D56160 ___ cfe-commits maili

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

lebedev.ri added a comment. Some thoughts. Comment at: clang-tidy/modernize/UseTrailingReturnCheck.cpp:21-22 + +// very similar to UseOverrideCheck +SourceLocation findTrailingReturnTypeLocation(const CharSourceRange &range, + const

[PATCH] D56241: expose a control flag for interger to pointer ext warning

lebedev.ri added a comment. Looks ok in general. Perhaps there should be a test for this flag specifically? Also, should "pointer-integer-compare" be in some group? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56241/new/ https://reviews.llvm.org/D56241 __

[PATCH] D56241: expose a control flag for interger to pointer ext warning

lebedev.ri added inline comments. Comment at: test/Sema/ext-typecheck-comparison-of-pointer-integer.c:1 +// RUN: %clang_cc1 -triple x86_64-apple-darwin -fsyntax-only -verify -Wpointer-integer-compare %s + I guess you want a run-line with no flag, this runline,

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

lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. To me this looks like a reasonably straight-forward refactoring. I'm guessing the initial code had good test coverage, and none of those tests break; and the new code appears to have re

[PATCH] D56241: expose a control flag for interger to pointer ext warning

lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. ok, LGTM. Comment at: test/Sema/ext-typecheck-comparison-of-pointer-integer.c:1 +// RUN: %clang_cc1 -triple x86_64-apple-darwin -fsyntax-only -verify -Wpointer-inte

[PATCH] D56358: [AST] Move back BasePathSize to the bit-fields of CastExpr

lebedev.ri added inline comments. Comment at: include/clang/AST/Expr.h:3029 +assert((CastExprBits.BasePathSize == BasePathSize) && + "BasePathSize overflow!"); assert(CastConsistency()); riccibruno wrote: > It cannot overflow now, but someone i

[PATCH] D56358: [AST] Move back BasePathSize to the bit-fields of CastExpr

lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. LG in principle. Comment at: include/clang/AST/Expr.h:3029 +assert((CastExprBits.BasePathSize == BasePathSize) && + "BasePathSize overflow!"); asse

[PATCH] D56358: [AST] Move back BasePathSize to the bit-fields of CastExpr

lebedev.ri added inline comments. Comment at: include/clang/AST/Expr.h:3029 +assert((CastExprBits.BasePathSize == BasePathSize) && + "BasePathSize overflow!"); assert(CastConsistency()); riccibruno wrote: > lebedev.ri wrote: > > rjmccall wrote:

[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

lebedev.ri added a comment. In D56424#1349218 , @karepker wrote: > Clean up comments in test file. For reference, what documentation sources did you read when creating the review itseft? I thought it was documented that an appropriate category (`[clang-

[PATCH] D67399: [ARM] Follow AACPS standard for volatile bitfields

lebedev.ri resigned from this revision. lebedev.ri added a reviewer: jfb. lebedev.ri added a comment. Herald added a subscriber: dexonsmith. I'm not sure why i'm added as a reviewer here. That being said i don't see why that load is needed there. As i read it, the document only states that if loa

[PATCH] D67399: [ARM] Follow AACPS standard for volatile bitfields

lebedev.ri added a comment. In D67399#1664785 , @dnsampaio wrote: > @ostannard might prove me wrong, but according to the AACPS: > > When a volatile bit-field is written, and its container does not overlap > with any non-bit-field member, its >conta

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

lebedev.ri added a comment. In D66324#1665773 , @gribozavr wrote: > Reverted in r371598. > > Another concern that I have is cross-compilation. LLVM's ADT is not set up to > be cross-compiled like the rest of compiler-rt is. Uhm, i have a better question

[PATCH] D67567: New ClangTidy checks to warn about misusing dispatch_once_t

lebedev.ri added a comment. 1. Please split each check into separate review. 2. Is `dispatch_once_t` OSX-specific thing? Should those checks be in `osx` module? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67567/new/ https://reviews.llvm.org/D6756

[PATCH] D67567: New ClangTidy check to warn when storing dispatch_once_t in non-static, non-global storage

lebedev.ri added a comment. In D67567#1670017 , @mwyman wrote: > In D67567#1669866 , @lebedev.ri > wrote: > > > 1. Please split each check into separate review. > > 2. Is `dispatch_once_t` OSX-specific thing? Shoul

[PATCH] D67588: Add builtin trait for add/remove cv (and similar)

lebedev.ri added a comment. > The `__remove_cv` test sees a 160% build time imporvement while the > `__add_cv` test sees an 8% improvement. Those numbers are specifically for those macrobenchmarks, right? What impact does this have on some real-world code? I'm just curious as to cost/benefit her

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

lebedev.ri added a comment. Layering feels weird here. Can this be implemented as/limited to just a `run-clang-misexpect.py` wrapper over clang itself? That would be best IMHO. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67253/new/ https://review

[PATCH] D66564: [clang-tidy] new FPGA struct pack align check

lebedev.ri added a comment. I, too, don't believe this is FPGA specific; it should likely go into `misc-` or even `performance-`. The wording of the diags seems weird to me, it would be good to 1. add more explanation to the docs and 2. reword the diags. Comment at: clang-tid

[PATCH] D66564: [clang-tidy] new FPGA struct pack align check

lebedev.ri added a comment. Forgot the most important question. Right now this will fire on every single struct. But it won't matter unless the alignment/size actually matters, and most often that will happen when you have e.g. a vector of such structs. What i'm asking is - should this be more pi

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

lebedev.ri added a comment. Re concurrency - you had standalone `LLVMContext` for each thread, right? In D67253#1670667 , @paulkirth wrote: > In D67253#1670569 , @lebedev.ri > wrote: > > > Layering feels weird her

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

lebedev.ri added a comment. In D67253#1670704 , @paulkirth wrote: > In D67253#1670681 , @lebedev.ri > wrote: > > > Re concurrency - you had standalone `LLVMContext` for each thread, right? > > > I believe there was

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

lebedev.ri added a comment. Some thoughts: 1. Docs missing 2. How does this play with C++17 aligned new? Assuming compiler/library support for that isn't broken ('bye, OSX!', ?), i'm not sure why it would be UB for C++17, see https://godbolt.org/z/kwxRbu vs https://godbolt.org/z/om-bR2 3. I'm n

[PATCH] D67567: [clang-tidy] New check to warn when storing dispatch_once_t in non-static, non-global storage

lebedev.ri added a comment. In D67567#1671499 , @mwyman wrote: > In D67567#1670264 , @NoQ wrote: > > > FTR, we already have a similar Static Analyzer check, eg.: > > > > https://github.com/llvm-mirror/clang/blob/r

[PATCH] D67661: [RISCV] Headers: Add Bitmanip extension Clang header files and rvintrin.h

lebedev.ri added a comment. Inline asm is //really// unfriendly to the optimizer. Ideally the plan should be to incrementally getting rid of it as soon as backend learns to properly match particular builtin. Comment at: clang-tools-extra/clang-include-fixer/find-all-symbols/S

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

lebedev.ri added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/DefaultOperatorNewCheck.cpp:51 + // The alignment used by default 'operator new' (in bits). + const unsigned DefaultAlignment = Context.getTargetInfo().getNewAlign(); + martong wrot

[PATCH] D67661: [RISCV] Headers: Add Bitmanip extension Clang header files and rvintrin.h

lebedev.ri added a comment. In D67661#1673712 , @s.egerton wrote: > I agree inline asm is a far from optimal solution but it seems like the > lesser of two evils for now. Hm, i thought some previous patch already adds llvm ir riscv-specific intrinsics

[PATCH] D67661: [RISCV] Headers: Add Bitmanip extension Clang header files and rvintrin.h

lebedev.ri added a comment. In D67661#1673993 , @lebedev.ri wrote: > In D67661#1673918 , @s.egerton wrote: > > > Sorry I misread your original comment. > > > (which one?) > > > These functions exist so that we can g

[PATCH] D67661: [RISCV] Headers: Add Bitmanip extension Clang header files and rvintrin.h

lebedev.ri added a comment. In D67661#1673918 , @s.egerton wrote: > Sorry I misread your original comment. (which one?) > These functions exist so that we can guarantee that these particular > instructions will be emitted; Sure, that makes sense. > t

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

lebedev.ri marked an inline comment as done. lebedev.ri added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/DefaultOperatorNewCheck.cpp:51 + // The alignment used by default 'operator new' (in bits). + const unsigned DefaultAlignment = Context.getTargetInfo().g

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

lebedev.ri added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp:69 +// MEM +CheckFactories.registerCheck("cert-mem57-cpp"); I find `DefaultOperatorNewCheck` to be insufficiently explanative. Maybe `DefaultOperatorNewAl

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

lebedev.ri added inline comments. Comment at: cfe/trunk/lib/CodeGen/BackendUtil.cpp:1426-1431 EmitAssemblyHelper AsmHelper(Diags, HeaderOpts, CGOpts, TOpts, LOpts, M); if (CGOpts.ExperimentalNewPassManager) AsmHelper.EmitAssemblyWithNewPassManager(Action, std::move(O

[PATCH] D67877: Conditionnaly include clang Analysis examples with cmake.

lebedev.ri added a comment. Please upload all patches with full context (`-U9`) Comment at: clang/lib/Analysis/plugins/CMakeLists.txt:1-2 -if(CLANG_ENABLE_STATIC_ANALYZER AND LLVM_ENABLE_PLUGINS) +if(CLANG_ENABLE_STATIC_ANALYZER AND LLVM_ENABLE_PLUGINS AND CLANG_BUILD_EXAM

[PATCH] D67877: [analyzer] Conditionnaly include clang Analysis examples with cmake.

lebedev.ri added a subscriber: tstellar. lebedev.ri added a comment. Actually, uh oh. @Jiboo can you please file this as a bug? This really should go into next patch release. CC @tstellar @Szelethus this breaks LLVMExports.cmake as compared with LLVM-8, so it's actually a critical bug. Reposit

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

lebedev.ri added inline comments. Comment at: cfe/trunk/lib/CodeGen/BackendUtil.cpp:1426-1431 EmitAssemblyHelper AsmHelper(Diags, HeaderOpts, CGOpts, TOpts, LOpts, M); if (CGOpts.ExperimentalNewPassManager) AsmHelper.EmitAssemblyWithNewPassManager(Action, std::move(O

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

lebedev.ri added inline comments. Comment at: cfe/trunk/lib/CodeGen/BackendUtil.cpp:1426-1431 EmitAssemblyHelper AsmHelper(Diags, HeaderOpts, CGOpts, TOpts, LOpts, M); if (CGOpts.ExperimentalNewPassManager) AsmHelper.EmitAssemblyWithNewPassManager(Action, std::move(O

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

lebedev.ri added inline comments. Comment at: cfe/trunk/lib/CodeGen/BackendUtil.cpp:1426-1431 EmitAssemblyHelper AsmHelper(Diags, HeaderOpts, CGOpts, TOpts, LOpts, M); if (CGOpts.ExperimentalNewPassManager) AsmHelper.EmitAssemblyWithNewPassManager(Action, std::move(O

[PATCH] D68074: [clang-tidy] Add readability-make-member-function-const

lebedev.ri added a comment. Awesome! I believe i have asked this question in the convert-to-static diff - can ExprMutAnalyzer be used here? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68074/new/ https://reviews.llvm.org/D68074 ___

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

lebedev.ri updated this revision to Diff 221929. lebedev.ri edited the summary of this revision. lebedev.ri added a comment. Rebased. I've added most (?) of the obviously-missing folds. This improved situation, although admittedly by not as much as i had hoped. ping @vsk; can this get going plea

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

lebedev.ri marked 2 inline comments as done. lebedev.ri added a comment. Comment at: clang/docs/ReleaseNotes.rst:238 -- ... +- * ``pointer-overflow`` check was extended added to catch the cases where +a non-zero offset being applied, either to a ``nullptr``, or the resul

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

lebedev.ri updated this revision to Diff 45. lebedev.ri marked 6 inline comments as done. lebedev.ri added a comment. Patch updated - mainly wording changes only. It isn't clear that for C the rule is actually different from C++, if it is it does not make much sense and is irrelevant for LLVM

[PATCH] D68161: [TimeProfiler] Fix "OptModule" section and add new "Backend" sections

lebedev.ri accepted this revision. lebedev.ri added a reviewer: thakis. lebedev.ri added a comment. This revision is now accepted and ready to land. Oh nice! F10096481: image.png F10096488: image.png I like this. One more r

[PATCH] D68172: Don't install example analyzer plugins

lebedev.ri added a comment. There's also D67877 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68172/new/ https://reviews.llvm.org/D68172 ___ cfe-commits mailing list c

[PATCH] D68172: Don't install example analyzer plugins

lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. I feel like i can stamp this. There's similar-isn problem with lit-cpuid which is referenced in LLVMExports.cmake but is packaged in debian in lldb... Repository: rG LLVM Github Mon

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

lebedev.ri added a comment. In D67122#1691307 , @lebedev.ri wrote: > Ping. > After talking with @aaron.ballman, revised the check to also handle the C > semantics ("ptr+0 is UB"). And that resulted in one more occurrence in test-suite - rL373486

[PATCH] D68388: [PR41008][OpenCL] Support restrict keyword in C++ mode

lebedev.ri added a comment. Is there a test that shows that that keyword is still not provided for C++ after this? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68388/new/ https://reviews.llvm.org/D68388 ___ cfe-com

[PATCH] D68410: [AttrDocs] document always_inline

lebedev.ri added a reviewer: aaron.ballman. lebedev.ri added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:4394-4395 } + + +def AlwaysInlineDocs : Documentation { One too many lines Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D44602: [clang-tidy] readability-function-size: add VariableThreshold param.

lebedev.ri added a comment. @alexfh i'm waiting for some comment from you. there was no comment added when you removed yourself as a reviewer, so i *assume* the usual message of that time was meant - "removing from my review queue, re-add when others have accepted.". Repository: rCTE Clang T

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

lebedev.ri added a comment. Third commit introducing linking errors. Did no buildbot catch those? Repository: rL LLVM https://reviews.llvm.org/D43764 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/li

[PATCH] D45555: [clang-apply-replacements] Fix a link failure in applyChanges()

lebedev.ri added a comment. In https://reviews.llvm.org/D4#1065387, @aheejin wrote: > Looks like the same patch https://reviews.llvm.org/rL329892 has already > landed. Sorry, somehow i missed this differential. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D4

[PATCH] D44602: [clang-tidy] readability-function-size: add VariableThreshold param.

lebedev.ri added a comment. In https://reviews.llvm.org/D44602#1065366, @alexfh wrote: > In https://reviews.llvm.org/D44602#1065331, @lebedev.ri wrote: > > > @alexfh i'm waiting for some comment from you. there was no comment added > > when you removed yourself as a reviewer, > > so i *assume*

[PATCH] D45601: Warn on bool* to bool conversion

lebedev.ri added a comment. @hiraditya I personally don't like when i'm being told so, but i'd like to see some numbers... **Please** run this on //some// big C++ project (LLVM (but you'll have to enable this diag specifically), google chrome, ???), and analyse the results. In https://reviews.l

[PATCH] D45615: [builtins] __builtin_dump_struct : added more types format

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

[PATCH] D45601: Warn on bool* to bool conversion

lebedev.ri added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:7806 + "comparing %0 as a boolean">, + InGroup; def warn_format_argument_needs_cast : Warning< Also, this really really should be under it's own flag, which in turn may be

[PATCH] D45601: Warn on bool* to bool conversion

lebedev.ri added a comment. In https://reviews.llvm.org/D45601#1067057, @aaron.ballman wrote: > ... > This makes me wary of making this a compiler diagnostic, but clang-tidy may > still be a reasonable place for this functionality to live. In https://reviews.llvm.org/D45601#1066572, @Eugene

[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)

lebedev.ri added a comment. In https://reviews.llvm.org/D44883#1068400, @brooksmoses wrote: > I have noticed two things when attempting to release LLVM with this revision > internally at Google: > > 1. It's catching real bugs, all in constructors where someone wrote "member_ > = member_" when t

[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)

lebedev.ri added a comment. Re false-positives - at least two [post-]reviewers need to agree on the way forward (see previous comments, mail thread), and then i will implement it. In https://reviews.llvm.org/D44883#1068576, @brooksmoses wrote: > A further concern about this in the general case

[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)

lebedev.ri added a comment. There are several options: 1. @rjmccall's idea: `-wtest` (lowercase), which in this case will disable that new code in `BuildOverloadedBinOp()`. i quite like it actually. 2. split it up like i had in the first revision - ``-Wself-assign-builtin``, ``-Wself-assign-fie

[PATCH] D45685: [Sema] Add -Wtest global flag that silences -Wself-assign for overloaded operators.

lebedev.ri created this revision. lebedev.ri added reviewers: rjmccall, aaron.ballman, dblaikie, thakis, rsmith, chandlerc, brooksmoses. Herald added a subscriber: klimek. Credit for the idea goes to @rjmccall. I'm not quite sure how to do it with `-wtest` (lowercase). so i just modelled it afte

[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)

lebedev.ri added a comment. In https://reviews.llvm.org/D44883#1068602, @lebedev.ri wrote: > There are several options: > > 1. @rjmccall's idea: `-wtest` (lowercase), which in this case will disable > that new code in `BuildOverloadedBinOp()`. i quite like it actually. > 2. split it up like i ha

[PATCH] D45685: [Sema] Add -Wtest global flag that silences -Wself-assign for overloaded operators.

lebedev.ri updated this revision to Diff 142627. lebedev.ri added a comment. - Don't mis-spell the name of the flag. FIXME: So do we want it to be `-Wtests`, `-Wtest`, or we really want it to be `-wtest` ? Repository: rC Clang https://reviews.llvm.org/D45685 Files: docs/ReleaseNotes.rst

[PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.

lebedev.ri updated this revision to Diff 142636. lebedev.ri retitled this revision from "[Sema] Add -Wtest global flag that silences -Wself-assign for overloaded operators." to "[Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.". lebedev.ri edited the summary of

[PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.

lebedev.ri added a comment. In https://reviews.llvm.org/D45685#1068981, @Quuxplusone wrote: > I don't understand the spelling of this option. You spell it `-wtest` > (because rjmccall suggested that spelling); but for my money it should be > spelled `-Wno-self-assign-nonfield`. You did read t

[PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.

lebedev.ri added a comment. In https://reviews.llvm.org/D45685#1069040, @dblaikie wrote: > I'm not sure this is a practical direction to pursue - though perhaps > others disagree. > It's likely non-trivial to plumb a flag through most build systems to be > applied only to test code I'm sor

[PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.

lebedev.ri added a comment. Uuuh, the fact that phab posts the top-postings, but silently ignores inline replies is annoying. >> lebedev.ri added a comment. >> >> In https://reviews.llvm.org/D45685#1069040, @dblaikie wrote: >> >>> I'm not sure this is a practical direction to pursue - though p

[PATCH] D45601: Warn on bool* to bool conversion

lebedev.ri added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:7806 + "comparing %0 as a boolean">, + InGroup; def warn_format_argument_needs_cast : Warning< thakis wrote: > lebedev.ri wrote: > > Also, this really really should be und

[PATCH] D45766: [Sema] Add -Wno-self-assign-overloaded

lebedev.ri created this revision. lebedev.ri added reviewers: dblaikie, aaron.ballman, thakis, rjmccall, rsmith. It seems there isn't much enthusiasm for `-wtest` https://reviews.llvm.org/D45685. This is more conservative version, which i had in the very first revision of https://reviews.llvm.or

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

lebedev.ri created this revision. lebedev.ri added reviewers: sbenza, alexfh, klimek. https://reviews.llvm.org/D5911 added support for profiling clang-tidy checks. It works nice, the tabulated report generated by `clang-tidy -enable-check-profile` is readable. Unfortunately, it gets complicated

[PATCH] D45766: [Sema] Add -Wno-self-assign-overloaded

lebedev.ri added a comment. Ping. At least one of these needs to land. Repository: rC Clang https://reviews.llvm.org/D45766 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45944: Disallow pointers to const in __sync_fetch_and_xxx

lebedev.ri added a comment. Please always upload all patches with full context (`-U9`) https://reviews.llvm.org/D45944 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45766: [Sema] Add -Wno-self-assign-overloaded

lebedev.ri added inline comments. Comment at: lib/Sema/SemaExpr.cpp:11527-11528 - S.Diag(OpLoc, diag::warn_self_assignment) - << LHSDeclRef->getType() - << LHSExpr->getSourceRange() << RHSExpr->getSourceRange(); + S.Diag(OpLoc, IsBuiltin ? diag::warn_self_assignment

[PATCH] D45766: [Sema] Add -Wno-self-assign-overloaded

lebedev.ri updated this revision to Diff 143645. lebedev.ri added a comment. Add negative tests, too. Repository: rC Clang https://reviews.llvm.org/D45766 Files: docs/ReleaseNotes.rst include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaExpr

[PATCH] D45766: [Sema] Add -Wno-self-assign-overloaded

lebedev.ri added inline comments. Comment at: lib/Sema/SemaExpr.cpp:11527-11528 - S.Diag(OpLoc, diag::warn_self_assignment) - << LHSDeclRef->getType() - << LHSExpr->getSourceRange() << RHSExpr->getSourceRange(); + S.Diag(OpLoc, IsBuiltin ? diag::warn_self_assignment

[PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.

lebedev.ri added a comment. @brooksmoses The simple `-Wno-self-assign-overloaded` variant (https://reviews.llvm.org/D45766) has landed, so the issue should be resolved? Not sure what to do with this differential, should i abandon it until there is more interest for such functionality? Reposit

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

lebedev.ri added inline comments. Comment at: include/__config:1016 +// because GCC does not silence them via (void) cast. +#if __has_cpp_attribute(nodiscard) && _LIBCPP_STD_VER > 17 +# define _LIBCPP_NODISCARD [[nodiscard]] mclow.lists wrote: > `[[nodiscard]]`

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

lebedev.ri added inline comments. Comment at: include/__config:1016 +// because GCC does not silence them via (void) cast. +#if __has_cpp_attribute(nodiscard) && _LIBCPP_STD_VER > 17 +# define _LIBCPP_NODISCARD [[nodiscard]] mclow.lists wrote: > lebedev.ri wrote

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

lebedev.ri added inline comments. Comment at: test/libcxx/diagnostics/force_nodiscard.pass.cpp:16 +// MODULES_DEFINES: _LIBCPP_FORCE_NODISCARD +#define _LIBCPP_DISABLE_NODISCARD_AFTER_CXX17 +#define _LIBCPP_FORCE_NODISCARD lebedev.ri wrote: > Quuxplusone wrote: >

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

lebedev.ri updated this revision to Diff 143872. lebedev.ri marked 8 inline comments as done. lebedev.ri added a comment. Updated based on @mclow.lists review. Repository: rCXX libc++ https://reviews.llvm.org/D45179 Files: include/__config test/libcxx/diagnostics/force_nodiscard.fail.cpp

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

lebedev.ri added inline comments. Comment at: test/libcxx/diagnostics/force_nodiscard.fail.cpp:22 + +_LIBCPP_NODISCARD_AFTER_CXX17 int foo() { return 6; } + mclow.lists wrote: > Shouldn't this be just `_LIBCPP_NODISCARD` ? > I don't think so? I thought we are in

[PATCH] D43341: [clang-doc] Implement reducer portion of the frontend framework

lebedev.ri added inline comments. Comment at: clang-doc/BitcodeWriter.h:129 // Check that the static size is large-enough. assert(Record.capacity() > BitCodeConstants::RecordSize); } juliehockett wrote: > lebedev.ri wrote: > > lebedev.ri wrote: > > >

[PATCH] D46135: [Driver, CodeGen] add options to enable/disable an FP cast optimization

lebedev.ri added inline comments. Comment at: test/CodeGen/no-junk-ftrunc.c:1 +// RUN: %clang_cc1 -S -ffp-cast-overflow-workaround %s -emit-llvm -o - | FileCheck %s + For a good measure, i'd add one more `RUN` line to test that it is currently the default. (Yes

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

lebedev.ri added a comment. I just feel like pointing out that you can already do that: $ clang-tidy -checks=clang-analyzer-alpha* -p <> (be wary of `*` expanding) `clang-tidy --help` says: -checks= - <...> This option's value is appended to

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

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

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

lebedev.ri added a comment. Oh, hm. I just needed something to view the call graph. I have almost wrote a clang-tidy check using `analysis/CallGraph.h`, but then i noticed/remembered that clang static analyzer has that already. But `$ clang-tidy -checks=* -list-checks | grep -i analyzer | grep -i

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

lebedev.ri added a comment. In https://reviews.llvm.org/D46159#1080939, @lebedev.ri wrote: > Oh, hm. > I just needed something to view the call graph. > I have almost wrote a clang-tidy check using `analysis/CallGraph.h`, but > then i noticed/remembered that clang static analyzer has that alre

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

lebedev.ri created this revision. lebedev.ri added reviewers: dcoughlin, alexfh, aaron.ballman, george.karpenkov, NoQ. Herald added subscribers: a.sidorin, szepet, xazax.hun. I would like to be able to trigger these debug checkers from clang-tidy, as a follow-up patch for https://reviews.llvm.or

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

lebedev.ri created this revision. lebedev.ri added reviewers: alexfh, aaron.ballman, hokein. Herald added a subscriber: xazax.hun. After https://reviews.llvm.org/D46159 was created by @pfultz2, and i initially thought it was already possible, i had the need to view the call graph. I have almost w

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

lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. So i suspect the history here is: https://reviews.llvm.org/D26310 added a way to filter-out these clang analyzer checks, which removed these checks from being available in clang-tidy, b

[PATCH] D46135: [Driver, CodeGen] add options to enable/disable an FP cast optimization

lebedev.ri added a comment. In https://reviews.llvm.org/D46135#1081165, @spatel wrote: > Should I add a bullet for this new flag/attribute to the clang release notes, > the LLVM release, both? Or do we view this as temporary and not making it to > the release? (Not everyone is always using tr

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

lebedev.ri added a comment. In https://reviews.llvm.org/D46159#1081366, @dcoughlin wrote: > ... Note that it is completely off by default, and is not listed in documentation, `clang-tidy --help`, so one would have to know of the existence of that hidden flag first, and only then when that fla

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

lebedev.ri added a comment. In https://reviews.llvm.org/D46159#1081392, @dcoughlin wrote: > In https://reviews.llvm.org/D46159#1081371, @lebedev.ri wrote: > > > Note that it is completely off by default, and is not listed in > > documentation, `clang-tidy --help`, > > so one would have to know

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

lebedev.ri added a comment. In https://reviews.llvm.org/D46159#1081559, @NoQ wrote: > Am i understanding correctly that the new `-enable-alpha-checks` flag doesn't > enable alpha checks, but it only allows enabling them with a separate command? Yes, absolutely! Same with my followup https://re

[PATCH] D46236: [Driver, CodeGen] rename options to disable an FP cast optimization

lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. Overall makes sense to me. Comment at: docs/ReleaseNotes.rst:94 + :option:`-fno-strict-float-cast-overflow` - + When a floating-point value is not representable in

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

lebedev.ri added a comment. 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 multiple translation units: per-file table after each file and > an aggregate ta

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

lebedev.ri added inline comments. Comment at: clang-doc/BitcodeWriter.cpp:275 +static SymbolID SID; void ClangDocBitcodeWriter::emitRecord(const SymbolID &Sym, RecordId ID) { 1. I'm not seeing where this is being changed? 2. This looks like some abstraction is

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

lebedev.ri added a comment. Global question: you are using `NamedDecl::getNameAsString()`, and passing it as `StringRef`. You have looked at this with ASAN, and it's ok? Also, have you considered using the `NamedDecl::getName()`, which already returns `StringRef`.? Comment a

[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); >>! In D46281#1083806, @lebedev.ri wrote: > Global questi

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

lebedev.ri added inline comments. Comment at: clang-tidy/bugprone/MapSubscriptOperatorLookupCheck.cpp:25 + hasName("operator[]"), + ofClass(hasAnyName("::std::map", "::std::unordered_map", + hasParent(implicitCastExpr( I wou

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

lebedev.ri created this revision. lebedev.ri added reviewers: craig.topper, asbirlea, rnk, GGanesh, andreadb. Herald added subscribers: Sanitizers, JDevlieghere, arichardson, aprantl, dberris, sdardis. The compiler-rt side of https://reviews.llvm.org/D46314 I have discovered an issue by accident

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