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 c

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

2016-03-18 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 http://review

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: > 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 o

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] 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' [misc-const-ref-builtin

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 to

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

[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 docs

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

2016-03-19 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: docs/clang-tidy/checks/list.rst:91 @@ -90,2 +90,3 @@ performance-implicit-cast-in-loop + performance-unnecessary-copy-initialization.rst readability-braces-around-statements This check was missing from

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.

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] D16529: [clang-tidy] Add modernize-raw-string-literal check

2016-03-22 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. Looks like I forgot to remove brace initializers from the test files. I will fix that. Chris Lattner has given me commit access now, so I can commit on my own. http://reviews.llvm.org/D16529 ___ cfe-commits maili

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 clang-tidy

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 clang-tidy/modernize/RawStringLiteralC

[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] 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

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] 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 wr

Re: [PATCH] D8149: Add hasUnderlyingType narrowing matcher for TypedefDecls, functionProtoType matcher for FunctionProtoType nodes, extend parameterCountIs to FunctionProtoType nodes

2015-12-29 Thread Richard via cfe-commits
LegalizeAdulthood updated this revision to Diff 43761. LegalizeAdulthood added a comment. Regenerate documentation from dump_ast_matchers.py http://reviews.llvm.org/D8149 Files: docs/LibASTMatchersReference.html include/clang/ASTMatchers/ASTMatchers.h lib/ASTMatchers/Dynamic/Registry.cpp

Re: [PATCH] D8149: Add hasUnderlyingType narrowing matcher for TypedefDecls, functionProtoType matcher for FunctionProtoType nodes, extend parameterCountIs to FunctionProtoType nodes

2015-12-30 Thread Richard via cfe-commits
LegalizeAdulthood marked an inline comment as done. Comment at: include/clang/ASTMatchers/ASTMatchers.h:4022 @@ +4021,3 @@ +/// functionProtoType() +/// matches "int (*f)(int)" and the type of "g". +AST_TYPE_MATCHER(FunctionProtoType, functionProtoType); aaron.b

Re: [PATCH] D8149: Add hasUnderlyingType narrowing matcher for TypedefDecls, functionProtoType matcher for FunctionProtoType nodes, extend parameterCountIs to FunctionProtoType nodes

2015-12-30 Thread Richard via cfe-commits
LegalizeAdulthood updated this revision to Diff 43800. LegalizeAdulthood added a comment. Add more unit tests. http://reviews.llvm.org/D8149 Files: docs/LibASTMatchersReference.html include/clang/ASTMatchers/ASTMatchers.h lib/ASTMatchers/Dynamic/Registry.cpp unittests/ASTMatchers/ASTMat

Re: [PATCH] D8149: Add hasUnderlyingType narrowing matcher for TypedefDecls, functionProtoType matcher for FunctionProtoType nodes, extend parameterCountIs to FunctionProtoType nodes

2016-01-04 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: unittests/ASTMatchers/ASTMatchersTest.cpp:4990 @@ +4989,3 @@ + EXPECT_TRUE(matches("typedef int hasUnderlyingTypeTest;", + typedefDecl(hasUnderlyingType(asString("int"); + EXPECT_TRUE(

Re: [PATCH] D8149: Add hasUnderlyingType narrowing matcher for TypedefDecls, functionProtoType matcher for FunctionProtoType nodes, extend parameterCountIs to FunctionProtoType nodes

2016-01-05 Thread Richard via cfe-commits
LegalizeAdulthood updated this revision to Diff 43965. LegalizeAdulthood added a comment. Add more unit tests from comments http://reviews.llvm.org/D8149 Files: docs/LibASTMatchersReference.html include/clang/ASTMatchers/ASTMatchers.h lib/ASTMatchers/Dynamic/Registry.cpp unittests/ASTMa

Re: [PATCH] D8149: Add hasUnderlyingType narrowing matcher for TypedefDecls, functionProtoType matcher for FunctionProtoType nodes, extend parameterCountIs to FunctionProtoType nodes

2016-01-05 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: unittests/ASTMatchers/ASTMatchersTest.cpp:4282 @@ +4281,3 @@ + EXPECT_TRUE(matches("void f(int i);", functionProtoType())); + EXPECT_TRUE(matches("void f();", functionProtoType(parameterCountIs(0; + EXPECT_TRUE( -

Re: [PATCH] D8149: Add hasUnderlyingType narrowing matcher for TypedefDecls, functionProtoType matcher for FunctionProtoType nodes, extend parameterCountIs to FunctionProtoType nodes

2016-01-06 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: unittests/ASTMatchers/ASTMatchersTest.cpp:4283 @@ +4282,3 @@ + EXPECT_TRUE(matches("void f(int i);", functionProtoType())); + EXPECT_TRUE(matches("void f();", functionProtoType(parameterCountIs(0; + EXPECT_TRUE(notMatches

Re: [PATCH] D8149: Add hasUnderlyingType narrowing matcher for TypedefDecls, functionProtoType matcher for FunctionProtoType nodes, extend parameterCountIs to FunctionProtoType nodes

2016-01-06 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: unittests/ASTMatchers/ASTMatchersTest.cpp:4992 @@ +4991,3 @@ + EXPECT_TRUE(matches("typedef int hasUnderlyingTypeTest;", + typedefDecl(hasUnderlyingType(asString("int"); + EXPECT_TRUE(matches("typedef

Re: [PATCH] D8149: Add hasUnderlyingType narrowing matcher for TypedefDecls, functionProtoType matcher for FunctionProtoType nodes, extend parameterCountIs to FunctionProtoType nodes

2016-01-06 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: unittests/ASTMatchers/ASTMatchersTest.cpp:4992 @@ +4991,3 @@ + EXPECT_TRUE(matches("typedef int hasUnderlyingTypeTest;", + typedefDecl(hasUnderlyingType(asString("int"); + EXPECT_TRUE(matches("typedef

Re: [PATCH] D8149: Add hasUnderlyingType narrowing matcher for TypedefDecls, functionProtoType matcher for FunctionProtoType nodes, extend parameterCountIs to FunctionProtoType nodes

2016-01-06 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: unittests/ASTMatchers/ASTMatchersTest.cpp:4992 @@ +4991,3 @@ + EXPECT_TRUE(matches("typedef int hasUnderlyingTypeTest;", + typedefDecl(hasUnderlyingType(asString("int"); + EXPECT_TRUE(matches("typedef

Re: [PATCH] D8149: Add hasUnderlyingType narrowing matcher for TypedefDecls, functionProtoType matcher for FunctionProtoType nodes, extend parameterCountIs to FunctionProtoType nodes

2016-01-06 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: unittests/ASTMatchers/ASTMatchersTest.cpp:4992 @@ +4991,3 @@ + EXPECT_TRUE(matches("typedef int hasUnderlyingTypeTest;", + typedefDecl(hasUnderlyingType(asString("int"); + EXPECT_TRUE(matches("typedef

Re: [PATCH] D8149: Add hasUnderlyingType narrowing matcher for TypedefDecls, functionProtoType matcher for FunctionProtoType nodes, extend parameterCountIs to FunctionProtoType nodes

2016-01-11 Thread Richard via cfe-commits
LegalizeAdulthood marked an inline comment as done. Comment at: unittests/ASTMatchers/ASTMatchersTest.cpp:4994 @@ +4993,3 @@ + EXPECT_TRUE(matches("typedef int hasUnderlyingTypeTest;", + typedefDecl(hasUnderlyingType(asString("int"); + EXPECT_TRUE(matche

Re: [PATCH] D8149: Add hasUnderlyingType narrowing matcher for TypedefDecls, functionProtoType matcher for FunctionProtoType nodes, extend parameterCountIs to FunctionProtoType nodes

2016-01-12 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: unittests/ASTMatchers/ASTMatchersTest.cpp:4994 @@ +4993,3 @@ + EXPECT_TRUE(matches("typedef int hasUnderlyingTypeTest;", + typedefDecl(hasUnderlyingType(asString("int"); + EXPECT_TRUE(matches("typedef

Re: [PATCH] D8149: Add hasUnderlyingType narrowing matcher for TypedefDecls, functionProtoType matcher for FunctionProtoType nodes, extend parameterCountIs to FunctionProtoType nodes

2016-01-15 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: unittests/ASTMatchers/ASTMatchersTest.cpp:4994 @@ +4993,3 @@ + EXPECT_TRUE(matches("typedef int hasUnderlyingTypeTest;", + typedefDecl(hasUnderlyingType(asString("int"); + EXPECT_TRUE(matches("typedef

[PATCH] D16259: Add clang-tidy readability-redundant-return check

2016-01-15 Thread Richard via cfe-commits
LegalizeAdulthood created this revision. LegalizeAdulthood added a reviewer: alexfh. LegalizeAdulthood added a subscriber: cfe-commits. This check looks for `return` statements at the end of a function returning `void`: ``` void f() { return; } ``` becomes ``` void f() { } ``` http://revie

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

2016-01-15 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tidy/readability/RedundantReturnCheck.cpp:46-49 @@ +45,6 @@ + auto ReturnRange = CharSourceRange::getCharRange( + Start, Lexer::findLocationAfterToken( + Return->getLocEnd(), tok::semi, *R

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

2016-01-17 Thread Richard via cfe-commits
LegalizeAdulthood updated this revision to Diff 45110. LegalizeAdulthood added a comment. Extend this check to handle redundant `continue` statements in loop bodies. With this extension, I'm wondering if the check should be renamed redundant-control-flow instead of redundant-return. Comments?

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

2016-01-17 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. I didn't know about the bug reports. I created this check because I have encountered such redundant control flow in real code bases. This would close 21984 , but not 22416

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

2016-01-17 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. I'm starting with stuff that I see in code bases where I work; chances are that other people see them too. In the future, I'll look in the bug database for stuff that is similar or identical to what I'm doing. http://reviews.llvm.org/D16259 __

Re: [PATCH] D16179: [clang-tidy] Handle decayed types and other improvements in VirtualNearMiss check.

2016-01-18 Thread Richard via cfe-commits
LegalizeAdulthood added a subscriber: LegalizeAdulthood. Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:54-55 @@ -53,16 +53,4 @@ // Both types must be pointers or references to classes. - if (const auto *DerivedPT = DerivedReturnTy->getAs()) { -if (const auto *Base

Re: [PATCH] D16286: [clang-tidy] Readability check for redundant parenthesis in return expression.

2016-01-18 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tidy/readability/ReturnWithRedundantParensCheck.cpp:41-44 @@ +40,6 @@ + + // Complex expression can be surrounded by parenthesis for readability + if (isComplexExpression(ParenExpression->getSubExpr())) { +return; +

Re: [PATCH] D8149: Add hasUnderlyingType narrowing matcher for TypedefDecls, functionProtoType matcher for FunctionProtoType nodes, extend parameterCountIs to FunctionProtoType nodes

2016-01-18 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. Is there any reason we can't proceed with the patch as-is and then see if it can be refactored into an overload of `hasType`? http://reviews.llvm.org/D8149 ___ cfe-commits mailing list cfe-commits@lists.llvm.org h

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

2016-01-18 Thread Richard via cfe-commits
LegalizeAdulthood created this revision. LegalizeAdulthood added a reviewer: alexfh. LegalizeAdulthood added a subscriber: cfe-commits. Expand the simplify boolean expression check to handle implicit conversion of integral types to bool and improve the handling of implicit conversion of member po

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

2016-01-19 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tidy/readability/RedundantReturnCheck.cpp:24 @@ +23,3 @@ + functionDecl(isDefinition(), returns(asString("void")), + has(compoundStmt(hasAnySubstatement(returnStmt() + .bind("fn"), -

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-19 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.h:50 @@ -49,3 +49,3 @@ /// opposite condition. /// 4. Implicit conversions of pointer to `bool` are replaced with explicit /// comparisons to `nullptr`.

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

2016-01-19 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: test/clang-tidy/readability-redundant-return.cpp:21-24 @@ +20,6 @@ + +void g(int i) { + if (i < 0) { +return; + } + if (i < 10) { kimgr wrote: > What happens to guard clauses invoking void functions? > >

Re: [PATCH] D16286: [clang-tidy] Readability check for redundant parenthesis in return expression.

2016-01-19 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. @alexfh: The check and the bug report originate in the preferences of the google style guide. http://reviews.llvm.org/D16286 ___ cfe-commits mailing list cfe-commits@

Re: [PATCH] D16286: [clang-tidy] Readability check for redundant parenthesis in return expression.

2016-01-19 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. With readability checks, there are always people who disagree about what makes for more readable code. I think I have seen this back-and-forth discussion with **every** readability check since I've been paying attention. It simply isn't possible to make ever

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

2016-01-19 Thread Richard via cfe-commits
LegalizeAdulthood added a subscriber: LegalizeAdulthood. LegalizeAdulthood added a comment. Why not supply a fixit that removes the cast? Repository: rL LLVM http://reviews.llvm.org/D16310 ___ cfe-commits mailing list cfe-commits@lists.llvm.org h

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

2016-01-19 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: test/clang-tidy/readability-redundant-return.cpp:21-24 @@ +20,6 @@ + +void g(int i) { + if (i < 0) { +return; + } + if (i < 10) { kimgr wrote: > LegalizeAdulthood wrote: > > kimgr wrote: > > > What happen

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-20 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: docs/clang-tidy/checks/readability-simplify-boolean-expr.rst:59 @@ -56,3 +58,3 @@ 4. The conditional return ``if (p) return true; return false;`` has an implicit conversion of a pointer to ``bool`` and becomes ---

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-20 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. 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] D16308: clang-tidy Enhance readability-simplify-boolean-expr check to handle implicit conversions of integral types to bool and member pointers

2016-01-20 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: docs/clang-tidy/checks/readability-simplify-boolean-expr.rst:59 @@ -56,3 +58,3 @@ 4. The conditional return ``if (p) return true; return false;`` has an implicit conversion of a pointer to ``bool`` and becomes ---

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

2016-01-20 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. If you state what the check does, then In http://reviews.llvm.org/D16310#331054, @danielmarjamaki wrote: > In http://reviews.llvm.org/D16310#330367, @LegalizeAdulthood wrote: > > > Why not supply a fixit that removes the cast? > > > I am skeptic. There are diff

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

2016-01-20 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tidy/misc/LongCastCheck.cpp:43 @@ +42,3 @@ + +static unsigned getMaxCalculationWidth(ASTContext &C, const Expr *E) { + E = E->IgnoreParenImpCasts(); Prefer anonymous namespace over `static` to scope visib

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

2016-01-21 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. In http://reviews.llvm.org/D16310#332200, @danielmarjamaki wrote: > Why is there a cast in the first place? It is unlikely that the programmer > added a useless cast for no reason. If this has universally been your experience on a code base, then I say you s

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

2016-01-21 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tidy/misc/LongCastCheck.cpp:97 @@ +96,3 @@ + + if (!CastType->isIntegerType() || !SubType->isIntegerType()) +return; danielmarjamaki wrote: > LegalizeAdulthood wrote: > > Why don't you check for casti

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

2016-01-22 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. I find this an interesting discussion. I do not mean to imply that my remarks constitute any sort of demand that this check produce my suggestion for a fixit. In http://reviews.llvm.org/D16310#333416, @danielmarjamaki wrote: > I expect that this warning will

Re: [PATCH] D16286: [clang-tidy] Readability check for redundant parenthesis in return expression.

2016-01-22 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. As a general comment, I am continually fascinated by the **huge** variety of opinion as to matters of readability within the C++ community. I don't know why we have so many differing opinions compared to the communities built around other programming language

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-23 Thread Richard via cfe-commits
LegalizeAdulthood updated this revision to Diff 45812. LegalizeAdulthood marked 2 inline comments as done. LegalizeAdulthood added a comment. Update from review comments. http://reviews.llvm.org/D16308 Files: clang-tidy/readability/SimplifyBooleanExprCheck.cpp clang-tidy/readability/Simplif

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

2016-01-23 Thread Richard via cfe-commits
LegalizeAdulthood marked 8 inline comments as done. Comment at: clang-tidy/readability/RedundantReturnCheck.cpp:24 @@ +23,3 @@ + functionDecl(isDefinition(), returns(asString("void")), + has(compoundStmt(hasAnySubstatement(returnStmt() + .bind("

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

2016-01-23 Thread Richard via cfe-commits
LegalizeAdulthood retitled this revision from "Add clang-tidy readability-redundant-return check" to "Add clang-tidy readability-redundant-control-flow check". LegalizeAdulthood updated this revision to Diff 45814. LegalizeAdulthood marked an inline comment as done. LegalizeAdulthood added a comm

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

2016-01-24 Thread Richard via cfe-commits
LegalizeAdulthood retitled this revision from "Add hasUnderlyingType narrowing matcher for TypedefDecls, functionProtoType matcher for FunctionProtoType nodes, extend parameterCountIs to FunctionProtoType nodes" to "Extend hasType narrowing matcher for TypedefDecls, add functionProtoType matcher

[PATCH] D16526: Add hasRetValue narrowing matcher for returnStmt

2016-01-24 Thread Richard via cfe-commits
LegalizeAdulthood created this revision. LegalizeAdulthood added a reviewer: klimek. LegalizeAdulthood added a subscriber: cfe-commits. Herald added a subscriber: klimek. Add `hasRetValue` narrowing matcher for `returnStmt` so that you can match on the expression returned by a return stmt (or lac

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

2016-01-24 Thread Richard via cfe-commits
LegalizeAdulthood updated this revision to Diff 45836. LegalizeAdulthood added a comment. Run clang-format http://reviews.llvm.org/D16526 Files: docs/LibASTMatchersReference.html include/clang/ASTMatchers/ASTMatchers.h lib/ASTMatchers/Dynamic/Registry.cpp unittests/ASTMatchers/ASTMatche

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

2016-01-24 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. In http://reviews.llvm.org/D16526#334742, @aaron.ballman wrote: > I get the same behavior from the existing has() matcher -- how is this meant > to differ? Hmm! Good question. I suppose it doesn't do anything different. I was thinking how to write such mat

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

2016-01-24 Thread Richard via cfe-commits
LegalizeAdulthood updated this revision to Diff 45842. LegalizeAdulthood added a comment. Improve matcher to simplify callback http://reviews.llvm.org/D16259 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/ReadabilityTidyModule.cpp clang-tidy/readability/RedundantContr

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

2016-01-25 Thread Richard via cfe-commits
LegalizeAdulthood created this revision. LegalizeAdulthood added a reviewer: alexfh. LegalizeAdulthood added a subscriber: cfe-commits. This check examines string literals and looks for those that could be more directly expressed as a raw string literal. Example: ``` char const *const BackSlash

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

2016-01-26 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:35 @@ +34,3 @@ + const bool HasBackSlash = Text.find(R"(\\)") != StringRef::npos; + const bool HasNewLine = Text.find(R"(\n)") != StringRef::npos; + const bool HasQuote = Text.fi

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

2016-01-26 Thread Richard via cfe-commits
LegalizeAdulthood updated this revision to Diff 46101. LegalizeAdulthood marked 2 inline comments as done. LegalizeAdulthood added a comment. Update from comments: - leave existing raw string literals unchanged - don't change UTF-8, UTF-16, UTF-32 or wide character string literals - apply changes

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

2016-01-27 Thread Richard via cfe-commits
LegalizeAdulthood added a comment. In http://reviews.llvm.org/D16310#335844, @danielmarjamaki wrote: > There were some new interesting warnings and imho they were TP. TP? http://reviews.llvm.org/D16310 ___ cfe-commits mailing list cfe-commits@lis

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

2016-01-27 Thread Richard via cfe-commits
LegalizeAdulthood added a subscriber: LegalizeAdulthood. LegalizeAdulthood added a comment. In http://reviews.llvm.org/D16012#337208, @rsmith wrote: > What's the benefit of storing this? You can get the same effect by > re-lexing. We don't guarantee that the pretty printed version of the AST >

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

2016-01-27 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:27 @@ +26,3 @@ + // we only transform ASCII string literals + if (!Literal->isAscii()) +return false; alexfh wrote: > Why can't the check convert non-ascii st

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

2016-01-27 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:107 @@ +106,3 @@ + if (const auto *Literal = Result.Nodes.getNodeAs("lit")) +preferRawStringLiterals(Result, Literal); +} alexfh wrote: > I don't see a reason

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

2016-01-28 Thread Richard via cfe-commits
LegalizeAdulthood updated this revision to Diff 46333. LegalizeAdulthood marked 3 inline comments as done. LegalizeAdulthood added a comment. Herald added a subscriber: klimek. Update from review comments. Extend analysis of newlines to prevent pathological raw strings from being introduced. Exte

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

2016-01-28 Thread Richard via cfe-commits
LegalizeAdulthood marked 4 inline comments as done. Comment at: clang-tidy/readability/RedundantControlFlowCheck.cpp:59 @@ +58,3 @@ + CompoundStmt::const_reverse_body_iterator last = Block->body_rbegin(); + if (const auto *Return = dyn_cast(*last)) { +issueDiagnostic(Result,

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

2016-01-28 Thread Richard via cfe-commits
LegalizeAdulthood updated this revision to Diff 46334. LegalizeAdulthood marked an inline comment as done. LegalizeAdulthood added a comment. Update from review comments http://reviews.llvm.org/D16259 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/ReadabilityTidyModule.

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

2016-01-28 Thread Richard via cfe-commits
LegalizeAdulthood updated this revision to Diff 46341. LegalizeAdulthood added a comment. Update from review comments, add test case http://reviews.llvm.org/D8149 Files: docs/LibASTMatchersReference.html include/clang/ASTMatchers/ASTMatchers.h include/clang/ASTMatchers/ASTMatchersInternal

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 http://lists.

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: 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: > > > > http://bb.pgr.jp/builders/cmake-clang-x86_64-linux/builds/44018/steps/test_ clang/logs/Clang-Unit%20%3A%3A%20ASTMatchers__Dynamic__DynamicASTMatchersTests_ _RegistryTest.

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 include/clang/ASTMatchers/ASTMatchersIn

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. It seems that phabricator hasn't been configured to allow me to reopen the revision, but I've updated the diff. http://reviews.llvm.org/D8149 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.ll

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

2016-01-29 Thread Richard via cfe-commits
LegalizeAdulthood updated this revision to Diff 46466. LegalizeAdulthood added a comment. Re-upload diff http://reviews.llvm.org/D16529 Files: clang-tidy/modernize/CMakeLists.txt clang-tidy/modernize/ModernizeTidyModule.cpp clang-tidy/modernize/RawStringLiteralCheck.cpp clang-tidy/moder

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] 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 Ther

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. + LegalizeAdulthood

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 clang-tidy

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 docs/clang-

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 docs/clang-tidy/checks/readability-simplify

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 docs/clang-tidy/

Re: [PATCH] D15121: A new clang-tidy module to find calls to `std::swap`, and change them to use ADL

2015-12-01 Thread Richard via cfe-commits
LegalizeAdulthood added a subscriber: LegalizeAdulthood. LegalizeAdulthood added a comment. I'm wondering if there isn't an existing module that would be a good fit for this check. Why not in the modernize module? http://reviews.llvm.org/D15121 __

Re: [PATCH] D15411: [clang-tidy] Check for suspicious string assignments.

2015-12-11 Thread Richard via cfe-commits
LegalizeAdulthood added a subscriber: LegalizeAdulthood. Comment at: test/clang-tidy/misc-string-integer-assignment.cpp:25 @@ +24,3 @@ +int main() { + std::string s("foobar"); + Use {} initialization perhaps? http://reviews.llvm.org/D15411 __

Re: [PATCH] D15411: [clang-tidy] Check for suspicious string assignments.

2015-12-14 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: test/clang-tidy/misc-string-integer-assignment.cpp:46 @@ +45,3 @@ + ws += L'c'; + ws += (wchar_t)6; + Use `static_cast<>` instead of C-style cast? http://reviews.llvm.org/D15411 __

Re: [PATCH] D15411: [clang-tidy] Check for suspicious string assignments.

2015-12-14 Thread Richard via cfe-commits
LegalizeAdulthood added inline comments. Comment at: test/clang-tidy/misc-string-integer-assignment.cpp:33 @@ +32,3 @@ + s = 'c'; + s = (char)6; + What happens if the test code had used `static_cast`? http://reviews.llvm.org/D15411 _

[PATCH] D15737: [clang-tidy] Preserve comments and preprocessor directives when simplifying boolean expressions

2015-12-22 Thread Richard via cfe-commits
LegalizeAdulthood created this revision. LegalizeAdulthood added a reviewer: alexfh. LegalizeAdulthood added a subscriber: cfe-commits. This changeset still emits the diagnostic that the expression could be simplified, but it doesn't generate any fix-its that would lose comments or preprocessor

Re: [PATCH] D15737: [clang-tidy] Preserve comments and preprocessor directives when simplifying boolean expressions

2015-12-26 Thread Richard via cfe-commits
LegalizeAdulthood updated this revision to Diff 43647. LegalizeAdulthood marked 6 inline comments as done. LegalizeAdulthood added a comment. Update from review comments http://reviews.llvm.org/D15737 Files: clang-tidy/readability/SimplifyBooleanExprCheck.cpp clang-tidy/readability/Simplify

<    1   2