[PATCH] D50389: [clang-tidy] new check for Abseil

alexfh added inline comments. Comment at: docs/clang-tidy/checks/abseil-duration-division.rst:8 +division of two ``absl::Duration`` objects returns an ``int64`` with any fractional +component truncated toward 0. See `this link

[PATCH] D49800: [clang-tidy: modernize] modernize-redundant-void-arg crashes when a function body is in a macro

alexfh added inline comments. Comment at: clang-tidy/modernize/RedundantVoidArgCheck.cpp:246-247 + Lambda->getLambdaClass()->getLambdaTypeInfo()->getTypeLoc(); + End = SM->getSpellingLoc(TypeLoc.getLocEnd()); + Begin = SM->getSpellingLoc(TypeLoc.getLocStart());

[PATCH] D50447: [clang-tidy] Omit cases where loop variable is not used in loop body in performance-for-range-copy.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50447 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llv

[PATCH] D49851: [clang-tidy] run-clang-tidy add synchronisation to the output

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

[PATCH] D36892: [clang-tidy] check_clang_tidy.py: support CHECK-NOTES prefix

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D36892#1187959, @JonasToth wrote: > @lebedev.ri and @alexfh i would change the tests in > https://reviews.llvm.org/D48714 to use CHECK-NOTES. Is it ok, to commit th

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

alexfh added inline comments. Comment at: clang-tidy/abseil/NoInternalDepsCheck.cpp:38 + "depends upon internal implementation details, which violates the " + "abseil compatibilty guidelines. These can be found at " + "https://abseil.io/about/compatibility";); -

[PATCH] D49800: [clang-tidy: modernize] modernize-redundant-void-arg crashes when a function body is in a macro

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG with a couple of nits. Do you need someone to commit the patch for you? Comment at: clang-tidy/modernize/RedundantVoidArgCheck.cpp:241 +SourceLocation End, Begin; +

[PATCH] D49800: [clang-tidy: modernize] modernize-redundant-void-arg crashes when a function body is in a macro

This revision was automatically updated to reflect the committed changes. Closed by commit rL339433: [clang-tidy: modernize] modernize-redundant-void-arg crashes when a function… (authored by alexfh, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://revie

[PATCH] D49800: [clang-tidy: modernize] modernize-redundant-void-arg crashes when a function body is in a macro

alexfh added a comment. I've fixed the comments and committed the patch myself. Hope that's fine by you. Repository: rL LLVM https://reviews.llvm.org/D49800 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mai

[PATCH] D46633: [analyzer] add range check for InitList lookup

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Thank you for the fix! LG The fix looks trivial and I'll commit your patch to unblock our internal release. If there are comments from other reviewers, they can be resolved post-commit. Rep

[PATCH] D46633: [analyzer] add range check for InitList lookup

This revision was automatically updated to reflect the committed changes. Closed by commit rL331870: Fixes issue introduced by r331556. (authored by alexfh, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D46633?vs=145880&id=145899#toc

[PATCH] D46602: [clang-tidy] Store checks profiling info as YAML files

alexfh added a comment. In https://reviews.llvm.org/D46602#1092111, @rja wrote: > +1 for JSON Could you explain why would you use YAML or JSON for this? How are you going to use/process this data? Repository: rL LLVM https://reviews.llvm.org/D46602 _

[PATCH] D46602: [clang-tidy] Store checks profiling info as YAML files

alexfh added a comment. Roman, it looks to me that a simpler storage scheme would be sufficient. For example, MMDDhhmmss-InputFileName.cpp.csv. Main things are: 1. include a timestamp, so there's no need to overwrite old results, 2. include just the name of the file without any parent direc

[PATCH] D46603: [Support] TimerGroup changes

alexfh added a comment. I wonder why JSON? What kind of a tool do folks use to process the outputs of printJSONValue? Is there anything existing or is JSON used "just in case"? I personally use either spreadsheets, python or shell when I deal with this kind of data, and all three of them can wo

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

alexfh added inline comments. Comment at: clang-tidy/modernize-use-auto-min-type-name-length.cpp:61-83 +long int li = static_cast(foo()); +// CHECK-FIXES-0-0: auto li = {{.*}} +// CHECK-FIXES-0-5: auto li = {{.*}} +// CHECK-FIXES-1-0: auto li = {{.*}} +// CHE

[PATCH] D33844: [clang-tidy] terminating continue check

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D33844#1095862, @koldaniel wrote: > In https://reviews.llvm.org/D33844#1086565, @alexfh wrote: > > > > > > Which comments do you think are still relevant? Weird. W

[PATCH] D46602: [clang-tidy] Store checks profiling info as YAML files

alexfh added a comment. In https://reviews.llvm.org/D46602#1092902, @lebedev.ri wrote: > In https://reviews.llvm.org/D46602#1092890, @alexfh wrote: > > > Roman, it looks to me that a simpler storage scheme would be sufficient. > > For example, MMDDhhmmss-InputFileName.cpp.csv. > > Main thin

[PATCH] D46602: [clang-tidy] Store checks profiling info as YAML files

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. In https://reviews.llvm.org/D46602#1095980, @lebedev.ri wrote: > In https://reviews.llvm.org/D46602#1095960, @alexfh wrote: > > > In https://reviews.llvm.org/D46602#1092902, @lebedev.

[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers

alexfh added a comment. In https://reviews.llvm.org/D46159#1098000, @pfultz2 wrote: > Is someone able to merge in my changes? Will do. https://reviews.llvm.org/D46159 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/c

[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers

alexfh added a comment. In https://reviews.llvm.org/D46159#1098018, @alexfh wrote: > In https://reviews.llvm.org/D46159#1098000, @pfultz2 wrote: > > > Is someone able to merge in my changes? > > > Will do. Please rebase the patch, it doesn't apply cleanly. https://reviews.llvm.org/D46159 _

[PATCH] D46602: [clang-tidy] Store checks profiling info as YAML files

alexfh added a comment. In https://reviews.llvm.org/D46602#1097954, @lebedev.ri wrote: > In https://reviews.llvm.org/D46602#1097902, @alexfh wrote: > > > In https://reviews.llvm.org/D46602#1095980, @lebedev.ri wrote: > > > > > In https://reviews.llvm.org/D46602#1095960, @alexfh wrote: > > > > > >

[PATCH] D46951: [clang-tidy] misc-unused-parameters - retain old behavior under StrictMode

alexfh created this revision. alexfh added reviewers: klimek, ilya-biryukov. Herald added a subscriber: xazax.hun. This addresses https://bugs.llvm.org/show_bug.cgi?id=37467. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46951 Files: clang-tidy/misc/UnusedParametersCheck.cpp

[PATCH] D33440: clang-format: better handle statement macros

alexfh added a comment. There are still outstanding comments. Repository: rC Clang https://reviews.llvm.org/D33440 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46939: [Timers] TimerGroup: add constructor from StringMap

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG Repository: rL LLVM https://reviews.llvm.org/D46939 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/m

[PATCH] D46938: [Timers] TimerGroup: make printJSONValues() method public

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG Repository: rL LLVM https://reviews.llvm.org/D46938 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/m

[PATCH] D46936: [Timers] TimerGroup::printJSONValues(): print mem timer with .mem suffix

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG Repository: rL LLVM https://reviews.llvm.org/D46936 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/m

[PATCH] D46659: [clang-tidy/google-readability-casting] Disable check for Objective-C++

alexfh accepted this revision. alexfh added a comment. LG Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46659 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46602: [clang-tidy] Store checks profiling info as YAML files

alexfh added a comment. A couple of comments from a cursory look. I'll try to look closer later this week. Comment at: clang-tidy/ClangTidyDiagnosticConsumer.h:173 + /// \brief Control storage of profile date. + void setStoreProfile(StringRef ProfilePrefix); + llvm::Optiona

[PATCH] D45177: CStringChecker, check strlcpy/strlcat

alexfh added a comment. This patch seems to cause an assertion failure: assert.h assertion failed at clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:427 in clang::ento::SVal clang::ento::SValBuilder::evalBinOp(clang::ento::ProgramStateRef, BinaryOperator::Opcode, clang::ento::SVal, clang::ento::

[PATCH] D45177: CStringChecker, check strlcpy/strlcat

alexfh added a comment. This is reproducible in r332425. Repository: rC Clang https://reviews.llvm.org/D45177 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers

This revision was automatically updated to reflect the committed changes. Closed by commit rL332609: [clang-tidy] Add a flag to enable alpha checkers (authored by alexfh, committed by ). Herald added subscribers: llvm-commits, klimek. Changed prior to commit: https://reviews.llvm.org/D46159?vs=

[PATCH] D45177: CStringChecker, check strlcpy/strlcat

alexfh added a comment. See https://bugs.llvm.org/show_bug.cgi?id=37503 for a test case. Repository: rC Clang https://reviews.llvm.org/D45177 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/c

[PATCH] D45177: CStringChecker, check strlcpy/strlcat

alexfh added a comment. In https://reviews.llvm.org/D45177#1103774, @devnexen wrote: > In https://reviews.llvm.org/D45177#1103162, @alexfh wrote: > > > See https://bugs.llvm.org/show_bug.cgi?id=37503 for a test case. > > > I was unable to reproduce both FreeBSD and Linux. Plus my changes come aft

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

alexfh added a comment. In https://reviews.llvm.org/D38171#878814, @lebedev.ri wrote: > On a somewhat related note, since this is already talking about aliases > > I feel like the current handling of the clang-tidy check aliases needs > adjusting. > If one enables all the checks (`Checks: '*'`

[PATCH] D37023: [analyzer] Fix bugreporter::getDerefExpr() again.

alexfh added a comment. Thank you, Artem! Repository: rL LLVM https://reviews.llvm.org/D37023 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

alexfh added inline comments. Comment at: clang-tidy/modernize/UseEqualsDefaultCheck.cpp:119-120 + + // A defaulted default constructor of a union with a field with a non trivial + // default constructor would be deleted. + if (Record->isUnion()) { malcolm.par

[PATCH] D38358: [analyzer] Fix autodetection of getSVal()'s type argument.

alexfh added a comment. Thank you for the fix! It seems to be the largest source of SA crashes on our code at the moment. https://reviews.llvm.org/D38358 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/

[PATCH] D38458: Fix assertion failure in thread safety analysis (PR34800).

alexfh created this revision. Fix an assertion failure and clean up unused code relevant to the fixed logic. https://reviews.llvm.org/D38458 Files: include/clang/Analysis/Analyses/ThreadSafetyTIL.h test/SemaCXX/warn-thread-safety-analysis.cpp Index: test/SemaCXX/warn-thread-safety-analysi

[PATCH] D38458: Fix assertion failure in thread safety analysis (PR34800).

alexfh added inline comments. Comment at: include/clang/Analysis/Analyses/ThreadSafetyTIL.h:931-936 +if (!SlotName) { + std::string Buffer; + llvm::raw_string_ostream OS(Buffer); + Cvdecl->printName(OS); + SlotName = OS.str(); +} BTW,

[PATCH] D38458: Fix assertion failure in thread safety analysis (PR34800).

alexfh updated this revision to Diff 117650. alexfh added a comment. Get rid of a local string variable. https://reviews.llvm.org/D38458 Files: include/clang/Analysis/Analyses/ThreadSafetyTIL.h test/SemaCXX/warn-thread-safety-analysis.cpp Index: test/SemaCXX/warn-thread-safety-analysis.cp

[PATCH] D38458: Fix assertion failure in thread safety analysis (PR34800).

This revision was automatically updated to reflect the committed changes. Closed by commit rL314895: Fix assertion failure in thread safety analysis (PR34800). (authored by alexfh). Repository: rL LLVM https://reviews.llvm.org/D38458 Files: cfe/trunk/include/clang/Analysis/Analyses/ThreadSa

[PATCH] D38549: [clang-tidy] Link targets and Asm parsers

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. The first attempt to fix this (https://reviews.llvm.org/D17981) was stuck on figuring out whether we care about the binary size. It seems to me that the binary size is secondary to maintaining

[PATCH] D38284: [clang-tidy] Fix google-readability-namespace-comments handling of C++17 nested namespaces

alexfh added inline comments. Comment at: clang-tidy/readability/NamespaceCommentCheck.cpp:97 + } + while (Lexer::getRawToken(LBracketLocation, Tok, Sources, getLangOpts()) || + !Tok.is(tok::l_brace)) { The check started triggering an assertion failure

[PATCH] D38284: [clang-tidy] Fix google-readability-namespace-comments handling of C++17 nested namespaces

alexfh added inline comments. Comment at: clang-tidy/readability/NamespaceCommentCheck.cpp:97 + } + while (Lexer::getRawToken(LBracketLocation, Tok, Sources, getLangOpts()) || + !Tok.is(tok::l_brace)) { alexfh wrote: > The check started triggering an as

[PATCH] D38284: [clang-tidy] Fix google-readability-namespace-comments handling of C++17 nested namespaces

alexfh added inline comments. Comment at: clang-tidy/readability/NamespaceCommentCheck.cpp:97 + } + while (Lexer::getRawToken(LBracketLocation, Tok, Sources, getLangOpts()) || + !Tok.is(tok::l_brace)) { alexfh wrote: > alexfh wrote: > > The check starte

[PATCH] D38284: [clang-tidy] Fix google-readability-namespace-comments handling of C++17 nested namespaces

alexfh added inline comments. Comment at: clang-tidy/readability/NamespaceCommentCheck.cpp:97 + } + while (Lexer::getRawToken(LBracketLocation, Tok, Sources, getLangOpts()) || + !Tok.is(tok::l_brace)) { alexfh wrote: > alexfh wrote: > > alexfh wrote: >

[PATCH] D38549: [clang-tidy] Link targets and Asm parsers

alexfh added a comment. Zachary, do you plan to commit this? I'd also suggest adding a regression test. Thanks! https://reviews.llvm.org/D38549 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/c

[PATCH] D39105: Patch bug 27628 (clang-tidy should exit with a failure code when there are errors)

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

[PATCH] D38549: [clang-tidy] Link targets and Asm parsers

alexfh added inline comments. Comment at: clang-tools-extra/test/clang-tidy/hicpp-no-assembler.cpp:13-16 + _asm { +mov al, 2; +// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: do not use inline assembler in safety-critical code [hicpp-no-assembler] + } Will

[PATCH] D38549: [clang-tidy] Link targets and Asm parsers

alexfh added inline comments. Comment at: clang-tools-extra/test/clang-tidy/hicpp-no-assembler.cpp:13-16 + _asm { +mov al, 2; +// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: do not use inline assembler in safety-critical code [hicpp-no-assembler] + } alex

[PATCH] D39105: Patch bug 27628 (clang-tidy should exit with a failure code when there are errors)

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. This patch breaks a number of tests. Please run the tests and fix the failures. https://reviews.llvm.org/D39105 ___ cfe-commits mailing

[PATCH] D39105: Patch bug 27628 (clang-tidy should exit with a failure code when there are errors)

alexfh added inline comments. Comment at: clang-tidy/tool/ClangTidyMain.cpp:452 + if (FoundErrors) { + return 1; Failed tests reminded me of the -fix-errors flag. I suppose, clang-tidy should not return an error code, when all errors it noticed, contain

[PATCH] D39121: [clang-tidy] Misplaced Operator in Strlen in Alloc

alexfh added a comment. Apart from all other comments, I think, this check would better be placed under bugprone/. Repository: rL LLVM https://reviews.llvm.org/D39121 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/

[PATCH] D39208: [Analyzer] Do not use static storage to for implementations created in BodyFarm.cpp

alexfh added inline comments. Comment at: cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp:607 +AnalysisDeclContextManager::~AnalysisDeclContextManager() { + if (!BdyFrm) +delete BdyFrm; This is an almost guaranteed memory leak. I think, you meant `if (BdyFrm)

[PATCH] D39015: [Analyzer] Always use non-reference types when creating expressions in BodyFarm, removes std::call_once crash

alexfh added inline comments. Comment at: cfe/trunk/lib/Analysis/BodyFarm.cpp:366 +M.makeLvalueToRvalue(D->getParamDecl(i), + /* RefersToEnclosingVariableOrCapture= */ false)); Remove the spaces inside the comment. `/*ParamNa

[PATCH] D39201: [Analyzer] Handle implicit function reference in bodyfarming std::call_once

alexfh added inline comments. Comment at: cfe/trunk/test/Analysis/call_once.cpp:298 + param = 42; +}; +void test_implicit_funcptr() { Any reason for the stray semicolon here? Repository: rL LLVM https://reviews.llvm.org/D39201 ___

[PATCH] D39220: [Analyzer] Store BodyFarm in std::unique_ptr

alexfh added inline comments. Comment at: include/clang/Analysis/AnalysisDeclContext.h:424 /// methods during the analysis. - BodyFarm *BdyFrm = nullptr; + std::unique_ptr BdyFrm; BdyFrm is somewhat cryptic. Maybe Farm, Bodies or something else that is no

[PATCH] D39015: [Analyzer] Always use non-reference types when creating expressions in BodyFarm, removes std::call_once crash

alexfh added inline comments. Comment at: cfe/trunk/lib/Analysis/BodyFarm.cpp:366 +M.makeLvalueToRvalue(D->getParamDecl(i), + /* RefersToEnclosingVariableOrCapture= */ false)); george.karpenkov wrote: > alexfh wrote: > > Remo

[PATCH] D39220: [Analyzer] Store BodyFarm in std::unique_ptr

alexfh added inline comments. Comment at: include/clang/Analysis/AnalysisDeclContext.h:424 /// methods during the analysis. - BodyFarm *BdyFrm = nullptr; + std::unique_ptr BdyFrm; george.karpenkov wrote: > alexfh wrote: > > BdyFrm is somewhat cryptic. Mayb

[PATCH] D49800: [clang-tidy: modernize] modernize-redundant-void-arg crashes when a function body is in a macro

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. Thank you for working on this! Comment at: clang-tidy/modernize/RedundantVoidArgCheck.cpp:241 +SourceLocation End = +Lambda->getBody()->getLocStart().is

[PATCH] D49918: [clang-tidy] Sequence declaration in while statement before the condition

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG modulo outstanding comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49918 ___ cfe-commits mailing list cfe-commits@li

[PATCH] D50060: [clang-tidy] add all clang-tidy modules to plugin

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50060 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llv

[PATCH] D49890: Clang-Tidy Export Problem

alexfh added a comment. Could you describe the specific problem you're solving and provide an example? As mentioned by others, a test would be very welcome as well. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49890 ___ cfe-commi

[PATCH] D49918: [clang-tidy] Sequence init statements, declarations, and conditions correctly in if, switch, and while

alexfh accepted this revision. alexfh added a comment. Still LG Comment at: test/clang-tidy/bugprone-use-after-move.cpp:1141 +A a1; +if (A a2= std::move(a1)) { + std::move(a2); nit: clang-format this, please. Repository: rCTE Clang Tools Extra

[PATCH] D42730: [clang-tidy]] Add check for use of types/classes/functions from header which are deprecated and removed in C++17

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG Comment at: clang-tidy/modernize/AvoidFunctionalCheck.h:19 + +/// Check for several deprecated types and classes from header +/// aaron.ballman wrote: >

[PATCH] D42730: [clang-tidy]] Add check for use of types/classes/functions from header which are deprecated and removed in C++17

alexfh added inline comments. Comment at: clang-tidy/modernize/ModernizeTidyModule.cpp:50 +CheckFactories.registerCheck( +"modernize-avoid-functional"); CheckFactories.registerCheck( aaron.ballman wrote: > I'm not keen on this name -- it suggests

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

alexfh added a reviewer: ioeric. alexfh added a comment. In https://reviews.llvm.org/D43500#1013497, @aaron.ballman wrote: > > It seems, when they are apply directly from clang-tidy, the RefactoringTool > > engine is smart enough to remove trailing tokens. However, when fixes are > > exported,

[PATCH] D33531: Clang-tidy readability: avoid const value return

alexfh added a comment. > The check ignores returns of const pointers (meaning * const, not const *); > while pointless, these are not particularly harmful. It could be made laxer > by ignoring all trivially copyable types (on the grounds that they won't have > interesting move constructors/ass

[PATCH] D33531: Clang-tidy readability: avoid const value return

alexfh added inline comments. Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:27 + // skip those too. + Finder->addMatcher(functionDecl(returns(qualType( +isConstQualified(), How about just matching definitions to avoid duplicate warnings?

[PATCH] D33531: Clang-tidy readability: avoid const value return

alexfh added a comment. > Would it make sense to silence this diagnostic in the presence of also > checking for cert-dcl21-cpp for such operators? Currently there's no mechanism in clang-tidy to express dependencies or compatibility issues between checks. Moreover, we already have checks that a

[PATCH] D33623: Make the parser close parens for you on EOF

alexfh added a comment. Sam knows this stuff much better. https://reviews.llvm.org/D33623 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33497: [clang-tidy] check for __func__/__FUNCTION__ in lambdas

alexfh added inline comments. Comment at: clang-tidy/misc/LambdaFunctionNameCheck.cpp:61-62 +void LambdaFunctionNameCheck::registerPPCallbacks(CompilerInstance &Compiler) { + Compiler.getPreprocessor().addPPCallbacks(std::unique_ptr( + new MacroExpansionsWithFileAndLine(&Su

[PATCH] D33531: Clang-tidy readability: avoid const value return

alexfh added a comment. In https://reviews.llvm.org/D33531#767628, @aaron.ballman wrote: > In https://reviews.llvm.org/D33531#767059, @alexfh wrote: > > > > Would it make sense to silence this diagnostic in the presence of also > > > checking for cert-dcl21-cpp for such operators? > > > > Curren

[PATCH] D33304: [clang-tidy] Add a new module Android and three new checks.

alexfh added a comment. It's awesome to see another two checks, and it seems to justify adding a separate module, but I'd prefer the other two checks to be sent for review as separate patches to make the review easier and faster. https://reviews.llvm.org/D33304 _

[PATCH] D33304: [clang-tidy] Add a new module Android and three new checks.

alexfh added a comment. In https://reviews.llvm.org/D33304#768961, @yawanng wrote: > In https://reviews.llvm.org/D33304#768731, @alexfh wrote: > > > It's awesome to see another two checks, and it seems to justify adding a > > separate module, but I'd prefer the other two checks to be sent for re

[PATCH] D33304: [clang-tidy][Part1] Add a new module Android and three new checks.

alexfh added a comment. In https://reviews.llvm.org/D33304#771215, @hokein wrote: > Thanks for the contributions. > > All your three checks are not android specific -- because they are checking > POSIX APIs (`open`, `creat`, `fopen`), which are more likely related to > POSIX. So personally, I'm

[PATCH] D33497: [clang-tidy] check for __func__/__FUNCTION__ in lambdas

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

[PATCH] D33497: [clang-tidy] check for __func__/__FUNCTION__ in lambdas

This revision was automatically updated to reflect the committed changes. Closed by commit rL304570: [clang-tidy] check for __func__/__FUNCTION__ in lambdas (authored by alexfh). Changed prior to commit: https://reviews.llvm.org/D33497?vs=101224&id=101237#toc Repository: rL LLVM https://rev

[PATCH] D33827: [clang-tidy] misc-static-assert shouldn't flag assert(!"msg")

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

[PATCH] D20693: [clang-tidy] New checker to replace dynamic exception specifications

alexfh added a comment. In https://reviews.llvm.org/D20693#774693, @hintonda wrote: > In order to fix diagnostic corruption in some of the buildbot tests > (unable to reproduce locally): > > - make NoexceptMacro a static variable so it's lifetime doesn't end when > UseNoexceptCheck is destroyed

[PATCH] D20693: [clang-tidy] New checker to replace dynamic exception specifications

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. In any case, I'm strongly against these hacks, please revert them. https://reviews.llvm.org/D20693 ___ cfe-commits mailing list cfe-com

[PATCH] D33982: [clang-format] Fix alignment of preprocessor trailing comments

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG Comment at: lib/Format/WhitespaceManager.cpp:112 +assert(PreviousOriginalWhitespaceEndOffset <= OriginalWhitespaceStartOffset); +StringRef Text(SourceMgr.getChara

[PATCH] D20693: [clang-tidy] New checker to replace dynamic exception specifications

alexfh added a comment. In https://reviews.llvm.org/D20693#775014, @hintonda wrote: > - Only pass %2 parameter if %2 is included in format. I thought, DiagnosticsBuilder handles placeholders in conditional parts correctly. Did you find an evidence of the opposite? Can you add a test that cons

[PATCH] D20693: [clang-tidy] New checker to replace dynamic exception specifications

alexfh added a comment. In https://reviews.llvm.org/D20693#775153, @alexfh wrote: > In https://reviews.llvm.org/D20693#775014, @hintonda wrote: > > > - Only pass %2 parameter if %2 is included in format. > > > I thought, DiagnosticsBuilder handles placeholders in conditional parts > correctly. D

[PATCH] D20693: [clang-tidy] New checker to replace dynamic exception specifications

alexfh added a comment. In https://reviews.llvm.org/D20693#775197, @hintonda wrote: > I have not, as yet, been able to reproduce the buildbot failures. They were > essentially intermittent seg-faults, and corrupt diag output. > > I will work on creating a test that can reproduce the problem.

[PATCH] D33841: [clang-tidy] redundant keyword check

alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: docs/clang-tidy/checks/readability-redundant-keyword.rst:8 + +`extern` is redundant in function declarations + Could you explain, w

[PATCH] D20693: [clang-tidy] New checker to replace dynamic exception specifications

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Now that you found the root cause, this looks like a proper fix ;) Thank you! LG https://reviews.llvm.org/D20693 ___ cfe-commits mailing list cf

[PATCH] D34002: [clang-tidy] When" -fno-exceptions is used", this warning is better to be suppressed.

alexfh added a comment. IIUC, when `vector` (for a class `T` that has both move and copy constructors) resizes, it will prefer move constructors, but only if they're declared `noexcept`. This is true even if `-fno-exceptions` is on. So I don't think this check should depend on `-fno-exceptions

[PATCH] D34002: [clang-tidy] When" -fno-exceptions is used", this warning is better to be suppressed.

alexfh added a comment. Here's a small test: https://godbolt.org/g/nNZgUb (look for `T::T(` and `S::S(` calls, adding `-fno-exceptions` doesn't change which constructors are called). https://reviews.llvm.org/D34002 ___ cfe-commits mailing list cfe-

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

alexfh added a comment. I would be interested in seeing the results of this check's run on LLVM+Clang code. https://reviews.llvm.org/D33722 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c

[PATCH] D33365: [clang-tidy] misc-assertion-count: A New Check

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. I guess, this check should go to `readability` or elsewhere, but definitely not to `misc`. Another big question is whether it's reasonable to set up specific ratio limits on the den

[PATCH] D33365: [clang-tidy] misc-assertion-count: A New Check

alexfh added a comment. In https://reviews.llvm.org/D33365#775880, @lebedev.ri wrote: > In https://reviews.llvm.org/D33365#775860, @alexfh wrote: > > > I guess, this check should go to `readability` or elsewhere, but definitely > > not to `misc`. > > > Hmm, `misc` may be a bad place for this, bu

[PATCH] D20693: [clang-tidy] New checker to replace dynamic exception specifications

alexfh added a comment. In https://reviews.llvm.org/D20693#776030, @hintonda wrote: > Great, thanks for you help. > > Could you commit this for me? Sure, running tests... https://reviews.llvm.org/D20693 ___ cfe-commits mailing list cfe-commits@li

[PATCH] D20693: [clang-tidy] New checker to replace dynamic exception specifications

This revision was automatically updated to reflect the committed changes. Closed by commit rL304977: [clang-tidy] New checker to replace dynamic exception specifications (authored by alexfh). Changed prior to commit: https://reviews.llvm.org/D20693?vs=101860&id=101910#toc Repository: rL LLVM

[PATCH] D34002: [clang-tidy] When" -fno-exceptions is used", this warning is better to be suppressed.

alexfh added a comment. In https://reviews.llvm.org/D34002#776193, @chh wrote: > In https://reviews.llvm.org/D34002#775830, @alexfh wrote: > > > IIUC, when `vector` (for a class `T` that has both move and copy > > constructors) resizes, it will prefer move constructors, but only if > > they're

[PATCH] D34002: [clang-tidy] When" -fno-exceptions is used", this warning is better to be suppressed.

alexfh added inline comments. Comment at: test/clang-tidy/misc-noexcept-move-constructor.cpp:2 +// RUN: clang-tidy %s -checks="-*,misc-noexcept-move-constructor" -- -std=c++11 \ +// RUN: | FileCheck %s -check-prefix=CHECK-EXCEPTIONS \ +// RUN: -implicit-check-not="{{warning|

[PATCH] D34002: [clang-tidy] When" -fno-exceptions is used", this warning is better to be suppressed.

alexfh added a comment. In https://reviews.llvm.org/D34002#776551, @chh wrote: > Android source is suppressing misc-noexcept-move-constructor warnings > because -fno-exceptions is used and Android does not like to add more > exception specific code. As I've said, the lack of `noexcept` on mov

[PATCH] D33304: [clang-tidy][Part1] Add a new module Android and three new checks.

alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: clang-tidy/android/FileOpenFlagCheck.cpp:60 + + LangOptions LangOpts = getLangOpts(); + SourceRange FlagsRange(FlagArg->getLocStart(), FlagArg->ge

[PATCH] D34002: [clang-tidy] When" -fno-exceptions is used", this warning is better to be suppressed.

alexfh added a comment. In https://reviews.llvm.org/D34002#776948, @chh wrote: > Thanks for providing the ODR reason for Android -fno-exceptions users to > change source code or suppress this warning themselves. I'm not sure I understand your comment. Could you clarify? Here's an explanation

[PATCH] D34524: [clang-tidy] Fix a false positive in modernize-use-nullptr.

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

  1   2   3   4   5   6   7   8   9   10   >