[PATCH] D47122: [clang-tidy] SimplifyBoolenExpr doesn't add parens if unary negotiation is of ExprWithCleanups type

2018-05-22 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added inline comments. Comment at: clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp:198 E = E->ignoreParenBaseCasts(); + if (const auto *EC = dyn_cast(E)) +E = EC->getSubExpr(); `E->IgnoreImplicit()` can be used

[PATCH] D47122: [clang-tidy] SimplifyBoolenExpr doesn't add parens if unary negotiation is of ExprWithCleanups type

2018-05-23 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added inline comments. Comment at: clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp:198 E = E->ignoreParenBaseCasts(); + if (const auto *EC = dyn_cast(E)) +E = EC->getSubExpr(); zinovy.nis wrote: > zinovy.nis wr

[PATCH] D38179: [clang-tidy] Handle unions in modernize-use-equals-default check

2017-09-26 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added inline comments. Comment at: clang-tidy/modernize/UseEqualsDefaultCheck.cpp:117 + const CXXMethodDecl *Operator) { + const auto *Record = Operator->getParent(); + aaron.ballman wrote: > Please don

[PATCH] D43500: [clang-tidy]: modernize-use-default-member-init: Remove trailing comma and colon.

2018-02-20 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added a comment. In https://reviews.llvm.org/D43500#1013497, @aaron.ballman wrote: > Is there a way to make clang-apply-replacements smarter rather than forcing > every check to jump through hoops? I'm worried that if we have to fix > individual checks we'll just run into the sa

[PATCH] D33526: Fix spurious Wunused-lambda-capture warning

2017-06-02 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added inline comments. Comment at: lib/Sema/SemaLambda.cpp:1525-1526 + if (!CurContext->isDependentContext() && !IsImplicit) +if ((IsGenericLambda && !From.isNonODRUsed()) || +(!IsGenericLambda && !From.isODRUsed())) + DiagnoseUnu

[PATCH] D32942: [clang-tidy] readability-function-size: add NestingThreshold param.

2017-06-13 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added a comment. In https://reviews.llvm.org/D32942#777001, @lebedev.ri wrote: > Which makes sense, since in AST, they are nested: They're not nested in the formatting, so I don't think it makes sense. Repository: rL LLVM https://reviews.llvm.org/D32942 _

[PATCH] D33526: Fix spurious Wunused-lambda-capture warning

2017-06-13 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added a comment. I don't understand why init captures have this problem. The variable is defined within the context of the lambda, but it's not within the context of the templated method. Are you working around a bug somewhere else? Repository: rL LLVM https://reviews.llvm.or

[PATCH] D33526: Fix spurious Wunused-lambda-capture warning

2017-06-13 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons accepted this revision. malcolm.parsons added a comment. This revision is now accepted and ready to land. LGTM! Repository: rL LLVM https://reviews.llvm.org/D33526 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lis

[PATCH] D32942: [clang-tidy] readability-function-size: add NestingThreshold param.

2017-06-14 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added a comment. In https://reviews.llvm.org/D32942#779143, @lebedev.ri wrote: > In https://reviews.llvm.org/D32942#778729, @malcolm.parsons wrote: > > > In https://reviews.llvm.org/D32942#777001, @lebedev.ri wrote: > > > > > Which makes sense, since in AST, they are nested: > > >

[PATCH] D32942: [clang-tidy] readability-function-size: add NestingThreshold param.

2017-06-14 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added a comment. In https://reviews.llvm.org/D32942#780090, @lebedev.ri wrote: > In https://reviews.llvm.org/D32942#780053, @malcolm.parsons wrote: > > > My prototype of this feature used `ifStmt(stmt().bind("if"), > > unless(hasParent(ifStmt(hasElse(equalsBoundNode("if"))`.

[PATCH] D34202: [clang-tidy] readability-function-size: fix nesting level calculation

2017-06-14 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added inline comments. Comment at: clang-tidy/readability/FunctionSizeCheck.cpp:38 +} else { + assert(!isa(Node)); + if (TrackedParent.back()) I don't think this assert adds anything. Comment at: clang-tidy/readabi

[PATCH] D34202: [clang-tidy] readability-function-size: fix nesting level calculation

2017-06-14 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added a comment. What do you expect for this? if (true) if (true) if (true) { int j; } Repository: rL LLVM https://reviews.llvm.org/D34202 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D34202: [clang-tidy] readability-function-size: fix nesting level calculation

2017-06-14 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added a comment. My coding standards require the `{}`, so while I may not agree with your definition of nesting, it doesn't matter. Repository: rL LLVM https://reviews.llvm.org/D34202 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D34202: [clang-tidy] readability-function-size: fix nesting level calculation

2017-06-14 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added a comment. LGTM! Comment at: clang-tidy/readability/FunctionSizeCheck.cpp:58 +// is already nested NestingThreshold levels deep, record the start location +// of this new compound statement +if (CurrentNestingLevel == Info.NestingThreshold) --

[PATCH] D34238: clang-format: Do not binpack initialization lists

2017-06-15 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added a comment. Some people write auto x = std::map{ { 0, "foo fjakfjaklf kljj" }, { 1, "bar fjakfjaklf kljj" }, { 2, "stuff fjakfjaklf kljj" }, }; https://reviews.llvm.org/D34238 ___ cfe-commits mailing list

[PATCH] D33722: [clang-tidy] Add checker for undelegated copy of base classes

2017-06-15 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added a comment. This check is similar to `misc-move-constructor-init`; could it have a similar name please. https://reviews.llvm.org/D33722 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailm

[PATCH] D41456: [clang-tidy] readability-else-after-return: also diagnose noreturn function calls.

2017-12-21 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added a comment. This check could also handle else after goto. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41456 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listin

[PATCH] D44295: Detects and fixes calls to grand-...parent virtual methods instead of calls to parent's virtual methods

2018-03-09 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added inline comments. Comment at: clang-tidy/bugprone/ParentVirtualCallCheck.cpp:152 +Member->getQualifierLoc().getSourceRange(), +GetNameAsString(*(Parents.front())) + "::"); + } What does this do for templated parent classes?

[PATCH] D44295: [clang-tidy] Detects and fixes calls to grand-...parent virtual methods instead of calls to parent's virtual methods

2018-03-09 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added inline comments. Comment at: clang-tidy/bugprone/ParentVirtualCallCheck.cpp:152 +Member->getQualifierLoc().getSourceRange(), +GetNameAsString(*(Parents.front())) + "::"); + } zinovy.nis wrote: > malcolm.parsons wrote: > > Wh

[PATCH] D44295: [clang-tidy] Detects and fixes calls to grand-...parent virtual methods instead of calls to parent's virtual methods

2018-03-10 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added inline comments. Comment at: test/clang-tidy/bugprone-parent-virtual-call.cpp:125 + // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'A::virt_1' is a grand-parent's method, not parent's. Did you mean 'BF'? + // CHECK-FIXES: int virt_1() override { return BF:

[PATCH] D44295: [clang-tidy] Detects and fixes calls to grand-...parent virtual methods instead of calls to parent's virtual methods

2018-03-12 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added inline comments. Comment at: test/clang-tidy/bugprone-parent-virtual-call.cpp:115 + int virt_1() override { return A::virt_1(); } + // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'A::virt_1' is a grand-parent's method, not parent's. Did you mean 'BF'? + //

[PATCH] D16008: [clang-tidy] Add calling virtual functions in constructors/destructors check.

2018-03-13 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added a comment. In https://reviews.llvm.org/D16008#1035789, @aaron.ballman wrote: > Do you know why the CSA checker is still in alpha? It isn't - https://reviews.llvm.org/D26768 moved it to optin. I don't know why https://reviews.llvm.org/D34275 didn't turn it on by default.

[PATCH] D53377: [clang-tidy] Ignore a case where the fix of make_unique check introduces side effect.

2018-10-17 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons accepted this revision. malcolm.parsons added a comment. This revision is now accepted and ready to land. LGTM. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53377 ___ cfe-commits mailing list cfe-commits@lists.llvm

[PATCH] D54941: [clang-tidy] Ignore bool -> single bit bitfield conversion in readability-implicit-bool-conversion

2018-11-27 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons created this revision. malcolm.parsons added reviewers: alexfh, aaron.ballman, hokein. Herald added subscribers: cfe-commits, xazax.hun. There is no ambiguity / information loss in this conversion Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54941 Files: cla

[PATCH] D54941: [clang-tidy] Ignore bool -> single bit bitfield conversion in readability-implicit-bool-conversion

2018-11-27 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons marked an inline comment as done. malcolm.parsons added inline comments. Comment at: docs/clang-tidy/checks/readability-implicit-bool-conversion.rst:77-78 -- boolean expression/literal to integer, +- boolean expression/literal to integer (conversion from boolea

[PATCH] D54941: [clang-tidy] Ignore bool -> single bit bitfield conversion in readability-implicit-bool-conversion

2018-11-27 Thread Malcolm Parsons via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL347671: [clang-tidy] Ignore bool -> single bit bitfield conversion in readability… (authored by malcolm.parsons, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM CHANGES SIN

[PATCH] D57665: [clang-tidy] Handle unions with existing default-member-init

2019-02-03 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons created this revision. malcolm.parsons added reviewers: aaron.ballman, alexfh. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. clang-tidy's modernize-use-default-member-init was crashing for unions with an existing default member initializer. Fixes

[PATCH] D57665: [clang-tidy] Handle unions with existing default-member-init

2019-02-04 Thread Malcolm Parsons via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE353092: [clang-tidy] Handle unions with existing default-member-init (authored by malcolm.parsons, committed by ). Changed prior to commit: https://reviews.llvm.org/D57665?vs=184976&id=185141#toc Rep

[PATCH] D57852: [clang-tidy] Don't use assignment for value-initialized enums

2019-02-06 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons created this revision. malcolm.parsons added reviewers: aaron.ballman, alexfh, JonasToth. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. The modernize-use-default-member-init check crashes when trying to create an assignment value for a value-initi

[PATCH] D57852: [clang-tidy] Don't use assignment for value-initialized enums

2019-02-07 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons marked an inline comment as done. malcolm.parsons added a comment. In D57852#1388526 , @JonasToth wrote: > How are the semantics for `enum class` in this case? No enumerators are present in the initialisation or the fixit, so there is no

[PATCH] D57852: [clang-tidy] Don't use assignment for value-initialized enums

2019-02-08 Thread Malcolm Parsons via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE353554: [clang-tidy] Don't use assignment for value-initialized enums (authored by malcolm.parsons, committed by ). Changed prior to commit: https://reviews.llvm.org/D57852?vs=185649&id=186014#toc Re

[PATCH] D57087: [clang-tidy] add OverrideMacro to modernize-use-override check

2019-02-17 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added a comment. Some Clang warnings use PP.getLastMacroWithSpelling() to determine the user's macro automatically. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57087/new/ https://reviews.llvm.org/D57087 ___ cfe-commits ma

[PATCH] D38179: [clang-tidy] Handle unions in modernize-use-equals-default check

2017-09-22 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons created this revision. Herald added subscribers: xazax.hun, JDevlieghere. Do not replace the constructor or destructor of a union with =default if it would be implicitly deleted. Fixes: PR27926 https://reviews.llvm.org/D38179 Files: clang-tidy/modernize/UseEqualsDefaultCheck.

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

2018-04-12 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added a comment. In https://reviews.llvm.org/D43764#1065345, @lebedev.ri wrote: > Third commit introducing linking errors. > Did no buildbot catch those? 2 buildbots sent me an email, but they were unrelated failures. Repository: rL LLVM https://reviews.llvm.org/D43764

[PATCH] D43322: Diagnose cases of "return x" that should be "return std::move(x)" for efficiency

2018-04-12 Thread Malcolm Parsons via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC329914: Diagnose cases of "return x" that should be "return std::move(x)" for efficiency (authored by malcolm.parsons, committed by ). Repository: rC Clang https://reviews.llvm.org/D43322 Files: in

[PATCH] D45611: [analyzer] Fix filename in cross-file HTML report

2018-04-13 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons created this revision. malcolm.parsons added reviewers: george.karpenkov, dcoughlin, vlad.tsyrklevich. Herald added subscribers: cfe-commits, a.sidorin, szepet, xazax.hun. The filename is currently taken from the start of the path, while the line and column are taken from the end o

[PATCH] D45591: Clean carriage returns from lib/ and include/. NFC.

2018-04-13 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons accepted this revision. malcolm.parsons added a comment. This revision is now accepted and ready to land. LGTM. Can't commit right now - using phone. Repository: rC Clang https://reviews.llvm.org/D45591 ___ cfe-commits mailing lis

[PATCH] D45591: Clean carriage returns from lib/ and include/. NFC.

2018-04-16 Thread Malcolm Parsons via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC330112: Clean carriage returns from lib/ and include/. NFC. (authored by malcolm.parsons, committed by ). Repository: rC Clang https://reviews.llvm.org/D45591 Files: lib/AST/ASTDumper.cpp lib/AST/

[PATCH] D45927: [clang-tidy] [modernize-use-auto] Correct way to calculate a type name length for multi-token types

2018-04-22 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added a comment. I think spaces that will be removed should be counted - long long is 9. OTOH, MinTypeNameLength isn't likely to be set high enough for that to matter. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45927 __

[PATCH] D45611: [analyzer] Fix filename in cross-file HTML report

2018-05-01 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons updated this revision to Diff 144742. malcolm.parsons added a comment. Add test Repository: rC Clang https://reviews.llvm.org/D45611 Files: lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp test/Coverage/html-multifile-diagnostics.c Index: test/Coverage/html-multifile-diagnos

[PATCH] D45611: [analyzer] Fix filename in cross-file HTML report

2018-05-02 Thread Malcolm Parsons via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC331361: [analyzer] Fix filename in cross-file HTML report (authored by malcolm.parsons, committed by ). Changed prior to commit: https://reviews.llvm.org/D45611?vs=144742&id=144881#toc Repository: rC

[PATCH] D48845: [Sema] Add fixit for unused lambda captures

2018-07-17 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added a comment. In https://reviews.llvm.org/D48845#1158103, @alexshap wrote: > I'm kind of interested in this fixit, but one thought which i have - probably > it should be more conservative (i.e. fix captures by reference, integral > types, etc) (since the code might rely on si

[PATCH] D48845: [Sema] Add fixit for unused lambda captures

2018-07-17 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added inline comments. Comment at: lib/Sema/SemaLambda.cpp:1567 +if (!CurHasPreviousCapture && !IsLast) { + // If there are no captures preceding this capture, remove the + // following comma. In clang-tidy ch

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

2019-10-29 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added a comment. I like that this check warns about copy constructors that don't copy. The warning can be suppressed with a NOLINT comment if not copying is intentional. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69145/new/ https

[PATCH] D30610: [clang-tidy] Added options to cppcoreguidelines-special-member-functions check

2017-11-24 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added a comment. I'd like an `AllowDeletedCopyFunctions` option that allows move and destructor functions to be missing when copying is disabled. struct A { A(const A&) = delete; A& operator=(const A&) = delete; } https://reviews.llvm.org/D30610 _

[PATCH] D30610: [clang-tidy] Added options to cppcoreguidelines-special-member-functions check

2017-11-24 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added a comment. In https://reviews.llvm.org/D30610#934617, @aaron.ballman wrote: > In https://reviews.llvm.org/D30610#934452, @malcolm.parsons wrote: > > > I'd like an `AllowDeletedCopyFunctions` option that allows move and > > destructor functions to be missing when copying is

[PATCH] D30610: [clang-tidy] Added options to cppcoreguidelines-special-member-functions check

2017-11-24 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added a comment. I have 1000s of warnings from this check. A lot of them are about google tests: warning: class 'Foo_Bar_Test' defines a copy constructor and a copy assignment operator but does not define a destructor [cppcoreguidelines-special-member-functions] I'd like an opti

[PATCH] D38171: [clang-tidy] Implement clang-tidy check aliases

2017-11-26 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added a comment. In https://reviews.llvm.org/D38171#927045, @alexfh wrote: > The only place I can think of, where -checks=* is useful is in combination > with -list-checks, where the presence of clang-diagnostic- entries would be > desired anyway. Can we replace -list-checks w

[PATCH] D40505: [clang-tidy] Ignore ExprWithCleanups when looking for else-after-throw

2017-11-27 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons created this revision. Herald added subscribers: cfe-commits, xazax.hun, klimek. The readability-else-after-return check was not warning about an else after a throw of an exception that had arguments that needed to be cleaned up. Repository: rCTE Clang Tools Extra https://revi

[PATCH] D40505: [clang-tidy] Ignore ExprWithCleanups when looking for else-after-throw

2017-11-27 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added a comment. In https://reviews.llvm.org/D40505#936170, @lebedev.ri wrote: > How about also matching on call to functions with no-return attribute? Great idea! But I'll keep this patch for just the throw bugfix. Repository: rCTE Clang Tools Extra https://reviews.llvm.o

[PATCH] D40505: [clang-tidy] Ignore ExprWithCleanups when looking for else-after-throw

2017-11-28 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons updated this revision to Diff 124566. malcolm.parsons added a comment. clang-format Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40505 Files: clang-tidy/readability/ElseAfterReturnCheck.cpp test/clang-tidy/readability-else-after-return.cpp Index: test/cl

[PATCH] D40505: [clang-tidy] Ignore ExprWithCleanups when looking for else-after-throw

2017-11-28 Thread Malcolm Parsons via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE319174: [clang-tidy] Ignore ExprWithCleanups when looking for else-after-throw (authored by malcolm.parsons). Changed prior to commit: https://reviews.llvm.org/D40505?vs=124566&id=124567#toc Reposito

[PATCH] D36672: [clang-tidy] readability-non-const-parameter: fixit on all function declarations

2017-12-05 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added a comment. Fixed by https://reviews.llvm.org/rL319021? Repository: rL LLVM https://reviews.llvm.org/D36672 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41016: [Sema] Fix crash in unused-lambda-capture warning for VLAs

2017-12-08 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons created this revision. Herald added a subscriber: cfe-commits. Clang was crashing when diagnosing an unused-lambda-capture for a VLA because From.getVariable() is null for the capture of a VLA bound. Warning about the VLA bound capture is not helpful, so only warn for the VLA itsel

[PATCH] D41016: [Sema] Fix crash in unused-lambda-capture warning for VLAs

2017-12-11 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons updated this revision to Diff 126394. malcolm.parsons marked an inline comment as done. malcolm.parsons added a comment. Add assert. Repository: rC Clang https://reviews.llvm.org/D41016 Files: include/clang/Sema/ScopeInfo.h lib/Sema/SemaLambda.cpp test/SemaCXX/warn-unus

[PATCH] D41016: [Sema] Fix crash in unused-lambda-capture warning for VLAs

2017-12-11 Thread Malcolm Parsons via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL320396: [Sema] Fix crash in unused-lambda-capture warning for VLAs (authored by malcolm.parsons). Repository: rL LLVM https://reviews.llvm.org/D41016 Files: cfe/trunk/include/clang/Sema/ScopeInfo.h

[PATCH] D41016: [Sema] Fix crash in unused-lambda-capture warning for VLAs

2017-12-11 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added inline comments. Comment at: lib/Sema/SemaLambda.cpp:1491 else diag << From.getVariable(); diag << From.isNonODRUsed(); dim wrote: > Just for sanity's sake, I would still put an assert here, that > `getVariable()` does not return

[PATCH] D36016: [clang-tidy] Support initializer-list constructor cases in modernize-make-unique.

2017-08-03 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added inline comments. Comment at: test/clang-tidy/modernize-make-unique.cpp:291 + + // Initialization with ordinary constructor. + std::unique_ptr PG3 = std::unique_ptr(new G{1, 2}); The comment doesn't match the test? https://reviews.llvm.or

[PATCH] D36051: Move from a long list of checkers to tables

2019-12-31 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:205 + `modernize-make-shared `_, "Yes", "low" + `modernize-make-unique `_, , "low" + `modernize-pass-by-value `_, "Yes", "low" modernize-make-unique offers

[PATCH] D72630: [clang-tidy] Ignore implicit casts in modernize-use-default-member-init

2020-01-13 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons created this revision. malcolm.parsons added reviewers: aaron.ballman, alexfh, JonasToth. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. Initialising a pointer from nullptr involves an implicit cast. Ignore it after getting initialiser from InitLis

[PATCH] D72630: [clang-tidy] Ignore implicit casts in modernize-use-default-member-init

2020-01-13 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons marked 2 inline comments as done. malcolm.parsons added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp:294 int e3 = {5}; - int e4 = 5; + int e4{5}; int e5 = -5; JonasToth wrot

[PATCH] D72630: [clang-tidy] Ignore implicit casts in modernize-use-default-member-init

2020-01-14 Thread Malcolm Parsons via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. malcolm.parsons marked an inline comment as done. Closed by commit rG45924eb46716: [clang-tidy] Ignore implicit casts in modernize-use-default-member-init (authored by malcolm.parsons). Repository: rG LLVM Github Monorepo

[PATCH] D72691: [clang-tidy] Match InitListExpr in modernize-use-default-member-init

2020-01-14 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons created this revision. malcolm.parsons added reviewers: aaron.ballman, alexfh, JonasToth. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. modernize-use-default-member-init wasn't warning about redundant initialisers when the initialiser was an InitL

[PATCH] D72691: [clang-tidy] Match InitListExpr in modernize-use-default-member-init

2020-01-14 Thread Malcolm Parsons via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG9738c757bd9b: [clang-tidy] Match InitListExpr in modernize-use-default-member-init (authored by malcolm.parsons). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D93333: tidy: supress warning for default member funcs

2020-12-17 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons accepted this revision as: malcolm.parsons. malcolm.parsons added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D9/new/ https://reviews.llvm.org/D9 ___

[PATCH] D93333: [clang-tidy] ppcoreguidelines-pro-type-member-init: suppress warning for default member funcs

2020-12-20 Thread Malcolm Parsons via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG35f2c3a8b41f: [clang-tidy] cppcoreguidelines-pro-type-member-init: suppress warning for… (authored by cwarner-8702, committed by malcolm.parsons).

[PATCH] D102313: [docs] Fix documentation for bugprone-dangling-handle

2021-05-11 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons created this revision. malcolm.parsons added a reviewer: aaron.ballman. malcolm.parsons requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. string_view isn't experimental anymore. This check has always handled both f

[PATCH] D102313: [docs] Fix documentation for bugprone-dangling-handle

2021-05-12 Thread Malcolm Parsons via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG5389a05836e7: [docs] Fix documentation for bugprone-dangling-handle (authored by malcolm.parsons). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102313/new/

[PATCH] D70144: clang-tidy: modernize-use-equals-default avoid adding redundant semicolons

2019-11-14 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added a reviewer: alexfh. malcolm.parsons added a comment. Should `clang::format::cleanupAroundReplacements()` handle this instead? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70144/new/ https://reviews.llvm.org/D70144 _

[PATCH] D71846: [ASTMatchers] Fix for https://bugs.llvm.org/show_bug.cgi?id=44364

2019-12-24 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added a comment. Does Clang support C++20's range-based for statements with initializer? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71846/new/ https://reviews.llvm.org/D71846 ___ cfe

[PATCH] D72630: [clang-tidy] Ignore implicit casts in modernize-use-default-member-init

2020-01-24 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added a comment. In D72630#1823525 , @hans wrote: > It seems that while this commit got pushed to GitHub, it's not actually part > of master or any other branches. This was my first push to llvm's GitHub repository, so I may have done s

[PATCH] D44295: [clang-tidy] Detects and fixes calls to grand-...parent virtual methods instead of calls to parent's virtual methods

2018-03-22 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added a comment. Please add a test where the parent class is in a differently named namespace. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44295 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists

[PATCH] D44948: Add diagnostic -Waggregate-ctors, "aggregate type has user-declared constructors"

2018-03-28 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added a comment. > If no real code triggers this diagnostic, ... Boost triggers this diagnostic: include/boost/core/noncopyable.hpp:23:9: error: aggregate type has user-declared constructors [-Werror,-Waggregate-ctors] class noncopyable ^ include/boost/iterator

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

2018-04-05 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added a comment. In https://reviews.llvm.org/D43764#1058120, @jdemeule wrote: > Can I help in some way to close this PR? I applied the patch and ran the tests. There was one unexpected failure: [100%] Running the Clang extra tools' regression tests FAIL: Clang Tools :: cla

[PATCH] D45405: [clang-tidy] [modernize-use-auto] Add a threshold for minimal type name length to be replaced with 'auto'

2018-04-07 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added a comment. Please add to release notes. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45405 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2018-04-09 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added a comment. In https://reviews.llvm.org/D43764#1061042, @jdemeule wrote: > I suspect some undefined order when applying replacements as directly > dependent on which order OS reads `file1.yaml`, `file2.yaml`and `file3.yaml`. $ ls -f /mparsons/llvm/llvm-build/tools/clang/t

[PATCH] D45405: [clang-tidy] [modernize-use-auto] Add a threshold for minimal type name length to be replaced with 'auto'

2018-04-09 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons accepted this revision. malcolm.parsons added a comment. This revision is now accepted and ready to land. LGTM! https://reviews.llvm.org/D45405 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mai

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

2018-04-10 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added a comment. In https://reviews.llvm.org/D43764#1062748, @jdemeule wrote: > Ok, I understand the problem. > Previously, during the deduplication step, replacements per file where > sorted and it is not the case anymore. > That means now, clang-apply-replacement is highly de

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

2018-04-11 Thread Malcolm Parsons via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL329813: [clang-apply-replacements] Convert tooling::Replacements to tooling… (authored by malcolm.parsons, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://revie

[PATCH] D27166: [clang-tidy] Enhance modernize-use-auto to templated function casts

2016-12-14 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons updated this revision to Diff 81465. malcolm.parsons added a comment. Add tests for implicit vars and pointers to pointers to auto. https://reviews.llvm.org/D27166 Files: clang-tidy/modernize/UseAutoCheck.cpp docs/ReleaseNotes.rst docs/clang-tidy/checks/modernize-use-auto.

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

2016-12-14 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added a comment. In https://reviews.llvm.org/D26167#621942, @JonasToth wrote: > what do you think about configuration of the allocating functions? e.g. for > aligned memory you must use OS-specific allocation functions. > should the check catch custom allocation functions as wel

[PATCH] D27166: [clang-tidy] Enhance modernize-use-auto to templated function casts

2016-12-15 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons updated this revision to Diff 81540. malcolm.parsons added a comment. Add FIXME comment. https://reviews.llvm.org/D27166 Files: clang-tidy/modernize/UseAutoCheck.cpp docs/ReleaseNotes.rst docs/clang-tidy/checks/modernize-use-auto.rst test/clang-tidy/modernize-use-auto-ca

[PATCH] D27166: [clang-tidy] Enhance modernize-use-auto to templated function casts

2016-12-15 Thread Malcolm Parsons via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL289797: [clang-tidy] Enhance modernize-use-auto to templated function casts (authored by malcolm.parsons). Changed prior to commit: https://reviews.llvm.org/D27166?vs=81540&id=81554#toc Repository: r

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

2016-12-15 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added inline comments. Comment at: clang-tidy/ClangTidy.cpp:296 const auto &RegisteredCheckers = - AnalyzerOptions::getRegisteredCheckers(/*IncludeExperimental=*/false); + AnalyzerOptions::getRegisteredCheckers(/*IncludeExperimental=*/true); bool A

[PATCH] D27767: NFC Changes from modernize-use-auto

2016-12-15 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added inline comments. Comment at: lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp:106 if (Optional P = Succ->getLocationAs()) +if (const auto *BO = P->getStmtAs()) { The check missed this one. https://reviews.llvm.org/D27767

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

2016-12-15 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added a comment. What about arguments 3 and 4 of `std::list::splice(It, list, It, It)`? Comment at: clang-tidy/obvious/ObviousTidyModule.cpp:24 void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { -// Add obvious checks here. +Ch

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

2016-12-15 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added a comment. In https://reviews.llvm.org/D27806#623856, @Prazek wrote: > So does FIXIT comment satisfies you for this patch? Yes. Comment at: clang-tidy/obvious/InvalidRangeCheck.cpp:80 +void InvalidRangeCheck::check(const MatchFinder::MatchResult &Result

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

2016-12-15 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added inline comments. Comment at: test/clang-tidy/misc-invalid-range.cpp:41 + auto &v2 = v; + std::move(v.begin(), v2.end(), v2.begin()); + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: call to algorithm with begin and end from different objects [misc-invalid-

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

2016-12-15 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added inline comments. Comment at: docs/clang-tidy/checks/list.rst:68 misc-inefficient-algorithm + misc-invalid-range misc-macro-parentheses misc. add_new_check.py can fix it. https://reviews.llvm.org/D27806

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

2016-12-15 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added inline comments. Comment at: docs/clang-tidy/checks/list.rst:68 misc-inefficient-algorithm + misc-invalid-range misc-macro-parentheses Prazek wrote: > malcolm.parsons wrote: > > misc. > > add_new_check.py can fix it. > How? I added

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

2016-12-17 Thread Malcolm Parsons via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL290051: [clang-tidy] Remove duplicated check from move-constructor-init (authored by malcolm.parsons). Changed prior to commit: https://reviews.llvm.org/D26453?vs=78867&id=81849#toc Repository: rL LL

[PATCH] D25659: [clang-tidy] Avoid running aliased checks twice

2016-12-18 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons abandoned this revision. malcolm.parsons added a comment. Abandon until we have a plan. https://reviews.llvm.org/D25659 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commit

[PATCH] D26750: [clang-tidy] Add modernize-use-default-member-init check

2016-12-19 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons marked 9 inline comments as done. malcolm.parsons added inline comments. Comment at: clang-tidy/modernize/UseDefaultMemberInitCheck.cpp:77-78 +static bool sameValue(const Expr *E1, const Expr *E2) { + E1 = ignoreUnaryPlus(getInitializer(E1->IgnoreImpCasts())); +

[PATCH] D26750: [clang-tidy] Add modernize-use-default-member-init check

2016-12-19 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added inline comments. Comment at: clang-tidy/modernize/UseDefaultMemberInitCheck.cpp:77-78 +static bool sameValue(const Expr *E1, const Expr *E2) { + E1 = ignoreUnaryPlus(getInitializer(E1->IgnoreImpCasts())); + E2 = ignoreUnaryPlus(getInitializer(E2->IgnoreImp

[PATCH] D26750: [clang-tidy] Add modernize-use-default-member-init check

2016-12-20 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons updated this revision to Diff 82072. malcolm.parsons marked 10 inline comments as done. malcolm.parsons added a comment. Use IgnoreParenImpCasts(). Elide braces. Handle character and string literals. Require C++11. Improve doc. https://reviews.llvm.org/D26750 Files: clang-tidy

[PATCH] D26750: [clang-tidy] Add modernize-use-default-member-init check

2016-12-20 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added inline comments. Comment at: clang-tidy/modernize/UseDefaultMemberInitCheck.cpp:33-34 + case Type::STK_Floating: + case Type::STK_IntegralComplex: + case Type::STK_FloatingComplex: +return "0.0"; aaron.ballman wrote: > Do these requir

[PATCH] D26750: [clang-tidy] Add modernize-use-default-member-init check

2016-12-20 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons updated this revision to Diff 82073. malcolm.parsons marked an inline comment as done. malcolm.parsons added a comment. Use 0 for IntegralComplex. https://reviews.llvm.org/D26750 Files: clang-tidy/modernize/CMakeLists.txt clang-tidy/modernize/ModernizeTidyModule.cpp clang-

[PATCH] D26750: [clang-tidy] Add modernize-use-default-member-init check

2016-12-20 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added inline comments. Comment at: clang-tidy/modernize/UseDefaultMemberInitCheck.cpp:34 + case Type::STK_IntegralComplex: +return InitType->isCharType() ? "'\\0'" : "0"; + case Type::STK_Floating: aaron.ballman wrote: > This is incorrect if

[PATCH] D26750: [clang-tidy] Add modernize-use-default-member-init check

2016-12-20 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added inline comments. Comment at: clang-tidy/modernize/UseDefaultMemberInitCheck.cpp:34 + case Type::STK_IntegralComplex: +return InitType->isCharType() ? "'\\0'" : "0"; + case Type::STK_Floating: aaron.ballman wrote: > malcolm.parsons wrot

[PATCH] D26750: [clang-tidy] Add modernize-use-default-member-init check

2016-12-20 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons updated this revision to Diff 82113. malcolm.parsons added a comment. Use character literal prefixes. Use float literal suffix. https://reviews.llvm.org/D26750 Files: clang-tidy/modernize/CMakeLists.txt clang-tidy/modernize/ModernizeTidyModule.cpp clang-tidy/modernize/UseD

  1   2   3   >