[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

whisperity added a comment. (Typing this in here so it is not forgotten.) @aaron.ballman suggested in http://reviews.llvm.org/D69560#inline-893813 that a case of argument and parameter swaps could be discovered between forward declarations and the definitions, i.e. void fn(int x, int y);

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

whisperity added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp:10 + +void declaration(int Param, int Other); // NO-WARN: No chance to change this function. + aaron.ballman wrote: >

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

whisperity added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp:10 + +void declaration(int Param, int Other); // NO-WARN: No chance to change this function. + aaron.ballman wrote: >

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

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:402-403 + +// Unfortunately, undersquiggly highlights are for some reason chewed up +// and not respected by diagnostics from Clang-Tidy. :( +

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

whisperity marked an inline comment as done. whisperity added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp:10 + +void declaration(int Param, int Other); // NO-WARN: No chance to change this function. +

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

whisperity added a comment. In D69560#2536570 , @steveire wrote: > I haven't read through all the comments, but the word 'easily' implies > 'desirable'. This check seems to be for finding params which are undesirably > swappable, right? The `easily`

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

whisperity marked 3 inline comments as done. whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:402-403 + +// Unfortunately, undersquiggly highlights are for some reason chewed up +// and not respected

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

whisperity marked 6 inline comments as done. whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:255 + std::string NodeTypeName = + Node->getType().getAsString(Node->getASTContext().getPrintingPolicy()); +

[PATCH] D95736: [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with `typedef` and `const &` diagnostics

whisperity created this revision. whisperity added a reviewer: aaron.ballman. whisperity added projects: clang, clang-tools-extra. Herald added subscribers: martong, gamesh411, Szelethus, rnkovacs, xazax.hun. whisperity requested review of this revision. The base patch only deals with strict

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

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:284 +/// The default value for the MinimumLength check option. +static constexpr unsigned DefaultMinimumLength = 2; + aaron.ballman wrote: >

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

whisperity updated this revision to Diff 319840. whisperity marked 9 inline comments as done. whisperity added a comment. - NFC Code style and organisation fixes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69560/new/

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

whisperity added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp:10 + +void declaration(int Param, int Other); // NO-WARN: No chance to change this function. + aaron.ballman wrote: > I think

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

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:58 + +namespace { + aaron.ballman wrote: > Is there a need for the anonymous namespace? (We typically only use them when > defining a class

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

whisperity updated this revision to Diff 319315. whisperity added a comment. - Added a fallback case to also diagnose when Clang parsed the two types to be canonically the same (we will elaborate a few cases of such equivalences later, such as `typedef`s) - Fixed Clang-Format and Clang-Tidy

[PATCH] D76545: [clang-tidy] Add a new check group 'experimental-'

whisperity abandoned this revision. whisperity added a comment. Obsolete. There was no community consensus, or even a large enough discussion about this. What's more, the only dependee of this patch has superseded in ellegance, dropping the need for this group. Repository: rG LLVM Github

[PATCH] D75041: [clang-tidy] Approximate implicit conversion issues in 'experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type'

whisperity planned changes to this revision. whisperity added a comment. This patch will be replaced with the refactoring of the base check. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75041/new/ https://reviews.llvm.org/D75041 ___

[PATCH] D78652: [clang-tidy] Add "ignore related parameters" heuristics to 'experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type'

whisperity planned changes to this revision. whisperity added a comment. Herald added a subscriber: shchenz. This patch will be replaced with the refactoring of the base check. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78652/new/

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

whisperity updated this revision to Diff 318220. whisperity retitled this revision from "[clang-tidy] Add 'experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type' check" to "[clang-tidy] Add 'bugprone-easily-swappable-parameters' check". whisperity edited the summary of this

[PATCH] D69560: [clang-tidy] Add 'experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type' check

whisperity added a comment. I have re-analysed Bitcoin and Xerces with the new version (not yet published to Phab), and the results are looking really good! There is an insignificant change in the number of reports, 1 or 2 new ones compared to the previous, all explained by the fact that

[PATCH] D69560: [clang-tidy] Add 'experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type' check

whisperity added a comment. In D69560#2487117 , @aaron.ballman wrote: > In D69560#2487093 , @whisperity > wrote: > >> Not sure what CVR-modelling's default should be... it finds less when "off", >> but too

[PATCH] D69560: [clang-tidy] Add 'experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type' check

whisperity added a comment. I have posted two questions to GitHub, mostly related to the guideline rule and how free the implementation could be: #1732 and #1733 , I think I tagged

[PATCH] D69560: [clang-tidy] Add 'experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type' check

whisperity added a comment. I was just about to write an issue to the CoreGuidelines project, but I was @vingeldal has posted about "relatedness" before me: https://github.com/isocpp/CppCoreGuidelines/issues/1579. It was in the list of //closed// issues. Herb Sutter has changed the rule's

[PATCH] D75041: [clang-tidy] Approximate implicit conversion issues in 'experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type'

whisperity updated this revision to Diff 315384. whisperity added a comment. Herald added a subscriber: shchenz. **Ignore one-way implicit conversions** One-way implicit conversions introduce too much noise, and except in very odd circumstances, do not provide a meaningful result. The stalwart

[PATCH] D69560: [clang-tidy] Add 'experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type' check

whisperity added a comment. Herald added a subscriber: shchenz. In D69560#2319340 , @aaron.ballman wrote: > Congrats on the SCAM acceptance, I hope the presentation goes well! The entire event was amazing, even if short. There was another paper which

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

whisperity added a comment. But of course, after having written all of that, I forgot to upload the results themselves...  F14738535: SuspiciousCall.sqlite Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

whisperity added a reviewer: aaron.ballman. whisperity added a comment. Right, let's bump. I've checked the output of the checker on a set of test projects (our usual testbed, I should say) which contains around 15 projects, half of which was C and the other half C++. All projects were analysed

[PATCH] D93222: [RFC][analyzer] Introduce MacroExpansionContext to libAnalysis

whisperity added inline comments. Comment at: clang/lib/Analysis/MacroExpansionContext.cpp:22 +: PP(PP), SM(PP.getSourceManager()), LangOpts(LangOpts) { + class MacroExpansionRangeRecorder : public PPCallbacks { +const Preprocessor xazax.hun wrote: >

[PATCH] D91948: [WIP][analyzer][doc] Add Container- and IteratorModeling developer docs

whisperity added a comment. Actually, while the explanation is understandable for me with additional knowledge about the representation... I think it would be useful to add the most simple example from the iterator checkers to the end of the document, how this whole thing ties together and how

[PATCH] D91948: [WIP][analyzer][doc] Add Container- and IteratorModeling developer docs

whisperity added a comment. In general, perhaps you should go over the rendered text once again and make use of //emph// and **bold** some more. Explanation looks okay from my part, although I'm really not knowledgeable about CSA internals. But there are plenty of "typesetting" issues. Do you

[PATCH] D84176: [CMake][CTE] Add "check-clang-extra-..." targets to test only a particular Clang extra tool

whisperity added a comment. In D84176#2300488 , @njames93 wrote: > Is this an artefact of how `check-clang-tools` also depends on all > clang-tools-extra targets. Yes, this was not changed in this patch. Fixing that will require a lot more CMake magic

[PATCH] D84176: [CMake][CTE] Add "check-clang-extra-..." targets to test only a particular Clang extra tool

This revision was automatically updated to reflect the committed changes. Closed by commit rG9d2ef5e74eea: [CMake][CTE] Add check-clang-extra-... targets to test only a particular… (authored by whisperity). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D84176: [CMake][CTE] Add "check-clang-extra-..." targets to test only a particular Clang extra tool

whisperity added a comment. Ping. Can this go in? Still using this on a local fork, and still feels nice to be able to specify just a single tool. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84176/new/ https://reviews.llvm.org/D84176 ___

[PATCH] D69560: [clang-tidy] Add 'experimental-cppcoreguidelines-avoid-adjacent-arguments-of-the-same-type' check

whisperity added a comment. I'd like to resurrect the discussion about this. Unfortunately, the concept of `experimental-` group (in D76545 ) has fallen silent, too. We will present the results of this analysis soon at IEEE SCAM (Source Code Analysis and

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp:43 +static bool isLiteral(const Expr *E) { + return isa(E) || + isa(E) || baloghadamsoftware wrote: > aaron.ballman wrote: > >

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

whisperity added a comment. Minor inline comments. Could you please add a test case where the constructor is defined out of line? So far in every test, the constructor //body// was inside the class. Something like: struct S { int i, j; S(); }; S::S() { i = 1; j = 2;

[PATCH] D87527: [ASTMatchers] Fix `hasBody` for the descendants of `FunctionDecl`

whisperity added a comment. What about http://clang.llvm.org/doxygen/classclang_1_1CXXConversionDecl.html ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87527/new/ https://reviews.llvm.org/D87527 ___

[PATCH] D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes

whisperity added inline comments. Comment at: clang/docs/ReleaseNotes.rst:509 + The currently beta-version tool is found in + (llvm-project repository)/clang/utils/analyzer/SATest.py. Let's put this into monospace font too. Otherwise LGTM from a readability

[PATCH] D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes

whisperity added inline comments. Comment at: clang/docs/ReleaseNotes.rst:439 - ... Static Analyzer ... here. Comment at: clang/docs/ReleaseNotes.rst:498 .. _release-notes-ubsan: @Szelethus Speaking of labels in the

[PATCH] D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes

whisperity added a comment. Some grammatical fixes and suggestions, inline. I might have absolutely butchered 80-col in the suggestions (thanks Phab for not showing any indication of line length...), so make sure you manually reformat the document before going forward!

[PATCH] D81272: [clang-tidy] New check `bugprone-redundant-branch-condition`

whisperity added a comment. In D81272#2218175 , @aaron.ballman wrote: > While I agree with your observation about data and flow sensitivity, I > approved on the belief that the check as-is provides enough utility to > warrant adding it as-is. If

[PATCH] D81272: [clang-tidy] New check `bugprone-redundant-branch-condition`

whisperity added a comment. In D81272#2218050 , @aaron.ballman wrote: > Thanks to the new info, I think the check basically LGTM. Can you add some > negative tests and documentation wording to make it clear that the check > doesn't currently handle all

[PATCH] D85728: [Analyzer] Support for the new variadic isa<> and isa_and_not_null<> in CastValueChecker

whisperity added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:289 + for (QualType CastToTy: CastToTyVec) { +if (CastFromTy->isPointerType()) + CastToTy = C.getASTContext().getPointerType(CastToTy); NoQ wrote: >

[PATCH] D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'.

whisperity added a comment. In D72705#2210255 , @balazske wrote: > More results in CodeChecker: emacs_errorreturn >

[PATCH] D85752: [Analyzer] Store the pointed/referenced type for dynamic casts

whisperity added a comment. @baloghadamsoftware Maybe there is a typo in the summary of the patch > then `` //is a// `` Shouldn't this be " is a "? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85752/new/ https://reviews.llvm.org/D85752

[PATCH] D85528: [analyzer] Fix cast evaluation on scoped enums in ExprEngine

whisperity added inline comments. Comment at: clang/test/Analysis/z3-refute-enum-crash.cpp:5 +// +// Requires z3 only for refutation. Works with both constraint managers. + You can have multiple `RUN` lines in the same test file, which will result in the tests

[PATCH] D84520: [Analyzer] Improve invalid dereference bug reporting in DereferenceChecker.

whisperity added a comment. In D84520#2206077 , @balazske wrote: > Do the null pointer and invalid pointer dereference belong to the same > checker, that is called //NullDereference//? I think both SA and Tidy auto-appends the checker's name into the

[PATCH] D85528: [analyzer] Fix cast evaluation on scoped enums in ExprEngine

whisperity added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:98-104 +const auto IsIntegralOrUnscopedCompleteEnumerationType = [](QualType Ty) { + const Type *CanonicalType = Ty.getCanonicalType().getTypePtr(); +

[PATCH] D85528: [analyzer] Fix cast evaluation on scoped enums in ExprEngine

whisperity added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:98-104 +const auto IsIntegralOrUnscopedCompleteEnumerationType = [](QualType Ty) { + const Type *CanonicalType = Ty.getCanonicalType().getTypePtr(); +

[PATCH] D85319: [analyzer] Get info from the LLVM IR for precision

whisperity added subscribers: dcoughlin, rjmccall, rsmith. whisperity added a comment. What will happen with the ability to analyse a translation unit whose target was not part of `LLVM_TARGETS_TO_BUILD` of the current `clang` binary? Will it break, because the binary lacks the information on

[PATCH] D83717: [clang-tidy] Add check fo SEI CERT item ENV32-C

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.cpp:64 +/// argument. +void ExitHandlerCheck::registerMatchers(MatchFinder *Finder) { + const auto IsRegisterFunction = aaron.ballman wrote: > whisperity wrote: > >

[PATCH] D83717: [clang-tidy] Add check fo SEI CERT item ENV32-C

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.cpp:33 +bool isExitFunction(StringRef FnName) { + return FnName == "_Exit" || FnName == "exit" || FnName == "quick_exit"; +} aaron.ballman wrote: > Because this

[PATCH] D75229: [clang-tidy] Add signal-in-multithreaded-program check

whisperity added a comment. In D75229#2189666 , @abelkocsis wrote: > I have managed to improve the checker, but could not set up the test files to > work only in POSIX platforms. Could you help me or show me an example? It's more so that, if I

[PATCH] D83717: [clang-tidy] Add check fo SEI CERT item ENV32-C

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.cpp:64 +/// argument. +void ExitHandlerCheck::registerMatchers(MatchFinder *Finder) { + const auto IsRegisterFunction = What happens if this test is run on C++ code

[PATCH] D84176: [CMake][CTE] Add "check-clang-extra-..." targets to test only a particular Clang extra tool

whisperity updated this revision to Diff 281922. whisperity retitled this revision from "[CMake][CTE] Add "check-clang-tools-..." targets to test only a particular Clang extra tool" to "[CMake][CTE] Add "check-clang-extra-..." targets to test only a particular Clang extra tool". whisperity

[PATCH] D78704: [analyzer][NFC] Add unittest for FalsePositiveRefutationBRVisitor

whisperity added inline comments. Comment at: clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp:19 +#include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h" +#include "llvm/Config/config.h" +#include "gtest/gtest.h" steakhal wrote: >

[PATCH] D77150: [Analyzer] New Option for ContainerModeling: AggressiveEraseModeling

whisperity added a comment. In D77150#2149870 , @dkrupp wrote: > Since the analyzer cannot cannot model the size of the containers just yet > (or precisely enough), what we are saying with this checker is to "always > check the return value of the

[PATCH] D84176: [CMake][CTE] Add "check-clang-tools-..." targets to test only a particular Clang extra tool

whisperity added a comment. @lebedev.ri If you have an idea on who's competent in accepting this change, please update the //reviewers// field, I'm in the dark here. Also, should the commands be called `check-clang-tools-clang-tidy` or `check-clang-`**`extra`**`-clang-tidy` instead?

[PATCH] D84176: [CMake][CTE] Add "check-clang-tools-..." targets to test only a particular Clang extra tool

whisperity created this revision. whisperity added a reviewer: klimek. whisperity added a project: clang-tools-extra. Herald added subscribers: cfe-commits, martong, gamesh411, Szelethus, dkrupp, rnkovacs, mgorny. Herald added a project: clang. Create targets `check-clang-tools-clang-tidy`,

[PATCH] D79437: [clang-tidy] Add fsetpos argument checker

whisperity added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-fsetpos-argument-check.cpp:1 +// RUN: %check_clang_tidy %s bugprone-fsetpos-argument-check %t + This is a C, not a C++ file, the extension should show it. In

[PATCH] D77493: [clang-tidy] Add do-not-refer-atomic-twice check

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/DoNotReferAtomicTwiceCheck.cpp:38 + diag(MatchedRef->getExprLoc(), + "Do not refer to '%0' atomic variable twice in an expression") + << MatchedVar->getName();

[PATCH] D75229: [clang-tidy] Add signal-in-multithreaded-program check

whisperity added a comment. In general, the test files should be cleaned up. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-config-std::thread.cpp:1 +// RUN: %check_clang_tidy %s bugprone-signal-in-multithreaded-program %t \

[PATCH] D81272: [clang-tidy] New check `misc-redundant-condition`

whisperity added a comment. As a future work, when something support `if`s, it should also support `?:`, and eliminate redundant conditionals from there, too. I believe this check (together with `misc-redundant-expr`) should go into the `readability-` group. CHANGES SINCE LAST ACTION

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

whisperity added inline comments. Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:119-120 + return "Invocation list file contains multiple references to the same " + "source" + " file."; +case

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

whisperity resigned from this revision. whisperity added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:26 +#include "llvm/ADT/Triple.h" +#include "llvm/Option/ArgList.h" #include

[PATCH] D79330: [Analyzer][VLASizeChecker] Check for VLA size overflow.

whisperity added a comment. In D79330#2035850 , @balazske wrote: > I was looking at CERT ARR32-C > > "Ensure size arguments

[PATCH] D79330: [Analyzer][VLASizeChecker] Check for VLA size overflow.

whisperity added inline comments. Comment at: clang/test/Analysis/vla-overflow.c:8 + if (x == BIGINDEX) { +// We expect here that size_t is a 64 bit value. +// Size of this array should be the first to overflow. While it's generally true nowadays,

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

whisperity requested changes to this revision. whisperity added a comment. This revision now requires changes to proceed. (Maybe this will make Phab not show "✅ Accepted"...) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75665/new/

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

whisperity added a comment. Please check the summary of the patch, it seems to contain old information as well. Comment at: clang/docs/analyzer/user-docs/CrossTranslationUnit.rst:210-212 +Preferably the same compilation database should be used when generating the external

[PATCH] D78704: [analyzer][NFC] Add unittest for FalsePositiveRefutationBRVisitor

whisperity added inline comments. Comment at: clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp:24 + do \ +if (!LLVM_WITH_Z3)

[PATCH] D78457: [analyzer][Z3-refutation] Fix refutation BugReporterVisitor bug and refactor

whisperity added a comment. @steakhal You might want to update the patch summary before committing this to the upstream (it still mentions "not needing a visitor" ) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78457/new/

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

whisperity commandeered this revision. whisperity added a reviewer: barancsuk. whisperity added a comment. Herald added subscribers: martong, Charusso, gamesh411, Szelethus. Assuming direct control. Previous colleagues and university mates departed for snowier pastures, time to try to do

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

whisperity updated this revision to Diff 259508. whisperity retitled this revision from "[clang-tidy] Suspicious Call Argument checker" to "[clang-tidy] Add 'readability-suspicious-call-argument' check". whisperity edited the summary of this revision. whisperity removed reviewers: varjujan,

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

whisperity added inline comments. Comment at: clang/docs/analyzer/user-docs/CrossTranslationUnit.rst:391 +Currently On-demand analysis is not supported with `scan-build-py`. \ No newline at end of file What's this line? Is this added by Phab, or is this a weird

[PATCH] D75041: [clang-tidy] Approximate implicit conversion issues in 'experimental-cppcoreguidelines-avoid-adjacent-arguments-of-the-same-type'

whisperity updated this revision to Diff 259324. whisperity retitled this revision from "[clang-tidy] Approximate implicit conversion issues for the 'experimental-cppcoreguidelines-avoid-adjacent-arguments-of-the-same-type' check" to "[clang-tidy] Approximate implicit conversion issues in

[PATCH] D69560: [clang-tidy] Add 'experimental-cppcoreguidelines-avoid-adjacent-arguments-of-the-same-type' check

whisperity marked an inline comment as done. whisperity added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.rst:136-139 +typedef T element_type; +

[PATCH] D75041: [clang-tidy] Approximate implicit conversion issues in 'experimental-cppcoreguidelines-avoid-adjacent-arguments-of-the-same-type'

whisperity marked 2 inline comments as done. whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/experimental/CppcoreguidelinesAvoidAdjacentParametersOfTheSameTypeCheck.cpp:747 OS << "..."; - } else + } else { // There are things like "GCC

[PATCH] D69560: [clang-tidy] Add 'experimental-cppcoreguidelines-avoid-adjacent-arguments-of-the-same-type' check

whisperity updated this revision to Diff 259320. whisperity added a comment. - Better organisation of code, cleanup of debug prints, fix nomenclature of functions - Explicitly mention the first and last param of the range for clarity - Downlift the logic of joining and breaking ranges to main

[PATCH] D78652: [clang-tidy] Add "ignore related parameters" heuristics to 'experimental-cppcoreguidelines-avoid-adjacent-arguments-of-the-same-type'

whisperity created this revision. whisperity added reviewers: aaron.ballman, alexfh, hokein, JonasToth, zporky. whisperity added projects: clang-tools-extra, clang. Herald added subscribers: cfe-commits, martong, gamesh411, dkrupp, rnkovacs, kbarton, nemanjai. whisperity added a parent revision:

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

whisperity added inline comments. Comment at: clang/docs/analyzer/user-docs/CrossTranslationUnit.rst:21 + +PCH based analysis +__ I think it's PCH-based with a -. Comment at:

[PATCH] D76545: [clang-tidy] Add a new check group 'experimental-'

whisperity added a comment. @aaron.ballman @njames93 Should I write up a pitch mail to cfe-dev about this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76545/new/ https://reviews.llvm.org/D76545 ___

[PATCH] D76541: [clang-tidy][NFC] Add missing check group docs and order entries

This revision was automatically updated to reflect the committed changes. Closed by commit rGdf5fa4873976: [clang-tidy][NFC] Add missing check group docs and order entries (authored by whisperity). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D76541: [clang-tidy][NFC] Add missing check group docs and order entries

whisperity added a comment. In D76541#1935163 , @njames93 wrote: > Forgive me if I'm wrong, but these kinds of changes typically don't require a > review. Ah, pardon me. This is the first time I'm touching the insides of Tidy. Repository: rG LLVM

[PATCH] D76545: [clang-tidy] Add a new check group 'experimental-'

whisperity added a comment. In D76545#1935603 , @aaron.ballman wrote: > I think we want to keep the experimental checks grouped to their parent > module rather than being in a module of their own. For this, wouldn't the fact that telling Tidy to

[PATCH] D69560: [clang-tidy] Add 'experimental-cppcoreguidelines-avoid-adjacent-arguments-of-the-same-type' check

whisperity updated this revision to Diff 251841. whisperity retitled this revision from "[clang-tidy] Add 'cppcoreguidelines-avoid-adjacent-arguments-of-same-type' check" to "[clang-tidy] Add 'experimental-cppcoreguidelines-avoid-adjacent-arguments-of-the-same-type' check". whisperity edited

[PATCH] D76541: [clang-tidy][NFC] Add missing check group docs and order entries

whisperity updated this revision to Diff 251839. whisperity retitled this revision from "[clang-tidy][docs][NFC] Check group doc line was missing from darwin & linuxkernel" to "[clang-tidy][NFC] Add missing check group docs and order entries". whisperity edited the summary of this revision.

[PATCH] D76545: [clang-tidy] Add a new check group 'experimental-'

whisperity created this revision. whisperity added a reviewer: aaron.ballman. whisperity added a project: clang-tools-extra. Herald added subscribers: cfe-commits, martong, gamesh411, Szelethus, arphaman, dkrupp, rnkovacs, xazax.hun, mgorny. Herald added a project: clang. whisperity added a

[PATCH] D69560: [clang-tidy] Add 'cppcoreguidelines-avoid-adjacent-arguments-of-same-type' check

whisperity added a comment. @aaron.ballman I've gone over LLVM (and a few other projects). Some general observations: - Length of `2` **is vile**. I understand that the C++CG rule says even lengths of 2 should be matched, but that is industrially infeasible unless one introduces such a rule

[PATCH] D76541: [clang-tidy][docs][NFC] Check group doc line was missing from darwin & linuxkernel

whisperity created this revision. whisperity added a reviewer: aaron.ballman. whisperity added a project: clang-tools-extra. Herald added subscribers: cfe-commits, martong, gamesh411, Szelethus, arphaman, dkrupp, rnkovacs, xazax.hun. Herald added a project: clang. Two directories for Tidy checks

[PATCH] D76287: [analysis][analyzer] Introduce the skeleton of a reaching definitions calculator

whisperity added inline comments. Comment at: clang/include/clang/Analysis/Analyses/ReachingDefinitions.h:79 + +struct VarAndCFGElementLess { + bool operator()(Definition Lhs, Definition Rhs) const { Shouldn't this be called `DefinitionLess` if this is the

[PATCH] D75171: [Analyzer] Fix for incorrect use of container and iterator checkers

whisperity added a comment. Also, //errors// should conceptually break the operation at hand, so this thing should be a warning instead? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75171/new/ https://reviews.llvm.org/D75171

[PATCH] D75171: [Analyzer] Fix for incorrect use of container and iterator checkers

whisperity added a comment. Do you have access to the `Driver` somehow? Instead of a `cerr` (-ish) output, something that is formatted like a "true" error diagnostic (or warning), such as `"no such file or directory"` would be much better, I fear this `[ERROR]` will simply be flooded away with

[PATCH] D75041: [WIP][clang-tidy] Approximate implicit conversion issues for the 'cppcoreguidelines-avoid-adjacent-arguments-of-same-type' check

whisperity added a reviewer: aaron.ballman. whisperity added subscribers: vingeldal, zporky. whisperity added a comment. **WIP** because there's a lot of debug stuff that should be cleared out from here. This is a crude patch. It works fancy, but the code is crude. Repository: rG LLVM Github

[PATCH] D69560: [clang-tidy] Add 'cppcoreguidelines-avoid-adjacent-arguments-of-same-type' check

whisperity added a comment. In D69560#1891167 , @aaron.ballman wrote: > Btw, we should update the terminology in the patch to use `parameter` instead > of `argument` (parameters are what the function declares, arguments are what > are passed in at the

[PATCH] D69560: [clang-tidy] Add 'cppcoreguidelines-avoid-adjacent-arguments-of-same-type' check

whisperity added a comment. @aaron.ballman @vingeldal I'll go over the findings (as I've run this on LLVM and various other projects, a few examples are uploaded in my first comment as HTML renders of the bug report!) soon, but I want to wait until a research paper I have based on this topic

[PATCH] D69560: [clang-tidy] Add 'cppcoreguidelines-avoid-adjacent-arguments-of-same-type' check

whisperity added a comment. In D69560#1889925 , @vingeldal wrote: > I think this value very well may strike a good balance in that compromise but > I still think it is a clear deviation from the guideline as specified. > IMO all deviations from the

[PATCH] D69560: [clang-tidy] Add 'cppcoreguidelines-avoid-adjacent-arguments-of-same-type' check

whisperity added a comment. In D69560#1890026 , @aaron.ballman wrote: > In D69560#1889925 , @vingeldal wrote: > > > Doesn't clang-tidy exclude warnings from system headers by default though? > > > Third-party SDKs

[PATCH] D69560: [clang-tidy] Add 'cppcoreguidelines-avoid-adjacent-arguments-of-same-type' check

whisperity added a comment. In D74463#1888270 , @aaron.ballman wrote: > I am also concerned about the false positives from this check because I don't > think there's going to be an easy heuristic for determining whether two > identifiers are "related"

[PATCH] D69560: [clang-tidy] Add 'cppcoreguidelines-avoid-adjacent-arguments-of-same-type' check

whisperity marked an inline comment as done. whisperity added a comment. I'd rather not call them //false positive// because the definition of `false positive` is ugly and mushy with regards to this check. This check attempts to enforce an interface rule, whether you(r project) wants to adhere

[PATCH] D75041: [WIP][clang-tidy] Approximate implicit conversion issues for the 'cppcoreguidelines-avoid-adjacent-arguments-of-same-type' check

whisperity created this revision. whisperity added a project: clang-tools-extra. Herald added subscribers: cfe-commits, martong, gamesh411, Szelethus, dkrupp, rnkovacs, kbarton, xazax.hun, nemanjai. Herald added a project: clang. whisperity added a parent revision: D69560: [clang-tidy] Add

[PATCH] D69560: [clang-tidy] Add 'cppcoreguidelines-avoid-adjacent-arguments-of-same-type' check

whisperity planned changes to this revision. whisperity added a comment. There are a few really minor bug fixes, test additions, documentation update, etc. coming along soon, but I've some more pressing matters. However, please feel free to review the patch as-is! CHANGES SINCE LAST ACTION

<    1   2   3   4   5   6   >