[PATCH] D28022: [clang-tidy] Handle constructors in performance-unnecessary-value-param

alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: clang-tidy/performance/UnnecessaryValueParamCheck.cpp:114 if (AllDeclRefExprs.size() == 1 && -!hasLoopStmtAncestor(**AllDeclRefExprs.be

[PATCH] D26453: [clang-tidy] Remove duplicated check from move-constructor-init

alexfh added a comment. LG. In https://reviews.llvm.org/D26453#592934, @flx wrote: > In https://reviews.llvm.org/D26453#590720, @malcolm.parsons wrote: > > > Add ValuesOnly option to modernize-pass-by-value. > > > Thanks for doing this. Alex, would this work for us? Yep, I think so. ===

[PATCH] D28022: [clang-tidy] Handle constructors in performance-unnecessary-value-param

alexfh added inline comments. Comment at: clang-tidy/performance/UnnecessaryValueParamCheck.cpp:96 - // Do not trigger on non-const value parameters when: - // 1. they are in a constructor definition since they can likely trigger - //modernize-pass-by-value which will su

[PATCH] D27806: [clang-tidy] Add obvious-invalid-range

alexfh added inline comments. Comment at: clang-tidy/obvious/InvalidRangeCheck.cpp:38 + +const auto CXX11_AlgorithmNames = +CXX_AlgorithmNames + "; " No global `auto` variables, please. In this case `auto` just isn't buying you anything, but in other cases i

[PATCH] D21298: [Clang-tidy] delete null check

alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: clang-tidy/readability/DeleteNullPointerCheck.cpp:29 + const auto BinaryPointerCheckCondition = binaryOperator( + allOf(hasEitherOperand(castE

[PATCH] D21298: [Clang-tidy] delete null check

alexfh added a comment. In https://reviews.llvm.org/D21298#632235, @aaron.ballman wrote: > In https://reviews.llvm.org/D21298#632154, @xazax.hun wrote: > > > Small ping, is this ready to be committed? > > > If @alexfh doesn't sign off by tomorrow, I think it's fine to commit. We can > deal with

[PATCH] D21298: [Clang-tidy] delete null check

alexfh added a comment. In https://reviews.llvm.org/D21298#632265, @alexfh wrote: > In https://reviews.llvm.org/D21298#632235, @aaron.ballman wrote: > > > In https://reviews.llvm.org/D21298#632154, @xazax.hun wrote: > > > > > Small ping, is this ready to be committed? > > > > > > If @alexfh doesn

[PATCH] D21298: [Clang-tidy] delete null check

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. I've noticed a few more minor issues. Otherwise looks good. Thank you for the new check! Comment at: clang-tidy/readability/DeleteNullPointerCheck.cpp:27-38 + const auto Po

[PATCH] D28022: [clang-tidy] Handle constructors in performance-unnecessary-value-param

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG https://reviews.llvm.org/D28022 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

[PATCH] D28189: Extend documentation of how to test clang-tidy checks.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Awesome! Thank you for adding clarity to this part. Looks good with a couple of nits. Comment at: docs/clang-tidy/index.rst:550 +separate `FileCheck`_ invocations: once with

[PATCH] D28189: Extend documentation of how to test clang-tidy checks.

alexfh added inline comments. Comment at: docs/clang-tidy/index.rst:558 +typically the basic `CHECK` forms (`CHECK-MESSAGES` and `CHECK-FIXES`) +are sufficient for matcher tests. Note that the `FileCheck` +documentation mostly assumes the default prefix (`CHECK`), and hence

[PATCH] D21298: [Clang-tidy] delete null check

alexfh added a comment. One more late comment (I should really add a check-list for new checks): this check lacks tests with macros and templates with multiple instantiations. Incorrect handling of templates will likely not manifest in the current state of the check, it's brittle, since it reli

[PATCH] D28189: Extend documentation of how to test clang-tidy checks.

alexfh added inline comments. Comment at: docs/clang-tidy/index.rst:565 +An additional check enabled by ``check_clang_tidy.py`` ensures that +if `CHECK-MESSAGES:` is used in a file then every warning or error +must have an associated CHECK in that file. Looks lik

[PATCH] D26137: [clang-tidy] Add check name to YAML export

alexfh added a comment. Err, looks like I forgot to post comments I entered a few days ago. Just a few nits. Comment at: include/clang/Tooling/Core/Diagnostic.h:62 + Diagnostic(llvm::StringRef DiagnosticName, DiagnosticMessage &Message, + llvm::StringMap &Fix, +

[PATCH] D26137: [clang-tidy] Add check name to YAML export

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good. Fixed the issues myself and running tests before committing this. Thank you for working on this! Comment at: tools/extra/clang-tidy/ClangTidy.cpp:106 void re

[PATCH] D26137: [clang-tidy] Add check name to YAML export

This revision was automatically updated to reflect the committed changes. Closed by commit rL290892: [clang-tidy] Add check name to YAML export (authored by alexfh). Changed prior to commit: https://reviews.llvm.org/D26137?vs=82860&id=82881#toc Repository: rL LLVM https://reviews.llvm.org/D

[PATCH] D25406: Fix doubled whitespace in modernize-use-auto fixit

alexfh added a comment. In https://reviews.llvm.org/D25406#633892, @malcolm.parsons wrote: > I'd prefer a -format option to clang-tidy. Exactly. https://reviews.llvm.org/D25406 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.

[PATCH] D20689: [clang-tidy] Suspicious Call Argument checker

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. In https://reviews.llvm.org/D20689#633889, @varjujan wrote: > I ran the check on multiple projects and tried to categorize the warnings: > real errors, false positives, naming errors

[PATCH] D20693: [clang-tidy] New checker to replace dynamic exception specifications

alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:100 + StringRef ReplacementStr = + IsNoThrow ? NoexceptMacro.empty() ? "noexcept" : NoexceptMacro +

[PATCH] D20693: [clang-tidy] New checker to replace dynamic exception specifications

alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:99 + + assert(CRange.isValid() && "Exception Specification Range is invalid."); + assert(FnTy && "Functi

[PATCH] D26015: Correctly classify main file includes if there is a prefix added

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG. Thank you for the fix! https://reviews.llvm.org/D26015 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/

[PATCH] D26015: Correctly classify main file includes if there is a prefix added

alexfh added a comment. Tests don't pass with this patch applied: $ ninja check-clang-tools [22/23] Running the Clang extra tools' regression tests FAIL: Extra Tools Unit Tests :: clang-tidy/ClangTidyTests/Inclu

[PATCH] D27700: [clang-tidy] refactor ExprSequence out of misc-use-after-move check

alexfh added a comment. One late comment. Comment at: clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.cpp:154-155 +const Stmt *ExprSequence::resolveSyntheticStmt(const Stmt *S) const { + if (SyntheticStmtSourceMap.count(S)) +return SyntheticStmtSourceMap.lookup(S); +

[PATCH] D30931: [clang-tidy] added new identifier naming case type for ignoring case style

alexfh added a comment. In https://reviews.llvm.org/D30931#702578, @jutocz wrote: > You are right, it does look misleading. I'll try to modify it the way you > suggest (though I'm new to LLVM, so be ready to give me more comments;) Sure, thank you for the work! https://reviews.llvm.org/D3093

[PATCH] D30748: [Lexer] Finding beginning of token with escaped new line

alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: lib/Lex/Lexer.cpp:457 +static bool isNewLineEscaped(const char *BufferStart, const char *Str) { + while (Str > BufferStart && isWhitespace(*Str)) +

[PATCH] D30841: [clang-tidy] readability-misleading-indentation: fix chained if

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. Thank you for the fix! One comment inline. Comment at: clang-tidy/readability/MisleadingIndentationCheck.cpp:40-42 + for (auto Iter = ChainedIfs.find(If); Iter !=

[PATCH] D30841: [clang-tidy] readability-misleading-indentation: fix chained if

alexfh added inline comments. Comment at: clang-tidy/readability/MisleadingIndentationCheck.cpp:40-42 + for (auto Iter = ChainedIfs.find(If); Iter != ChainedIfs.end(); + Iter = ChainedIfs.find(Iter->second)) +IfLoc = Iter->second->getIfLoc(); alexfh wr

[PATCH] D30841: [clang-tidy] readability-misleading-indentation: fix chained if

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D30841#702980, @fgross wrote: > Now using `ASTContext::getParents` instead of `ChainedIfs` map. > > For some reason I thought of `getParents` as an expensive functio

[PATCH] D30841: [clang-tidy] readability-misleading-indentation: fix chained if

This revision was automatically updated to reflect the committed changes. Closed by commit rL298059: [clang-tidy] readability-misleading-indentation: fix chained if (authored by alexfh). Changed prior to commit: https://reviews.llvm.org/D30841?vs=92026&id=92116#toc Repository: rL LLVM https

[PATCH] D31049: [Clang-tidy] Fix for misc-noexcept-move-constructor false triggers on defaulted declarations

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG Repository: rL LLVM https://reviews.llvm.org/D31049 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/m

[PATCH] D31049: [Clang-tidy] Fix for misc-noexcept-move-constructor false triggers on defaulted declarations

alexfh added a comment. Do you have commit rights? Repository: rL LLVM https://reviews.llvm.org/D31049 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31049: [Clang-tidy] Fix for misc-noexcept-move-constructor false triggers on defaulted declarations

This revision was automatically updated to reflect the committed changes. Closed by commit rL298101: [Clang-tidy] Fix for misc-noexcept-move-constructor false triggers on defaulted… (authored by alexfh). Changed prior to commit: https://reviews.llvm.org/D31049?vs=92037&id=92159#toc Repository:

[PATCH] D30931: [clang-tidy] modified identifier naming case to use CT_AnyCase for ignoring case style

alexfh added a comment. Could you generate a diff with full context (http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface)? https://reviews.llvm.org/D30931 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://li

[PATCH] D31097: [clang-tidy] don't warn about implicit widening casts in function calls

alexfh added a comment. In https://reviews.llvm.org/D31097#704621, @xazax.hun wrote: > I wonder whether warning on implicit casts still makes sense for example in > mission critical code. So maybe it is worth to have a configuration option > with the default setting being less strict and chatty

[PATCH] D31097: [clang-tidy] don't warn about implicit widening casts in function calls

alexfh added a comment. In https://reviews.llvm.org/D31097#704626, @alexfh wrote: > In https://reviews.llvm.org/D31097#704621, @xazax.hun wrote: > > > I wonder whether warning on implicit casts still makes sense for example in > > mission critical code. So maybe it is worth to have a configurati

[PATCH] D31160: [clang-tidy] Fix misc-move-const-arg for move-only types.

alexfh created this revision. Fix misc-move-const-arg false positives on move-only types. https://reviews.llvm.org/D31160 Files: clang-tidy/misc/MoveConstantArgumentCheck.cpp test/clang-tidy/misc-move-const-arg.cpp Index: test/clang-tidy/misc-move-const-arg.cpp ===

[PATCH] D30931: [clang-tidy] modified identifier naming case to use CT_AnyCase for ignoring case style

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. A couple of nits, otherwise looks good. Do you need me to commit the patch for you? Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:169 .Case("camel_Sn

[PATCH] D30567: [clang-tidy] Fix treating non-space whitespaces in checks list.

alexfh added a comment. In https://reviews.llvm.org/D30567#696436, @curdeius wrote: > Hi Alex and sorry for the late reply. > > The main use case is a more readable `.clang-tidy` configuration checks. > Before this correction one can use something like this: > > --- > Checks: ' > ,*, >

[PATCH] D30931: [clang-tidy] modified identifier naming case to use CT_AnyCase for ignoring case style

This revision was automatically updated to reflect the committed changes. Closed by commit rL298499: [clang-tidy] modified identifier naming case to use CT_AnyCase for ignoring… (authored by alexfh). Changed prior to commit: https://reviews.llvm.org/D30931?vs=92619&id=92623#toc Repository: r

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

alexfh added a comment. In https://reviews.llvm.org/D31130#707197, @aaron.ballman wrote: > This change was reverted in r298470. The use of the include is a > problem because this is not a clang-supplied header file. Also, there's a > (good) question about what array decay is happening in the `

[PATCH] D30567: [clang-tidy] Fix treating non-space whitespaces in checks list.

alexfh added a comment. There's one more trim() you missed. And the test needs to be updated (`s/\\n/ /`). https://reviews.llvm.org/D30567 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-co

[PATCH] D30567: [clang-tidy] Fix treating non-space whitespaces in checks list.

alexfh added inline comments. Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:133 + StringRef UntrimmedGlob = GlobList.substr(0, GlobList.find(',')); + StringRef Glob = UntrimmedGlob.trim(); + GlobList = GlobList.substr(UntrimmedGlob.size() + 1); s/trim

[PATCH] D27210: [clang-tidy] misc-string-compare. Adding a new check to clang-tidy

alexfh added a comment. A late comment here: the check seems to suit better readability module instead of misc. Could you move it there? Repository: rL LLVM https://reviews.llvm.org/D27210 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D31252: [clang-tidy] add readability-compound-statement-size check.

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. Hi Roman, Welcome to the community! As others noted, adding a separate check so similar functionally and implementation-wise to the existing one is not the best way to go here. A si

[PATCH] D30567: [clang-tidy] Fix treating non-space whitespaces in checks list.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG. Thanks! Do you have commit rights? https://reviews.llvm.org/D30567 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm

[PATCH] D29858: [clang-tidy] Catch trivially true statements like a != 1 || a != 3

This revision was automatically updated to reflect the committed changes. Closed by commit rL298607: [clang-tidy] Catch trivially true statements like a != 1 || a != 3 (authored by alexfh). Changed prior to commit: https://reviews.llvm.org/D29858?vs=90087&id=92802#toc Repository: rL LLVM ht

[PATCH] D29858: [clang-tidy] Catch trivially true statements like a != 1 || a != 3

alexfh added a comment. In https://reviews.llvm.org/D29858#707897, @watsond wrote: > Following up. Was this checked in? Do I need to do anything further? Committed the patch now. Thanks for the reminder! Repository: rL LLVM https://reviews.llvm.org/D29858 ___

[PATCH] D30592: [clang-tidy] Fix diag message for catch-by-value

This revision was automatically updated to reflect the committed changes. Closed by commit rL298608: [clang-tidy] Fix diag message for catch-by-value (authored by alexfh). Changed prior to commit: https://reviews.llvm.org/D30592?vs=90572&id=92805#toc Repository: rL LLVM https://reviews.llvm

[PATCH] D29262: Fixes to modernize-use-using

alexfh added a comment. Krystyna, do you need help committing the patch after you address the outstanding comments? https://reviews.llvm.org/D29262 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listin

[PATCH] D29262: Fixes to modernize-use-using

alexfh added a comment. In https://reviews.llvm.org/D29262#709351, @krystyna wrote: > I have plan to finish this patch next week, when I finish academic year at my > school. If I will have any issues with submitting, Prazek offered to help me. Sure, no stress. Good luck with your studies! ht

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

alexfh added a comment. In https://reviews.llvm.org/D31308#709709, @mgehre wrote: > Thanks for your feedback, Eugene! > > I'm not sure if it would be helpful to have this check in both ways. I did a > code search for "not_eq", "bitand" and "and_eq" > on github, and their usage seems to be a cle

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

alexfh added a comment. > So I would propose to keep the features as-is for now, > change the name to readability-operators-representation, and then later > (someone else?) might also add an option > for making this work the other way around. Would that be ok for you? Fine by me. Repository:

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

alexfh added a comment. FWIW, I'm pretty sure this can and should be done on the lexer level - it will be faster and more universal. Repository: rL LLVM https://reviews.llvm.org/D31308 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

alexfh added inline comments. Comment at: clang-tidy/readability/OperatorsRepresentationCheck.cpp:34 + + if (const auto *B = Result.Nodes.getNodeAs("binary")) { +switch (B->getOpcode()) { aaron.ballman wrote: > I think this would make more sense lifted into

[PATCH] D31406: [clang-tidy] Reuse FileID in getLocation

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. Thank you for finding out the root cause here! Comment at: clang-tidy/ClangTidy.cpp:241 -const FileEntry *File = SourceMgr.getFileManager().getFile(FilePath);

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

alexfh added inline comments. Comment at: clang-tidy/readability/OperatorsRepresentationCheck.cpp:34 + + if (const auto *B = Result.Nodes.getNodeAs("binary")) { +switch (B->getOpcode()) { aaron.ballman wrote: > mgehre wrote: > > aaron.ballman wrote: > > > al

[PATCH] D31406: [clang-tidy] Reuse FileID in getLocation

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG Do you have commit rights? https://reviews.llvm.org/D31406 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-b

[PATCH] D31308: [clang-tidy] new check readability-no-alternative-tokens

alexfh added inline comments. Comment at: clang-tidy/readability/OperatorsRepresentationCheck.cpp:34 + + if (const auto *B = Result.Nodes.getNodeAs("binary")) { +switch (B->getOpcode()) { Eugene.Zelenko wrote: > aaron.ballman wrote: > > alexfh wrote: > > > a

[PATCH] D31706: [clang-format] Handle NSString literals by merging tokens.

alexfh created this revision. Herald added a subscriber: klimek. This fixes a couple of outstanding bugs: - incorrect breaking of NSString literals containing double-width characters; - inconsistent formatting of ObjC dictionary literals containing NSString literals; - AlwaysBreakBeforeMultiline

[PATCH] D31713: [Basic] getColumnNumber returns location of CR+LF on Windows

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG with one nit. Thank you for tracking down this bug and fixing it! Comment at: lib/Basic/SourceManager.cpp:1153 + if (FilePos + 1 == LineEnd && FilePos > LineStart) {

[PATCH] D31706: [clang-format] Handle NSString literals by merging tokens.

alexfh added a comment. Friendly ping. https://reviews.llvm.org/D31706 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31860: Add more examples to clang tidy checkers

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Thanks! LG with a couple of nits. Comment at: docs/clang-tidy/checks/misc-inefficient-algorithm.rst:25 + std::set s; + auto c = count(s.begin(), s.end(), 43); + --

[PATCH] D31860: Add more examples to clang tidy checkers

alexfh added a comment. Another couple of post-commit comments. Comment at: docs/clang-tidy/checks/misc-unused-parameters.rst:15 + + void a(int /*i*/) {} + nit: two spaces before the comment. Comment at: docs/clang-tidy/checks/readability-u

[PATCH] D31862: docs/clang-tidy/tools/dump_check_docs.py: Remove deprecated script

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG https://reviews.llvm.org/D31862 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

[PATCH] D31706: [clang-format] Handle NSString literals by merging tokens.

This revision was automatically updated to reflect the committed changes. Closed by commit rL299927: [clang-format] Handle NSString literals by merging tokens. (authored by alexfh). Changed prior to commit: https://reviews.llvm.org/D31706?vs=94218&id=94801#toc Repository: rL LLVM https://re

[PATCH] D31860: Add more examples to clang tidy checkers

alexfh added inline comments. Comment at: docs/clang-tidy/checks/misc-unused-parameters.rst:15 + + void a(int /*i*/) {} + sylvestre.ledru wrote: > alexfh wrote: > > nit: two spaces before the comment. > I believe it is the case ? > I stole this from the unit t

[PATCH] D32112: [clang] Register isConstexpr matcher

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG Repository: rL LLVM https://reviews.llvm.org/D32112 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/m

[PATCH] D32170: Add a FixItHint for -Wmissing-prototypes to insert 'static '.

alexfh created this revision. Missing 'static' on functions that are intended to be local to a translation unit seems to be the most common cause of -Wmissing-prototypes warnings, so suggesting a fix seems to be convenient and useful. https://reviews.llvm.org/D32170 Files: include/clang/Basic

[PATCH] D32164: [clang-tidy] misc-misplaced-widening-cast: Disable checking of implicit widening casts by default

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG https://reviews.llvm.org/D32164 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

[PATCH] D31757: [clang-tidy] Add a clang-tidy check for possible inefficient vector operations

alexfh added a comment. Awesome, thanks! A few late comments inline. Comment at: clang-tools-extra/trunk/clang-tidy/performance/InefficientVectorOperationCheck.cpp:56 +void InefficientVectorOperationCheck::registerMatchers(MatchFinder *Finder) { + const auto VectorDecl = cxxR

[PATCH] D31542: [clang-tidy] Extend readability-container-size-empty to add comparisons to newly-constructed objects

alexfh accepted this revision. alexfh added a comment. LG with one nit. Comment at: clang-tidy/readability/ContainerSizeEmptyCheck.cpp:209 + "the 'empty' method should be used to check " + "for emptiness instead of comparing to an empty object.") +<< Hin

[PATCH] D24886: Add [[clang::suppress(rule, ...)]] attribute

alexfh added inline comments. Comment at: cfe/trunk/include/clang/Basic/AttrDocs.td:2792 + namespace N { +[[clang::suppress("type", "bounds")]]; +... Should this be `gsl::suppress` as well? Repository: rL LLVM https://reviews.llvm.org/D24886

[PATCH] D26015: Correctly classify main file includes if there is a prefix added

This revision was automatically updated to reflect the committed changes. Closed by commit rL291767: Correctly classify main file includes if there is a prefix added (authored by alexfh). Changed prior to commit: https://reviews.llvm.org/D26015?vs=84001&id=84121#toc Repository: rL LLVM http

[PATCH] D26015: Correctly classify main file includes if there is a prefix added

alexfh added a comment. LG, committed as r291767. Repository: rL LLVM https://reviews.llvm.org/D26015 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28334: [clang-tidy] Add -extra-arg and -extra-arg-before to run-clang-tidy.py

alexfh added inline comments. Comment at: clang-tidy/tool/run-clang-tidy.py:80 + for arg in extra_arg: + start.append('-extra-arg=%s' % arg[0]) + for arg in extra_arg_before: Why arg[0] and not just arg? https://reviews.llvm.org/D28334 ___

[PATCH] D28729: [clang-tidy] Add -enable-alpha-checks command

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. As discussed with the Static Analyzer maintainers, alpha checkers are completely unsupported and are suitable for very early testing only. We had problems with them routinely, that's

[PATCH] D28729: [clang-tidy] Add -enable-alpha-checks command

alexfh added inline comments. Comment at: clang-tidy/ClangTidy.cpp:296 const auto &RegisteredCheckers = - AnalyzerOptions::getRegisteredCheckers(/*IncludeExperimental=*/false); bool AnalyzerChecksEnabled = false; This is the place where a small local c

[PATCH] D28667: [clang-tidy] Don't modernize-raw-string-literal if replacement is longer.

alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: test/clang-tidy/modernize-raw-string-literal.cpp:94 +char const *const Concatenated("\"foo\"" + "\"bar\"");

[PATCH] D28729: [clang-tidy] Add -enable-alpha-checks command

alexfh added a comment. In https://reviews.llvm.org/D28729#646555, @aaron.ballman wrote: > In https://reviews.llvm.org/D28729#646548, @alexfh wrote: > > > As discussed with the Static Analyzer maintainers, alpha checkers are > > completely unsupported and are suitable for very early testing only

[PATCH] D28729: [clang-tidy] Add -enable-alpha-checks command

alexfh added a comment. In https://reviews.llvm.org/D28729#647250, @Prazek wrote: > Does solution like this works for you? We don't officially support alpha > checkers, but it is much easier to check if something is already implemented > in static analyzer easily Is it the only problem you're

[PATCH] D28667: [clang-tidy] Don't modernize-raw-string-literal if replacement is longer.

alexfh added inline comments. Comment at: test/clang-tidy/modernize-raw-string-literal.cpp:94 +char const *const Concatenated("\"foo\"" + "\"bar\""); leanil wrote: > alexfh wrote: > > Does this test fail without the patch? Also, sh

[PATCH] D28768: [clang-tidy] Add check 'modernize-return-braced-init-list'

alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: test/clang-tidy/modernize-return-braced-init-list.cpp:132 +auto v1 = []() { return vector({1, 2}); }(); +auto v2 = []() -> vector { return vector({1

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:44 +} else { + /* If a single one is not valid, we cannot apply the fix as we need to + *

[PATCH] D27621: [clang-tidy] check to find declarations declaring more than one name

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. A few more comments. Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:153 + const SourceRange FVLoc(DeclStmt->getLocStart(), Location); + std::st

[PATCH] D28973: Supresses misc-move-constructor-init warning for const fields.

alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: clang-tools-extra/test/clang-tidy/misc-move-constructor-init.cpp:88 +struct O { + O(O&& other) : b(other.b) {} // ok + const B b;

[PATCH] D26466: [clang-tidy] Fix NOLINT test

This revision was automatically updated to reflect the committed changes. Closed by commit rL292926: [clang-tidy] Fix NOLINT test (authored by alexfh). Changed prior to commit: https://reviews.llvm.org/D26466?vs=77378&id=85571#toc Repository: rL LLVM https://reviews.llvm.org/D26466 Files:

[PATCH] D28768: [clang-tidy] Add check 'modernize-return-braced-init-list'

alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: test/clang-tidy/modernize-return-braced-init-list.cpp:150 +template +T f16() { + return T(); "With multiple instantiations" is an

[PATCH] D28667: [clang-tidy] Don't modernize-raw-string-literal if replacement is longer.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG https://reviews.llvm.org/D28667 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

[PATCH] D20693: [clang-tidy] New checker to replace dynamic exception specifications

alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:20-33 +static StringRef +makeDynamicExceptionString(const SourceManager &SM, + c

[PATCH] D26418: [clang-tidy] Add '-suppress-checks-filter' option to suppress diagnostics from certain files

alexfh added a comment. My apologies again for the delay. There are a few things I'm concerned about: 1. Suppression list being a command-line option is going to be inconvenient to completely useless in many setups. The `-line-filter` option is special in this regard, since it was added for a r

[PATCH] D20693: [clang-tidy] New checker to replace dynamic exception specifications

alexfh accepted this revision. alexfh added a comment. LGTM, if Aaron has no concerns. Thank you for the new check! https://reviews.llvm.org/D20693 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listin

[PATCH] D25024: [clang-tidy] Add check for detecting declarations with multiple names

alexfh added a comment. https://reviews.llvm.org/D27621 seems to have a more up-to-date version of this patch. https://reviews.llvm.org/D25024 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cf

[PATCH] D27621: [clang-tidy] check to find declarations declaring more than one name

alexfh added inline comments. Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:130 +if (isa(*(FirstVarIt + 1))) + return "typedef " + TypeString; + Should we suggest `using x = ...;` in C++11 code? Comment at: clang-ti

[PATCH] D21507: Changes after running check modernize-use-emplace (D20964)

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. If you're still considering to submit this patch, could you rebase it (or maybe re-generate instead?) and split into easier to digest parts? A couple of things I noticed: 1. `v.push

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. A couple of nits. Please address Aaron's comment as well. Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:53 + for (const auto &NoExceptRange : NoExceptRang

[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.

alexfh added a comment. In https://reviews.llvm.org/D29151#662504, @zaks.anna wrote: > Before clang-tidy came into existence the guidelines were very clear. One > should write a clang warning if the diagnostic: > > - can be implemented without path-insensitive analysis (including > flow-sensiti

[PATCH] D29267: [clang-tidy] safety-no-assembler

alexfh added a comment. I wonder whether there's a compiler diagnostic for this purpose. Compiler diagnostics are more efficient at reaching users and should be preferred where they are appropriate (this seems like one of such cases). https://reviews.llvm.org/D29267

[PATCH] D26418: [clang-tidy] Add '-suppress-checks-filter' option to suppress diagnostics from certain files

alexfh added a comment. Adding a mechanism to supply suppression lists would be useful as long as it's flexible and extensible enough and doesn't significantly affect performance (especially, when not in use). In particular, it shouldn't be bound to a specific format or a specific way to store

[PATCH] D28973: Supresses misc-move-constructor-init warning for const fields.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG. I'll commit the patch for you. https://reviews.llvm.org/D28973 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/

[PATCH] D28973: Supresses misc-move-constructor-init warning for const fields.

This revision was automatically updated to reflect the committed changes. Closed by commit rL294459: [clang-tidy] Supresses misc-move-constructor-init warning for const fields. (authored by alexfh). Changed prior to commit: https://reviews.llvm.org/D28973?vs=87484&id=87655#toc Repository: rL

<    4   5   6   7   8   9   10   11   12   >