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

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] 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: > > http://bb.pgr.jp/builders/cmake-clang-tools-x86_64-linux/builds/23351 > http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/492 >

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 > http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/492 > http://lab.llvm.org:8011/builders/clang-p

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, has_check_messag

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 clang-tidy/readability/SimplifyBooleanExprCh

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 , Richard via cfe-commits writes: > make check-all passes for me. I don't see what the problem is in the logs > eit

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] 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 still seems unlikely for / and %. I thought the whole point was to flag instances where the

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

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

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 47928. LegalizeAdulthood added a comment. Add FIXME for non-ASCII string literals. Allow delimiter stem to be configured. Avoid some string concatenation. http://reviews.llvm.org/D16529 Files: clang-tidy/modernize/CMakeLists.txt clang-tidy/mod

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

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 seem

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

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 extend

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] 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 cfe-commits@lists.ll

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

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"; --

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] 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 @@ +MatcherStmt>addrLabelExprMatcherAddrL

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 that

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] 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] 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 http://lists.llvm.org/cgi-bin/mailman/listi

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

  1   2   >