Re: [PATCH] D18852: [clang-tidy] fix a crash with -fdelayed-template-parsing in UnnecessaryValueParamCheck.

2016-04-06 Thread Richard via cfe-commits
LegalizeAdulthood added a subscriber: LegalizeAdulthood. LegalizeAdulthood added a comment. That we've had to fix this twice now tells me that our collective memory is forgetful :). We need to collect the community wisdom for `clang-tidy` gotchas somewhere in the docs...

Re: [PATCH] D18783: [clang-tidy] add new checker for string literal with NUL character.

2016-04-06 Thread Richard via cfe-commits
LegalizeAdulthood added a subscriber: LegalizeAdulthood. LegalizeAdulthood added a comment. There are some APIs where a series of strings are passed as a single `char *` with NUL separators between the strings and the last string is indicated by two NUL characters in a row. For instance, see

Re: [clang-tools-extra] r264563 - clang-tidy: Fix broken buildbot

2016-04-05 Thread Richard via cfe-commits
In article , Alexander Kornienko writes: > > On Mon, Mar 28, 2016 at 6:15 AM, Richard Thomson via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > > > Author: legalize > > Date: Sun Mar 27 23:15:41

Re: [PATCH] D17482: [clang-tidy] Allow tests to verify changes made to header files

2016-04-04 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. In http://reviews.llvm.org/D17482#390237, @alexfh wrote: > My main concern is that this change makes the already complicated and > unobvious testing mechanism [...] The complexity and obtuseness of the existing testing mechanism is unrelated to this

Re: [PATCH] D17482: [clang-tidy] Allow tests to verify changes made to header files

2016-04-04 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. As it stands currently, you can't commit a header with `CHECK-MESSAGES` and `CHECK-FIXES` lines and have them verified. That's the whole point of this changeset. Currently you have to do something very hacky in order to verify changes made to headers.

Re: [PATCH] D18509: clang-tidy: add_new_check.py stubs out release notes

2016-04-04 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. In http://reviews.llvm.org/D18509#388071, @hintonda wrote: > With this change, won't you need to update 2 different repos: clang and > extra? Is it possible to update both with a single Phabricator patch? This is updating the release notes in the extra

Re: [PATCH] D18608: note for top-level consts in function decls tidy

2016-03-30 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: docs/ReleaseNotes.rst:153-154 @@ -152,2 +152,4 @@ + * `readability-avoid-const-params-in-decls +

Re: [PATCH] D18582: [Clang-tidy] Update release notes with list of checks added since 3.8

2016-03-30 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: docs/ReleaseNotes.rst:143 @@ -71,3 +142,3 @@ Improvements to ``modularize`` ^^ LegalizeAdulthood wrote: > The formatting here has been changed on trunk. Please rebase with a

Re: [PATCH] D18608: note for top-level consts in function decls tidy

2016-03-30 Thread Richard via cfe-commits
LegalizeAdulthood added a subscriber: LegalizeAdulthood. Comment at: docs/ReleaseNotes.rst:153-154 @@ -152,2 +152,4 @@ + * `readability-avoid-const-params-in-decls +

Re: [PATCH] D17482: [clang-tidy] Allow tests to verify changes made to header files

2016-03-30 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. I'm sorry to be such a nag and I know you're busy, but This changeset has been held up for a month and a half. (6 weeks since originally posted in http://reviews.llvm.org/D16953) The change isn't that complicated and there haven't really been any comments

Re: [PATCH] D18509: clang-tidy: add_new_check.py stubs out release notes

2016-03-30 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tidy/add_new_check.py:283 @@ +282,3 @@ + elif not clang_tidy_heading_found: +if line.startswith('^^'): + clang_tidy_heading_found = True Alex changed the

Re: [PATCH] D18584: Complete support for C++ Core Guidelines Type.6: Always initialize a member variable.

2016-03-30 Thread Richard via cfe-commits
LegalizeAdulthood added a subscriber: LegalizeAdulthood. LegalizeAdulthood added a comment. Can you summarize the improvements in `docs/ReleaseNotes.rst`, please? http://reviews.llvm.org/D18584 ___ cfe-commits mailing list

Re: [PATCH] D18582: [Clang-tidy] Update release notes with list of checks added since 3.8

2016-03-30 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: docs/ReleaseNotes.rst:143 @@ -71,3 +142,3 @@ Improvements to ``modularize`` ^^ The formatting here has been changed on trunk. Please rebase with a full file context so we can

Re: [PATCH] D18475: [clang-tidy] Add more detection rules for redundant c_str calls.

2016-03-30 Thread Richard via cfe-commits
LegalizeAdulthood added a subscriber: LegalizeAdulthood. LegalizeAdulthood added a comment. Can you summarize the improvements in `docs/ReleaseNotes.rst` please? http://reviews.llvm.org/D18475 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D18457: [clang-tidy] Add a new checker to detect missing comma in initializer list.

2016-03-30 Thread Richard via cfe-commits
LegalizeAdulthood added a subscriber: LegalizeAdulthood. LegalizeAdulthood added a comment. Can you summarize this check in `docs/ReleaseNotes.rst` in the clang-tidy section, please? http://reviews.llvm.org/D18457 ___ cfe-commits mailing list

Re: [PATCH] D18408: readability check for const params in declarations

2016-03-30 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. Maybe you need to rebase first? I haven't used arc. Repository: rL LLVM http://reviews.llvm.org/D18408 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D18136: boost-use-to-string check

2016-03-30 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. Please summarize this check in `docs/ReleaseNotes.rst` in the clang-tidy section. http://reviews.llvm.org/D18136 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D18396: Clang-tidy:modernize-use-override. Fix for __declspec attributes and const=0 without spacse

2016-03-30 Thread Richard via cfe-commits
LegalizeAdulthood added a subscriber: LegalizeAdulthood. LegalizeAdulthood added a comment. Please summarize the improvements in `docs/ReleaseNotes.rst` in the clang tidy section. http://reviews.llvm.org/D18396 ___ cfe-commits mailing list

Re: [PATCH] D18265: [clang-tidy] New: checker misc-assign-operator-return

2016-03-30 Thread Richard via cfe-commits
LegalizeAdulthood added a subscriber: LegalizeAdulthood. LegalizeAdulthood added a comment. Please summarize this check in `docs/ReleaseNotes.rst`. There is a pending review that will stub this out automatically, but it hasn't been approved yet http://reviews.llvm.org/D18265

Re: [PATCH] D18408: readability check for const params in declarations

2016-03-30 Thread Richard via cfe-commits
LegalizeAdulthood added a subscriber: LegalizeAdulthood. LegalizeAdulthood added a comment. You can submit the release notes changes in a new patch. Repository: rL LLVM http://reviews.llvm.org/D18408 ___ cfe-commits mailing list

Re: [PATCH] D17811: [clang-tidy] Add check to detect dangling references in value handlers.

2016-03-30 Thread Richard via cfe-commits
LegalizeAdulthood added a subscriber: LegalizeAdulthood. LegalizeAdulthood added a comment. Can you add a synopsis of this new check to `docs/ReleaseNotes.rst` please? Repository: rL LLVM http://reviews.llvm.org/D17811 ___ cfe-commits mailing

Re: [PATCH] D7982: Add readability-duplicate-include check to clang-tidy

2016-03-29 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tidy/readability/DuplicateIncludeCheck.cpp:62 @@ +61,3 @@ +StringRef SearchPath, StringRef RelativePath, const Module *Imported) { + if (!SM_.isInMainFile(HashLoc)) { +return; LegalizeAdulthood

Re: [PATCH] D17491: Add performance check to flag function parameters of expensive to copy types that can be safely converted to const references.

2016-03-28 Thread Richard via cfe-commits
LegalizeAdulthood added a subscriber: LegalizeAdulthood. LegalizeAdulthood added a comment. Can you add something to the docs/ReleaseNotes.rst for this new check, please? Repository: rL LLVM http://reviews.llvm.org/D17491 ___ cfe-commits mailing

Re: [PATCH] D17482: [clang-tidy] Allow tests to verify changes made to header files

2016-03-28 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. Squeak http://reviews.llvm.org/D17482 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D18509: clang-tidy: add_new_check.py stubs out release notes

2016-03-27 Thread Richard via cfe-commits
LegalizeAdulthood created this revision. LegalizeAdulthood added a reviewer: alexfh. LegalizeAdulthood added a subscriber: cfe-commits. Update `add_new_check.py` to stub out information in the release notes for the new check. http://reviews.llvm.org/D18509 Files: clang-tidy/add_new_check.py

Re: [PATCH] D16529: [clang-tidy] Add modernize-raw-string-literal check

2016-03-27 Thread Richard via cfe-commits
LegalizeAdulthood updated this revision to Diff 51738. LegalizeAdulthood added a comment. Remove brace initializers from test code. http://reviews.llvm.org/D16529 Files: clang-tidy/modernize/CMakeLists.txt clang-tidy/modernize/ModernizeTidyModule.cpp

Re: [PATCH] D16529: [clang-tidy] Add modernize-raw-string-literal check

2016-03-26 Thread Richard via cfe-commits
LegalizeAdulthood updated this revision to Diff 51727. LegalizeAdulthood added a comment. Update release notes http://reviews.llvm.org/D16529 Files: clang-tidy/modernize/CMakeLists.txt clang-tidy/modernize/ModernizeTidyModule.cpp clang-tidy/modernize/RawStringLiteralCheck.cpp

Re: [PATCH] D15332: new clang-tidy checker readability-non-const-parameter

2016-03-22 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: test/clang-tidy/readability-non-const-parameter.cpp:116-134 @@ +115,21 @@ + +// CHECK-MESSAGES: :[[@LINE+1]]:18: warning: parameter 'p' can be const +int return1(int *p) { + // CHECK-FIXES: {{^}}int return1(const int *p)

Re: [PATCH] D18191: [clang-tidy] Add check for function parameters that are const& to builtin types

2016-03-19 Thread Richard via cfe-commits
LegalizeAdulthood added a subscriber: LegalizeAdulthood. LegalizeAdulthood added a comment. There is utility in the definition of a function in saying that an argument is `const int i` instead of `int i`. The const-ness declares the intent that this local variable is not going to be modified.

[PATCH] D18263: [clang-tools-extra] Add release notes documentation

2016-03-19 Thread Richard via cfe-commits
LegalizeAdulthood created this revision. LegalizeAdulthood added a reviewer: alexfh. LegalizeAdulthood added a subscriber: cfe-commits. Add a file for Extra Clang Tools release notes, based on 3.9 release notes file for Clang. http://reviews.llvm.org/D18263 Files: docs/ReleaseNotes.rst

Re: [PATCH] D18263: [clang-tools-extra] Add release notes documentation

2016-03-19 Thread Richard via cfe-commits
LegalizeAdulthood updated this revision to Diff 51013. LegalizeAdulthood added a comment. Sort check names in list http://reviews.llvm.org/D18263 Files: docs/ReleaseNotes.rst docs/clang-tidy/checks/list.rst docs/index.rst Index: docs/index.rst

Re: [PATCH] D18136: boost-use-to-string check

2016-03-19 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tidy/boost/UseToStringCheck.cpp:56 @@ +55,3 @@ + const auto *MatchedToString = Result.Nodes.getNodeAs("to_string"); + auto Q = Result.Nodes.getNodeAs("char_type")->getAsType(); + Q seems pretty obtuse

Re: [PATCH] D18191: [clang-tidy] Add check for function parameters that are const& to builtin types

2016-03-19 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: test/clang-tidy/misc-const-ref-builtin.cpp:32-34 @@ +31,4 @@ + +void f(const int& i, const int& j); +// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: const reference to 'int' at parameter 'i' in function 'f'

Re: [PATCH] D18238: [clang-tidy] Fix clang-tidy crashes when using -fdelayed-template-parsing.

2016-03-19 Thread Richard via cfe-commits
LegalizeAdulthood added a subscriber: LegalizeAdulthood. LegalizeAdulthood added a comment. LGTM http://reviews.llvm.org/D18238 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17482: [clang-tidy] Allow tests to verify changes made to header files

2016-03-19 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. In http://reviews.llvm.org/D17482#372677, @alexfh wrote: > Sorry for leaving this without attention for a while. > > I'm somewhat concerned about this change. It's adding a certain level of > complexity, but doesn't cover some less trivial cases like handling

Re: [PATCH] D16529: [clang-tidy] Add modernize-raw-string-literal check

2016-03-19 Thread Richard via cfe-commits
LegalizeAdulthood updated this revision to Diff 51105. LegalizeAdulthood marked an inline comment as done. LegalizeAdulthood added a comment. Update from comments. Update documentation to reflect changes in the implementation. I do not have commit access. Patch by Richard Thomson

Re: [PATCH] D17482: [clang-tidy] Allow tests to verify changes made to header files

2016-03-18 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. In http://reviews.llvm.org/D17482#372677, @alexfh wrote: > Can you [...] say how many times this feature is going to be used, if it gets > added to the testing script? Since you can't verify changes against headers right now, noone has bothered to add test

Re: [PATCH] D17482: [clang-tidy] Allow tests to verify changes made to header files

2016-03-18 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. test/clang-tidy/misc-unused-parameters.cpp currently does a hack to verify headers by committing the manually fixed header and diffing it: // RUN: echo "static void staticFunctionHeader(int i) {}" > %T/header.h // RUN: echo "static void

Re: [PATCH] D18263: [clang-tools-extra] Add release notes documentation

2016-03-18 Thread Richard via cfe-commits
LegalizeAdulthood updated this revision to Diff 51102. LegalizeAdulthood marked 2 inline comments as done. LegalizeAdulthood added a comment. Update from review comments. I do not have commit access. Patch by Richard Thomson http://reviews.llvm.org/D18263 Files: docs/ReleaseNotes.rst

Re: [PATCH] D18191: [clang-tidy] Add check for function parameters that are const& to builtin types

2016-03-18 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. In http://reviews.llvm.org/D18191#377686, @sdowney wrote: > In http://reviews.llvm.org/D18191#376471, @LegalizeAdulthood wrote: > > > There is utility in the definition of a function in saying that an argument > > is `const int i` instead of `int i`. The

Re: [PATCH] D17482: [clang-tidy] Allow tests to verify changes made to header files

2016-03-10 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. Another week without reviews... These changes were already split from a previous changeset without being reviewed properly. Squeak squeak squeak. http://reviews.llvm.org/D17482 ___ cfe-commits mailing list

Re: [PATCH] D16529: [clang-tidy] Add modernize-raw-string-literal check

2016-03-10 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. I do not have commit access, so I will need someone to commit this for me. Patch by Richard Thomson thanks. Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:82 @@ +81,3 @@ +? std::string{R"lit()")lit"} +

Re: [PATCH] D7982: Add readability-duplicate-include check to clang-tidy

2016-03-02 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. I need to update from review comments and upload a new diff. http://reviews.llvm.org/D7982 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D10013: Refactor: Simplify boolean conditional return statements in lib/Driver

2016-03-02 Thread Richard via cfe-commits
LegalizeAdulthood abandoned this revision. LegalizeAdulthood added a comment. Discarding due to inaction by reviewers. http://reviews.llvm.org/D10013 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D16529: [clang-tidy] Add modernize-raw-string-literal check

2016-03-02 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. Squeak. This has been waiting on review for over two weeks http://reviews.llvm.org/D16529 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17482: [clang-tidy] Allow tests to verify changes made to header files

2016-03-01 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. Squeak http://reviews.llvm.org/D17482 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D16535: [clang-tidy] Check to find unintended semicolons that changes the semantics.

2016-02-26 Thread Richard via cfe-commits
LegalizeAdulthood added a subscriber: LegalizeAdulthood. Comment at: docs/clang-tidy/checks/misc-suspicious-semicolon.rst:48-49 @@ +47,4 @@ + +while(readWhitespace()); + Token t = readNextToken(); + In this documentation, can we please format code so

Re: [PATCH] D17446: ASTMatchers: add new statement/decl matchers

2016-02-22 Thread Richard via cfe-commits
LegalizeAdulthood added a subscriber: LegalizeAdulthood. Comment at: docs/LibASTMatchersReference.html:512 @@ +511,3 @@

Re: [PATCH] D17484: [clang-tidy] introduce modernize-deprecated-headers check

2016-02-22 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. In http://reviews.llvm.org/D17484#358617, @alexfh wrote: > Thank you for this check! Mostly looks good, but there are a number of style > nits. > > The most important question to this check is that the standard doesn't > guarantee that the C++ headers declare

Re: [PATCH] D17484: [clang-tidy] introduce modernize-deprecated-headers check

2016-02-20 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. Other than a few nits, LGTM. Comment at: clang-tidy/modernize/DeprecatedHeadersCheck.cpp:54 @@ +53,3 @@ +: Check(Check), LangOpts(LangOpts) { + CStyledHeaderToCxx["assert.h"] = "cassert"; + CStyledHeaderToCxx["float.h"] = "cfloat";

[PATCH] D17482: [clang-tidy] Allow tests to verify changes made to header files

2016-02-20 Thread Richard via cfe-commits
LegalizeAdulthood created this revision. LegalizeAdulthood added a reviewer: alexfh. LegalizeAdulthood added a subscriber: cfe-commits. This changeset improves check_clang_tidy.py to allow chnages made by a check to a header to be verified. A new argument `--header-filter` allows the test file

Re: [PATCH] D10013: Refactor: Simplify boolean conditional return statements in lib/Driver

2016-02-19 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. I do not have commit access, so someone else will need to commit this. Let me know if it needs rebasing. Patch by Richard Thomson. http://reviews.llvm.org/D10013 ___ cfe-commits mailing list

Re: [PATCH] D16012: Carry raw string literal information through to the AST StringLiteral representation

2016-02-18 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. In http://reviews.llvm.org/D16012#351827, @rsmith wrote: > I'm definitely sympathetic to making `StringLiteralParser` expose information > [...] I was unaware of this class; so far I have only studied the classes appearing in the AST. I did notice that the

Re: [PATCH] D15332: new clang-tidy checker readability-non-const-parameter

2016-02-17 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: test/clang-tidy/readability-non-const-parameter.cpp:3 @@ +2,3 @@ + +// Currently the checker only warns about pointer arguments. +// danielmarjamaki wrote: > LegalizeAdulthood wrote: > > How hard is it to

Re: [PATCH] D15332: new clang-tidy checker readability-non-const-parameter

2016-02-15 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: test/clang-tidy/readability-non-const-parameter.cpp:3 @@ +2,3 @@ + +// Currently the checker only warns about pointer arguments. +// How hard is it to extend it to references? Certainly the confusion about

Re: [PATCH] D15332: new clang-tidy checker readability-non-const-parameter

2016-02-15 Thread Richard via cfe-commits
LegalizeAdulthood added a subscriber: LegalizeAdulthood. Comment at: docs/clang-tidy/checks/readability-non-const-parameter.rst:15 @@ +14,3 @@ + + // warning here; p should be const + char f1(char *p) { With pointers, there are always two layers of constness:

Re: [PATCH] D17244: [clang-tidy] readability-ternary-operator new check

2016-02-15 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: docs/clang-tidy/checks/readability-ternary-operator.rst:10 @@ +9,3 @@ +And it gets transformed into:: + (condition ? expression0 : expression1); + Why add the parentheses around the entire statement? They

Re: [PATCH] D16529: [clang-tidy] Add modernize-raw-string-literal check

2016-02-13 Thread Richard via cfe-commits
LegalizeAdulthood updated this revision to Diff 47927. LegalizeAdulthood marked 4 inline comments as done. LegalizeAdulthood added a comment. Update from review comments. Avoid some string concatenation when delimiter is empty. http://reviews.llvm.org/D16529 Files:

Re: [PATCH] D16529: [clang-tidy] Add modernize-raw-string-literal check

2016-02-13 Thread Richard via cfe-commits
LegalizeAdulthood updated this revision to Diff 47929. LegalizeAdulthood marked an inline comment as done. LegalizeAdulthood added a comment. Add FIXME for macro argument test case. http://reviews.llvm.org/D16529 Files: clang-tidy/modernize/CMakeLists.txt

Re: [PATCH] D16529: [clang-tidy] Add modernize-raw-string-literal check

2016-02-12 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:97 @@ +96,3 @@ +} + +} // namespace alexfh wrote: > > I believe we agree on the following: ... > > Yes. > > > Where we seem to disagree is what algorithm should

Re: [PATCH] D16953: Enhance clang-tidy modernize-redundant-void-arg check to apply fixes to header files

2016-02-11 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. Squeak? http://reviews.llvm.org/D16953 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D16529: [clang-tidy] Add modernize-raw-string-literal check

2016-02-11 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. Squeak? http://reviews.llvm.org/D16529 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D16308: clang-tidy Enhance readability-simplify-boolean-expr check to handle implicit conversions of integral types to bool and member pointers

2016-02-11 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. If someone could review the changes and commit this corrected version for me, that would be great. Patch by Richard Thomson. http://reviews.llvm.org/D16308 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D16310: new clang-tidy checker misc-long-cast

2016-02-10 Thread Richard via cfe-commits
In article , Daniel Marjamäki writes: > > You can get overflow with / and %. Consider: > > > > int i = INT_MIN / -1; // similar for % > > you are right, I did not think of that. > > imho overflow

Re: [PATCH] D16308: clang-tidy Enhance readability-simplify-boolean-expr check to handle implicit conversions of integral types to bool and member pointers

2016-02-09 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. In article , Aaron Ballman writes: > aaron.ballman added a comment. > > This is causing build bot failures: > >

Re: [PATCH] D16953: Enhance clang-tidy modernize-redundant-void-arg check to apply fixes to header files

2016-02-09 Thread Richard via cfe-commits
LegalizeAdulthood marked an inline comment as done. Comment at: test/clang-tidy/check_clang_tidy.py:152 @@ -68,4 +151,3 @@ - has_check_fixes = input_text.find('CHECK-FIXES') >= 0 - has_check_messages = input_text.find('CHECK-MESSAGES') >= 0 + has_check_fixes,

Re: [PATCH] D16308: clang-tidy Enhance readability-simplify-boolean-expr check to handle implicit conversions of integral types to bool and member pointers

2016-02-09 Thread Richard via cfe-commits
In article , Aaron Ballman writes: > aaron.ballman added a comment. > > This is causing build bot failures: > > http://bb.pgr.jp/builders/cmake-clang-tools-x86_64-linux/builds/23351 >

Re: [PATCH] D16953: Enhance clang-tidy modernize-redundant-void-arg check to apply fixes to header files

2016-02-09 Thread Richard via cfe-commits
LegalizeAdulthood updated this revision to Diff 47319. LegalizeAdulthood added a comment. Update from review comments http://reviews.llvm.org/D16953 Files: clang-tidy/modernize/RedundantVoidArgCheck.cpp test/clang-tidy/check_clang_tidy.py test/clang-tidy/modernize-redundant-void-arg.cpp

Re: [PATCH] D16308: clang-tidy Enhance readability-simplify-boolean-expr check to handle implicit conversions of integral types to bool and member pointers

2016-02-09 Thread Richard via cfe-commits
LegalizeAdulthood updated this revision to Diff 47414. LegalizeAdulthood added a comment. Fixes StringRef problem that crashes tests in release builds only. http://reviews.llvm.org/D16308 Files: clang-tidy/readability/SimplifyBooleanExprCheck.cpp

Re: [PATCH] D16310: new clang-tidy checker misc-long-cast

2016-02-09 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tools-extra/trunk/clang-tidy/misc/MisplacedWideningCastCheck.cpp:21-23 @@ +20,5 @@ +void MisplacedWideningCastCheck::registerMatchers(MatchFinder *Finder) { + auto Calc = expr(anyOf(binaryOperator(anyOf( +

Re: [PATCH] D16308: clang-tidy Enhance readability-simplify-boolean-expr check to handle implicit conversions of integral types to bool and member pointers

2016-02-09 Thread Richard via cfe-commits
[Please reply *only* to the list and do not include my email directly in the To: or Cc: of your reply; otherwise I will not see your reply. Thanks.] In article <e1ata3p-0002am...@shell.xmission.com>, Richard via cfe-commits <cfe-commits@lists.llvm.org> writes: > make check-al

Re: [PATCH] D16529: [clang-tidy] Add modernize-raw-string-literal check

2016-02-08 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. I wrote: > Repeated searching of the string for the literal delimiter could be avoided > by changing the routine to search for the delimiter. If present, it could > examine the characters following the literal to make the literal more unique > and then

Re: [PATCH] D16953: Enhance clang-tidy modernize-redundant-void-arg check to apply fixes to header files

2016-02-08 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: test/clang-tidy/check_clang_tidy.py:122 @@ -40,2 +121,3 @@ parser.add_argument('-resource-dir') + parser.add_argument('--header-filter') parser.add_argument('input_file_name') alexfh wrote: > There's no

Re: [PATCH] D16529: [clang-tidy] Add modernize-raw-string-literal check

2016-02-07 Thread Richard via cfe-commits
LegalizeAdulthood marked 5 inline comments as done. Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:96 @@ +95,3 @@ + std::string Delimiter; + for (int Counter = 0; containsDelimiter(Bytes, Delimiter); ++Counter) { +Delimiter = (Counter == 0) ? "lit" : "lit" +

Re: [PATCH] D16308: clang-tidy Enhance readability-simplify-boolean-expr check to handle implicit conversions of integral types to bool and member pointers

2016-02-07 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. It's been almost a week. Is there some reason this hasn't been committed yet? http://reviews.llvm.org/D16308 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D16953: Enhance clang-tidy modernize-redundant-void-arg check to apply fixes to header files

2016-02-06 Thread Richard via cfe-commits
LegalizeAdulthood created this revision. LegalizeAdulthood added a reviewer: alexfh. LegalizeAdulthood added a subscriber: cfe-commits. Update `check_clang_tidy.py` to handle fixes applied to header files by adding `--header-filter` argument that: - Passes `-header-filter` down to `clang-tidy` -

Re: [PATCH] D16529: [clang-tidy] Add modernize-raw-string-literal check

2016-02-04 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. I agree it needs more testing. I think also my current approach to newlines is overly aggressive and will result in more raw string literals than people would prefer. It's really the Windows style path separators and regex ones that are not controversial

Re: [PATCH] D16700: [Clang-tidy] Make null pointer literals for fixes configurable for two checks

2016-02-03 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tidy/readability/ImplicitBoolCastCheck.h:32 @@ -30,1 +31,3 @@ +Options.get("NullPointerLiteral", +Context->getLangOpts().CPlusPlus11 ? "nullptr" : "0")) {} void

Re: [PATCH] D16794: [Clang-tidy] Make readability-simplify-boolean-expr working with included files

2016-02-03 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. In http://reviews.llvm.org/D16794#341552, @aaron.ballman wrote: > Missing a test case for this functionality. The isExpansionInMainFile() was > used to silence the diagnostic in macros, but it was pointed out during > review that this should be fixed and it

Re: [PATCH] D16794: [Clang-tidy] Make readability-simplify-boolean-expr working with included files

2016-02-03 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. Again, isn't this already assigned to me in the bug tracker? In general, my assumption in bug trackers is that if someone has assigned the bug to themselves, then they are working on it. Repository: rL LLVM http://reviews.llvm.org/D16794

Re: [PATCH] D16786: [Clang-tidy] Make modernize-redundant-void-arg working with included files

2016-02-03 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. I am already working on fixing this bug (isn't PR25894 already assigned to me?), but what's missing from this patch is an automated test that proves it works. In order to write such an automated test, I believe

Re: [PATCH] D16259: Add clang-tidy readability-redundant-control-flow check

2016-02-03 Thread Richard via cfe-commits
In article

Re: [PATCH] D16308: clang-tidy Enhance readability-simplify-boolean-expr check to handle implicit conversions of integral types to bool and member pointers

2016-02-02 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:366 @@ +365,3 @@ + binaryOperator( + isExpansionInMainFile(), hasOperatorName(OperatorName), + hasLHS(allOf( aaron.ballman wrote: >

Re: [PATCH] D16308: clang-tidy Enhance readability-simplify-boolean-expr check to handle implicit conversions of integral types to bool and member pointers

2016-02-02 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. I do not have commit access. If someone could commit this for me, I would appreciate it. Patch by Richard Thomson. Thanks. http://reviews.llvm.org/D16308 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D16308: clang-tidy Enhance readability-simplify-boolean-expr check to handle implicit conversions of integral types to bool and member pointers

2016-02-01 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.h:21-22 @@ -20,71 +20,4 @@ /// them to use the appropriate boolean expression directly. /// -/// Examples: -/// -/// ===

Re: [PATCH] D16308: clang-tidy Enhance readability-simplify-boolean-expr check to handle implicit conversions of integral types to bool and member pointers

2016-02-01 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.h:21-22 @@ -20,71 +20,4 @@ /// them to use the appropriate boolean expression directly. /// -/// Examples: -/// -/// ===

Re: [PATCH] D16526: Add hasRetValue narrowing matcher for returnStmt

2016-02-01 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. In http://reviews.llvm.org/D16526#334938, @aaron.ballman wrote: > I'm of two minds on this [...] I just need a thumbs up/down on this. Thumbs down, I discard the review. Thumbs up, you take the patch. Patch by Richard Thomson.

Re: [PATCH] D16308: clang-tidy Enhance readability-simplify-boolean-expr check to handle implicit conversions of integral types to bool and member pointers

2016-02-01 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.h:21-22 @@ -20,71 +20,4 @@ /// them to use the appropriate boolean expression directly. /// -/// Examples: -/// -/// ===

Re: [PATCH] D16529: [clang-tidy] Add modernize-raw-string-literal check

2016-02-01 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:88 @@ +87,3 @@ +} + +bool containsDelimiter(StringRef Bytes, const std::string ) { alexfh wrote: > aaron.ballman wrote: > > I think Alex's point is: why not

Re: [PATCH] D16308: clang-tidy Enhance readability-simplify-boolean-expr check to handle implicit conversions of integral types to bool and member pointers

2016-01-31 Thread Richard via cfe-commits
LegalizeAdulthood updated this revision to Diff 46505. LegalizeAdulthood added a comment. Update from review comments. trunk clang-format http://reviews.llvm.org/D16308 Files: clang-tidy/readability/SimplifyBooleanExprCheck.cpp clang-tidy/readability/SimplifyBooleanExprCheck.h

Re: [PATCH] D16308: clang-tidy Enhance readability-simplify-boolean-expr check to handle implicit conversions of integral types to bool and member pointers

2016-01-31 Thread Richard via cfe-commits
LegalizeAdulthood updated this revision to Diff 46506. LegalizeAdulthood added a comment. Inline Variable http://reviews.llvm.org/D16308 Files: clang-tidy/readability/SimplifyBooleanExprCheck.cpp clang-tidy/readability/SimplifyBooleanExprCheck.h

Re: [PATCH] D16529: [clang-tidy] Add modernize-raw-string-literal check

2016-01-31 Thread Richard via cfe-commits
LegalizeAdulthood updated this revision to Diff 46504. LegalizeAdulthood added a comment. Update from comments http://reviews.llvm.org/D16529 Files: clang-tidy/modernize/CMakeLists.txt clang-tidy/modernize/ModernizeTidyModule.cpp clang-tidy/modernize/RawStringLiteralCheck.cpp

Re: [PATCH] D16529: [clang-tidy] Add modernize-raw-string-literal check

2016-01-31 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: docs/clang-tidy/checks/modernize-raw-string-literal.rst:46 @@ +45,3 @@ +editor. Trailing whitespace is likely to be stripped by editors and other +tools, changing the meaning of the literal. +

Re: [PATCH] D16529: [clang-tidy] Add modernize-raw-string-literal check

2016-01-31 Thread Richard via cfe-commits
LegalizeAdulthood marked an inline comment as done. Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:79 @@ +78,3 @@ + if (Text.find('R') < Text.find('"')) +return false; + Take a look at http://en.cppreference.com/w/cpp/language/string_literal

Re: [PATCH] D16308: clang-tidy Enhance readability-simplify-boolean-expr check to handle implicit conversions of integral types to bool and member pointers

2016-01-31 Thread Richard via cfe-commits
LegalizeAdulthood updated this revision to Diff 46507. LegalizeAdulthood added a comment. Move local function to anonymous namespace http://reviews.llvm.org/D16308 Files: clang-tidy/readability/SimplifyBooleanExprCheck.cpp clang-tidy/readability/SimplifyBooleanExprCheck.h

Re: [PATCH] D16529: [clang-tidy] Add modernize-raw-string-literal check

2016-01-29 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. In http://reviews.llvm.org/D16529#339192, @bkramer wrote: > Why are you re-adding all those Makefiles? Ugh, I didn't even notice they were in there. It must have errantly slipped in from rebasing. http://reviews.llvm.org/D16529

Re: [PATCH] D8149: Extend hasType narrowing matcher for TypedefDecls, add functionProtoType matcher for FunctionProtoType nodes, extend parameterCountIs to FunctionProtoType nodes

2016-01-29 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. If someone could commit this for me, that would be great. I do not have commit access. Patch by Richard Thomson http://reviews.llvm.org/D8149 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: r259210 - Extend hasType narrowing matcher for TypedefDecls, add functionProtoType matcher for FunctionProtoType nodes, extend parameterCountIs to FunctionProtoType nodes.

2016-01-29 Thread Richard via cfe-commits
In article , Aaron Ballman writes: > On Fri, Jan 29, 2016 at 1:28 PM, Hans Wennborg wrote: > > This broke tests: > > > >

Re: [PATCH] D16308: clang-tidy Enhance readability-simplify-boolean-expr check to handle implicit conversions of integral types to bool and member pointers

2016-01-29 Thread Richard via cfe-commits
LegalizeAdulthood marked 2 inline comments as done. Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.h:77 @@ -74,3 +76,3 @@ /// implicit conversion of `i & 1` to `bool` and becomes -/// `bool b = static_cast(i & 1);`. +/// `bool b = i & 1 != 0;`. ///

Re: [PATCH] D8149: Extend hasType narrowing matcher for TypedefDecls, add functionProtoType matcher for FunctionProtoType nodes, extend parameterCountIs to FunctionProtoType nodes

2016-01-29 Thread Richard via cfe-commits
LegalizeAdulthood updated this revision to Diff 46464. LegalizeAdulthood added a comment. Fix RegistryTest unit test make check-all passes http://reviews.llvm.org/D8149 Files: docs/LibASTMatchersReference.html include/clang/ASTMatchers/ASTMatchers.h

  1   2   >