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

alexfh updated this revision to Diff 97971. alexfh added a comment. Herald added a subscriber: xazax.hun. Rebased on HEAD. https://reviews.llvm.org/D31160 Files: clang-tidy/misc/MoveConstantArgumentCheck.cpp test/clang-tidy/misc-move-const-arg.cpp Index: test/clang-tidy/misc-move-const-ar

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

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

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

alexfh added inline comments. Comment at: clang-tidy/misc/MoveConstantArgumentCheck.cpp:76 if (IsConstArg || IsTriviallyCopyable) { +if (const CXXRecordDecl *R = Arg->getType()->getAsCXXRecordDecl()) { + for (const auto *Ctor : R->ctors()) { sbenza wr

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

alexfh added inline comments. Comment at: clang-tidy/misc/MoveConstantArgumentCheck.cpp:76 if (IsConstArg || IsTriviallyCopyable) { +if (const CXXRecordDecl *R = Arg->getType()->getAsCXXRecordDecl()) { + for (const auto *Ctor : R->ctors()) { alexfh wr

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL302261: [clang-tidy] Fix misc-move-const-arg for move-only types. (authored by alexfh). Changed prior to commit: https://reviews.llvm.org/D31160?vs=97971&id=97985#toc Repository: rL LLVM https://rev

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

alexfh added a comment. Paweł, are you planning to finish this patch? https://reviews.llvm.org/D30748 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

alexfh added a comment. In https://reviews.llvm.org/D30748#750626, @idlecode wrote: > Oh, sorry about this - I forgot. I will send patch during this weekend No worries, I just stumbled upon the bug and recalled that there had been a patch to fix it. https://reviews.llvm.org/D30748 ___

[PATCH] D30896: [Clang-tidy] add check misc-prefer-switch-for-enums

alexfh requested changes to this revision. alexfh added a comment. In https://reviews.llvm.org/D30896#703046, @jbcoe wrote: > I think that clang-tidy allows case-specific subsetting of C++. It is said > that C++ contains a beautiful language and I've found that definition of > beauty to be very

[PATCH] D33010: Make google-build-using-namespace skip std::.*literals

alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: clang-tidy/google/UsingNamespaceDirectiveCheck.cpp:43 + if (StandardLiteralsNamespaceRE.match( + U->getNominatedNamespace()->getQualifiedN

[PATCH] D33173: Modify test to look for patterns in stderr as well

alexfh added a comment. Feel free to commit this to fix buildbots, however the underlying https://reviews.llvm.org/D33013 raises some questions. Clang-tidy is also used as a library, in which case it should capture all compiler diagnostics and pass them to the caller. This test verifies that c

[PATCH] D33013: Driver must return non-zero code on errors in command line

alexfh added inline comments. Comment at: lib/Tooling/CompilationDatabase.cpp:208 IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions(); - UnusedInputDiagConsumer DiagClient; + TextDiagnosticPrinter DiagnosticPrinter(llvm::errs(), &*DiagOpts); + UnusedInputDiagConsumer Diag

[PATCH] D33010: Make google-build-using-namespace skip std::.*literals

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG with one nit. Thank you for addressing this! Do you need me to commit the patch for you? Comment at: clang-tidy/google/UsingNamespaceDirectiveCheck.cpp:48 +bool UsingNa

[PATCH] D33010: Make google-build-using-namespace skip std::.*literals

This revision was automatically updated to reflect the committed changes. Closed by commit rL303085: Make google-build-using-namespace skip std::.*literals (authored by alexfh). Changed prior to commit: https://reviews.llvm.org/D33010?vs=99011&id=99025#toc Repository: rL LLVM https://review

[PATCH] D33207: Fix an assertion failure in FormatASTNodeDiagnosticArgument.

alexfh created this revision. The test being added in this patch used to cause an assertion failure: /build/./bin/clang -cc1 -internal-isystem /build/lib/clang/5.0.0/include -nostdsysteminc -verify -fsyntax-only -std=c++11 -Wshadow-all /src/tools/clang/test/SemaCXX/warn-shadow.cpp -

[PATCH] D33207: Fix an assertion failure in FormatASTNodeDiagnosticArgument.

alexfh added a comment. The patch is trivial, I'm mostly wondering about the wording and whether we want to expand the "linkage specification" to specify whether it is `extern "C"` or `extern "C++"`, for example. https://reviews.llvm.org/D33207 __

[PATCH] D33209: [clang-tidy] Add "emplace_back" detection in inefficient-vector-operation.

alexfh added inline comments. Comment at: clang-tidy/performance/InefficientVectorOperationCheck.cpp:208 + "consider pre-allocating the vector capacity before the loop") + << VectorAppendCall->getMethodDecl()->getDeclName(); Diagnostic builder sh

[PATCH] D33207: Fix an assertion failure in FormatASTNodeDiagnosticArgument.

alexfh updated this revision to Diff 99112. alexfh added a comment. Herald added a subscriber: krytarowski. Instead of handling LinkageSpecDecl, use getRedeclContext() when issuing -Wshadow diagnostic. https://reviews.llvm.org/D33207 Files: lib/Sema/SemaDecl.cpp test/SemaCXX/warn-shadow.cp

[PATCH] D33207: Fix an assertion failure in FormatASTNodeDiagnosticArgument.

alexfh added inline comments. Comment at: test/SemaCXX/warn-shadow.cpp:214 +void handleLinkageSpec() { + typedef void externC; // expected-warning {{declaration shadows a typedef in linkage specification}} +} rsmith wrote: > We should be producing a diagnostic

[PATCH] D33209: [clang-tidy] Add "emplace_back" detection in inefficient-vector-operation.

alexfh added inline comments. Comment at: clang-tidy/performance/InefficientVectorOperationCheck.cpp:208 + "consider pre-allocating the vector capacity before the loop") + << VectorAppendCall->getMethodDecl()->getDeclName(); hokein wrote: > alexf

[PATCH] D33209: [clang-tidy] Add "emplace_back" detection in inefficient-vector-operation.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG once the comments are addressed. https://reviews.llvm.org/D33209 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org

[PATCH] D31700: [clang-tidy] Ignore blank spaces between cast's ")" and its sub expr.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Sorry, missed your patch somehow. LG with one nit. Comment at: clang-tidy/google/AvoidCStyleCastsCheck.cpp:139 ")"); + ReplaceRange = CharSourceRange::getChar

[PATCH] D32700: [clang-tidy] Add misc-suspicious-memset-usage check.

alexfh added a comment. > The existing google-runtime-memset-zero-length check is related. It finds > cases when the byte count parameter is zero and offers to swap that with the > fill value argument. Perhaps the two could be merged while maintaining > backward compatibility through an alias.

[PATCH] D33207: Fix an assertion failure in FormatASTNodeDiagnosticArgument.

This revision was automatically updated to reflect the committed changes. Closed by commit rL303325: Fix an assertion failure in FormatASTNodeDiagnosticArgument. (authored by alexfh). Changed prior to commit: https://reviews.llvm.org/D33207?vs=99112&id=99386#toc Repository: rL LLVM https://

[PATCH] D33304: [WIP][clang-tidy] Add a new module Android and a new check for file descriptors.

alexfh added a comment. In https://reviews.llvm.org/D33304#758713, @aaron.ballman wrote: > In https://reviews.llvm.org/D33304#758624, @srhines wrote: > > > In https://reviews.llvm.org/D33304#758621, @joerg wrote: > > > > > I find the use of "must" at the very least inappropriate. If there was no

[PATCH] D33358: [clang-tidy] readability-redundant-declaration false positive for defaulted function

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

[PATCH] D33354: [clang-tidy] readability-braces-around-statements false positive with char literals

alexfh added a comment. I'll repeat my comment from https://reviews.llvm.org/D25558: "I'm thoroughly confused as to why the code in your test was not handled correctly and why this is the right fix. Can you explain?" The "For some reason ..." part doesn't really explain anything. I guess, we're

[PATCH] D33354: [clang-tidy] readability-braces-around-statements false positive with char literals

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

[PATCH] D33354: [clang-tidy] readability-braces-around-statements false positive with char literals

alexfh added a comment. I'm going to commit the patch for you, but I guess, you might want to ask for commit access (http://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access). https://reviews.llvm.org/D33354 ___ cfe-commits mailing list c

[PATCH] D33304: [WIP][clang-tidy] Add a new module Android and a new check for file descriptors.

alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: clang-tidy/android/FileDescriptorCheck.cpp:25 + callExpr( + callee(functionDecl(allOf( + isExpansionInFileMatching(HEADER

[PATCH] D33272: Method loadFromCommandLine should be able to report errors

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Thank you! This looks good with one nit (see the inline comment). Comment at: lib/Tooling/CompilationDatabase.cpp:292 + if (Argc == 0) { +ErrorMsg = "error: no arguments

[PATCH] D33272: Method loadFromCommandLine should be able to report errors

alexfh added inline comments. Comment at: lib/Tooling/CompilationDatabase.cpp:164 +// to user. +} else if (DiagLevel >= DiagnosticsEngine::Error) + Other.HandleDiagnostic(DiagLevel, Info); Another couple of nits: 1. the comment below would probably b

[PATCH] D32346: [clang-tidy] New readability check for strlen argument

alexfh added inline comments. Comment at: clang-tidy/readability/StrlenArgumentCheck.cpp:23-24 + Finder->addMatcher( + callExpr(anyOf(callee(functionDecl(hasName("::strlen"))), + callee(functionDecl(hasName("std::strlen", + hasAnyArgume

[PATCH] D32346: [clang-tidy] New readability check for strlen argument

alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: docs/clang-tidy/checks/readability-strlen-argument.rst:20 +char *p = new char[(strlen(s) - 1)] +strcpy(p, s); + JonasToth w

[PATCH] D33354: [clang-tidy] readability-braces-around-statements false positive with char literals

This revision was automatically updated to reflect the committed changes. Closed by commit rL303551: [clang-tidy] readability-braces-around-statements false positive with char… (authored by alexfh). Changed prior to commit: https://reviews.llvm.org/D33354?vs=99576&id=99756#toc Repository: rL

[PATCH] D33358: [clang-tidy] readability-redundant-declaration false positive for defaulted function

This revision was automatically updated to reflect the committed changes. Closed by commit rL303552: [clang-tidy] readability-redundant-declaration false positive for defaulted… (authored by alexfh). Changed prior to commit: https://reviews.llvm.org/D33358?vs=99570&id=99757#toc Repository: r

[PATCH] D33272: Method loadFromCommandLine should be able to report errors

alexfh added inline comments. Comment at: lib/Tooling/CommonOptionsParser.cpp:122 + FixedCompilationDatabase::loadFromCommandLine(argc, argv, ErrorMessage); + if (!Compilations && StringRef(ErrorMessage).startswith("error")) +llvm::errs() << ErrorMessage; --

[PATCH] D33272: Method loadFromCommandLine should be able to report errors

alexfh accepted this revision. alexfh added a comment. Thanks, LG! https://reviews.llvm.org/D33272 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116048: [clang][CodeGen][UBSan] VLA size checking for unsigned integer parameter

alexfh added a comment. We've started seeing some tests dying with SIGILL (illegal instruction) after this patch. I'm working on a reduced repro, but please take a look, if you can find any issues yourself. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D116048: [clang][CodeGen][UBSan] VLA size checking for unsigned integer parameter

alexfh added a comment. In D116048#3261573 , @alexfh wrote: > We've started seeing some tests dying with SIGILL (illegal instruction) after > this patch. I'm working on a reduced repro, but please take a look, if you > can find any issues yourself. Fal

[PATCH] D117376: Remove reference type when checking const structs

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good. Thanks for the fix! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117376/new/ https://reviews.llvm.org/D117376

[PATCH] D69560: [clang-tidy] Add 'bugprone-easily-swappable-parameters' check

alexfh added a comment. Thanks for the great work! This is mostly looking fine, but I've added a few comments. I could only make a quite superficial review so far, but I'll try to take a deeper look into this soon. Comment at: clang-tools-extra/clang-tidy/bugprone/EasilySwap

[PATCH] D69560: [clang-tidy] Add 'bugprone-easily-swappable-parameters' check

alexfh added a comment. BTW, any ideas why "patch application failed"? (See the Build status in the Diff Detail section) It would be really nice to get pre-merge checks to work for the patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69560/new

[PATCH] D117460: [clang-tidy][NFC] Reduce map lookups in IncludeSorter

alexfh added a comment. I wonder what motivated the patch. Is this a performance optimization? If so, do you have any measurements? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117460/new/ https://reviews.llvm.org/D117460 ___

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good with one suggestion. Comment at: clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py:123 + 'lines.' +

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

alexfh added a comment. Do you need help landing the change? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D49864/new/ https://reviews.llvm.org/D49864 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D98070: [clang-tidy] Add option to ignore macros in readability-function-cognitive-complexity check.

alexfh accepted this revision. alexfh added a comment. LG with a couple of nits. Comment at: clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:230 + explicit FunctionASTVisitor(const bool IgnoreMacros) + : IgnoreMacros(IgnoreMacros){}; + -

[PATCH] D98070: [clang-tidy] Add option to ignore macros in readability-function-cognitive-complexity check.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG8a944d82cd14: [clang-tidy] Add option to ignore macros in readability-function-cognitive… (authored by massberg, committed by alexfh). Repository:

[PATCH] D91303: [clang-tidy] readability-container-size-empty: simplify implementation

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91303/new/ https://reviews.llvm.org/D91303 ___ cfe

[PATCH] D91303: [clang-tidy] readability-container-size-empty: simplify implementation

alexfh added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/readability-container-size-empty.cpp:480-482 -// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: the 'empty' method should be used -// CHECK-MESSAGES: :101:18: note: method 'Lazy'::empty() defined her

[PATCH] D111109: AddGlobalAnnotations for function with or without function body.

alexfh added a comment. It looks like this patch causes clang to crash on valid code with the following stack trace: #0 0x560442f12f45 SignalHandler(int) #1 0x7ffac7c789a0 __restore_rt #2 0x560440083992 llvm::LazyCallGraph::Node::populateSlow() #3 0x560440082a60 llvm:

[PATCH] D111109: AddGlobalAnnotations for function with or without function body.

alexfh added a comment. Reduced test case: template void b(a); template class d { public: class e { public: c *f(); }; e *g(); }; template class j; template class j { public: class k { public: k(int *); ~k(); }; int n(); d l;

[PATCH] D111109: AddGlobalAnnotations for function with or without function body.

alexfh added a comment. Reduced the test further to: struct k { ~k() __attribute__((annotate(""))) {} }; void m() { k(); } Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D09/new/ https://reviews.llvm.org/D09 __

<    7   8   9   10   11   12