[PATCH] D50488: [Analyzer] Checker for non-determinism caused by sorting of pointer-like keys

whisperity added inline comments. Comment at: lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp:88 + + if (!II->getName().equals("sort")) +return; Brrr... `equals`. StringRef has a `==` and `!=` operator which accepts string literals on the other side,

[PATCH] D50488: [Analyzer] Checker for non-determinism caused by sorting of pointer-like keys

whisperity added a comment. The basics of the heuristics look okay as comparing pointers from non-continuous block of memory is undefined, it would be worthy to check if no compiler warning (perhaps by specifying `-W -Wall -Wextra -Weverything` and various others of these enable-all flags!) is

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length

whisperity added a comment. Whew, this is a big one. Generally looks good, although I would separate implementation detail functions a bit better, with perhaps more comments to move them apart a bit, it is really harsh to scroll through. Could you please re-run the findings on the projects? The

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length

whisperity added a comment. Generally grammar / phrasing things that have caught my eye. The rest of the code looks okay, bar my previous comment about better commenting / separating chunks of helper functions. Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:

[PATCH] D50488: [Analyzer] Checker for non-determinism caused by sorting of pointer-like keys

whisperity added a comment. Minor comments in-line, looking good on first glance. I'm not sure if we discussed, has this checker been tried on some in-the-wild examples? To see if there are some real findings or crashes? There is a good selection of projects here in this tool: https://github.co

[PATCH] D50488: [Analyzer] Checker for non-determinism caused by sorting of pointer-like keys

whisperity added a project: clang. whisperity added a comment. @Szelethus `clang-query` seems to sometimes not include matcher functions that are perfectly available in the code... I recently had some issue with using `isUserDefined()`, it was available in my code, but `clang-query` always reje

[PATCH] D46081: [analyzer] Expand conversion check to check more expressions for overflow and underflow

whisperity added a comment. Ping. Shouldn't let this thing go to waste. https://reviews.llvm.org/D46081 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50488: [Analyzer] Checker for non-determinism caused by sorting of pointer-like elements

whisperity added a subscriber: martong. whisperity added a comment. Make sure you use only the C++ projects like BitCoin, LLVM/Clang, ProtoBuf and such from the Xazax CSA Test suite because this checker is not really applicable to C projects. Generally I like the idea (personally I've ran into

[PATCH] D50488: [Analyzer] Checker for non-determinism caused by sorting of pointer-like elements

whisperity added a comment. In https://reviews.llvm.org/D50488#1225064, @george.karpenkov wrote: > Why explicitly skip C projects? We would not be able to match `std::X` > functions anyway. Why spend time constructing matchers and running on the AST when we are sure that generating any resul

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length

whisperity added a comment. https://reviews.llvm.org/D50353 has landed, so after a rebase this patch will not compile. https://reviews.llvm.org/D45050 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/lis

[PATCH] D51926: [scan-build-py] Prevent crashes of CTU analysis by suppressing warnings

whisperity added a comment. Will this properly synergise across compilers with user-specified warning options, such as `-Wall -Werror`? Repository: rC Clang https://reviews.llvm.org/D51926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D45094: [LibTooling] Make interface of VFS injection into ClangTool more user-friendly

whisperity added a comment. Softly pinging this. Perhaps we could discuss this and get it in before Clang7 rides off into the sunset? https://reviews.llvm.org/D45094 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-

[PATCH] D45095: [clang-tidy] Align usage of ClangTool interface with new VFS injection

whisperity added a comment. Akin to https://reviews.llvm.org/D45094, pinging this too. 🙂 https://reviews.llvm.org/D45095 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49228: [analyzer][UninitializedObjectChecker] Void pointer objects are casted back to their dynmic type in note message

whisperity added inline comments. Comment at: test/Analysis/cxx-uninitialized-object-ptr-ref.cpp:290 struct IntDynTypedVoidPointerTest1 { - void *vptr; // expected-note{{uninitialized pointee 'this->vptr'}} + void *vptr; // expected-note{{uninitialized pointee 'this->static_ca

[PATCH] D43120: [clang-tidy] New checker for exceptions that are created but not thrown

whisperity requested changes to this revision. whisperity added inline comments. This revision now requires changes to proceed. Comment at: clang-tidy/misc/ThrowKeywordMissingCheck.cpp:21 +void ThrowKeywordMissingCheck::registerMatchers(MatchFinder *Finder) { + // This is a C++

[PATCH] D43120: [clang-tidy] New checker for exceptions that are created but not thrown

whisperity resigned from this revision. whisperity added a comment. Works for me but I haven't any sayings in these. 😇 https://reviews.llvm.org/D43120 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/list

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

whisperity requested changes to this revision. whisperity added a comment. This revision now requires changes to proceed. In general, make sure the documentation page renders well in a browser. Mostly style and phrasing stuff inline: Comment at: clang-tidy/bugprone/NotNullTerm

[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

whisperity accepted this revision. whisperity added a comment. This revision is now accepted and ready to land. Ah, and the function names in the test files have been made more logical. Let's roll. 😉 https://reviews.llvm.org/D45532 ___ cfe-commits

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length

whisperity accepted this revision. whisperity added a comment. This revision is now accepted and ready to land. Have been looked at this far and wide too many times now. I say we roll this out and fix and improve later on, lest this only going into Clang 72. Results look promising, let's hope us

[PATCH] D33672: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker

whisperity added a comment. Herald added subscribers: donat.nagy, Szelethus, mikhail.ramalho, rnkovacs, szepet. Herald added a reviewer: george.karpenkov. //Soft ping.// https://reviews.llvm.org/D33672 ___ cfe-commits mailing list cfe-commits@lists

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length

whisperity added a comment. @aaron.ballman Neither I nor @Charusso has commit rights, could you please commit this in our stead? https://reviews.llvm.org/D45050 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/m

[PATCH] D53334: [Frontend] Show diagnostics on prebuilt module configuration mismatch too

whisperity created this revision. whisperity added reviewers: rsmith, doug.gregor. whisperity added a project: clang. Herald added subscribers: Szelethus, dkrupp, rnkovacs. The current version only emits the below error for a module (attempted to be loaded) from the `prebuilt-module-path`: er

[PATCH] D52790: [analyzer][PlistMacroExpansion] New flag to convert macro expansions to events

whisperity accepted this revision. whisperity added a comment. This revision is now accepted and ready to land. Herald added a subscriber: donat.nagy. Remove the custom file paths and URLs but other than that this is pretty trivial. https://reviews.llvm.org/D52790 ___

[PATCH] D53354: [WIP][NOT FOR COMMIT][PROTOTYPE] clang-scan-deps: dependency scanning tool rough prototype

whisperity added a comment. With regards to https://reviews.llvm.org/D53352: you can change the diff related to a patch whilst keeping discuccion and metadata of the diff. Please add a generic description to the diff for an easy skimming on what you are accomplishing. If I get this right, your

[PATCH] D52988: [analyzer][PlistMacroExpansion] Part 5.: Support for # and ##

whisperity added a comment. Looks good. What happens if the macro is to stringify a partially string argument? #define BOOYAH(x) #x "; ... std::string foo = BOOYAH(blabla) std::string foo2 = BOOYAH("blabla) int x = 2; Not sure if these cases are even valid C(XX), but if they ar

[PATCH] D53076: [analyzer] Enhance ConditionBRVisitor to write out more information

whisperity added inline comments. Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:238 + /// message on that constraint being changed. + bool isChangedOrInsertConstraint(ConstraintMap &Constraints, const Stmt *Cond, +

[PATCH] D53334: [Frontend] Show diagnostics on prebuilt module configuration mismatch too

whisperity added a comment. @dblaikie I have created a test, but unfortunately `%clang_cpp` in LIT invokes `clang --driver-mode=cpp` which is not the same as if `clang++` is called. I'm trying to construct the following command-line `clang++ -fmodules-ts -fprebuilt-module-path=%t/mods --precomp

[PATCH] D53334: [Frontend/Modules] Show diagnostics on prebuilt module configuration mismatch too

whisperity updated this revision to Diff 170832. whisperity retitled this revision from "[Frontend] Show diagnostics on prebuilt module configuration mismatch too" to "[Frontend/Modules] Show diagnostics on prebuilt module configuration mismatch too". whisperity added a comment. Herald added a su

[PATCH] D53812: [Analyzer] Iterator Checker - Forbid increments past the begin() and decrements past the end() of containers

whisperity added inline comments. Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:420-465 if (ChecksEnabled[CK_InvalidatedIteratorChecker] && isAccessOperator(Func->getOverloadedOperator())) { // Check for any kind of access of invalidated iterato

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length

whisperity added a reviewer: dyung. whisperity added a subscriber: dyung. whisperity added a comment. In https://reviews.llvm.org/D45050#1280831, @dyung wrote: > I have attached a bzipped preprocessed file that I can confirm will show the > failure if built with clang++ version 3.7.1, specifical

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

whisperity added a comment. In D35068#1361902 , @Szelethus wrote: > Edit: it doesn't, but CMake is mostly a C project and it does! CMake really isn't a C project if you look at what language it actually uses - the C files come from tests and system util

[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable

whisperity added inline comments. Comment at: docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst:18 +This for loop is an infinite loop because the short type can't represent all +values in the [0..size] interval. + Format the interval as code (monospace)

[PATCH] D52795: [analyzer][PlistMacroExpansion] Part 3.: Macro arguments are expanded

whisperity added inline comments. Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:1018 +auto It = CurrExpArgTokens.begin(); +while (It != CurrExpArgTokens.end()) { + if (It->isNot(tok::identifier)) { xazax.hun wrote: > Maybe a for loop mor n

[PATCH] D53334: [Frontend/Modules] Show diagnostics on prebuilt module configuration mismatch too

whisperity planned changes to this revision. whisperity added a comment. In https://reviews.llvm.org/D53334#1288057, @dblaikie wrote: > In https://reviews.llvm.org/D53334#1273877, @whisperity wrote: > > > @dblaikie I have created a test, but unfortunately `%clang_cpp` in LIT > > invokes `clang -

[PATCH] D53334: [Frontend/Modules] Show diagnostics on prebuilt module configuration mismatch too

whisperity updated this revision to Diff 172908. whisperity added a comment. Test was added. Repository: rC Clang https://reviews.llvm.org/D53334 Files: lib/Frontend/CompilerInstance.cpp test/Modules/Inputs/module-mismatch.cppm test/Modules/mismatch-diagnostics.cpp Index: test/Module

[PATCH] D53334: [Frontend/Modules] Show diagnostics on prebuilt module configuration mismatch too

whisperity updated this revision to Diff 173124. whisperity added a comment. Yes, down the line I realised the function is not needed. (It emits a diagnostic because the diagnostic comes from comparing the AST file's config blocks to the current (at the time of import) compilation.) I have simp

[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable

whisperity added a comment. In https://reviews.llvm.org/D53974#1294385, @ztamas wrote: > I also tested on LLVm code. > The output is here: > https://gist.github.com/tzolnai/fe4f23031d3f9fdbdbf1ee38abda00a4 > > I found 362 warnings. > > Around 95% of these warnings are similar to the next exampl

[PATCH] D53069: [analyzer][www] Update avaible_checks.html

whisperity added a comment. Let's register the ID... Superseded by https://reviews.llvm.org/D54429. https://reviews.llvm.org/D53069 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54757: [clang-tidy] new check: bugprone-branch-clone

whisperity added a comment. Bar the previous comments, I really like this. The test suite is massive and well-constructed. Do we know of any real-world findings, maybe even from LLVM? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54757 __

[PATCH] D54757: [clang-tidy] new check: bugprone-branch-clone

whisperity added a comment. In https://reviews.llvm.org/D54757#1305306, @Eugene.Zelenko wrote: > In https://reviews.llvm.org/D54757#1305114, @whisperity wrote: > > > Bar the previous comments, I really like this. The test suite is massive > > and well-constructed. Do we know of any real-world fi

[PATCH] D57855: [analyzer] Reimplement checker options

whisperity added inline comments. Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:49 + "NoDiagnoseCallsToSystemHeaders", + "", + "false"> Let's put at least a FIXME here that the documentation for t

[PATCH] D58065: [analyzer] Document the frontend library

whisperity added inline comments. Comment at: docs/analyzer/developer-docs/FrontendLibrary.rst:85 +-fuse-ld=lld \ +../llvm + george.karpenkov wrote: > Information on compiling LLVM IMO should not be here. > Also, why Sphinx? Why X86? Why LLD and not gold?

[PATCH] D57860: [analyzer] Validate checker option names and values

whisperity added inline comments. Comment at: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp:329 + + // Insertation was successful -- CmdLineOption's constructor will validate + // whether values received from plugins or TableGen files are correct. Insertion

[PATCH] D58573: [analyzer] Move UninitializedObject out of alpha

whisperity added a comment. In case no bug reports //against// the checker are reported on the mailing list or Bugzilla, I wholeheartedly agree with kicking the ball here. As for the package, perhaps we could go into `optin.bugprone` or `optin.conventions`? (These are new package names...) Re

[PATCH] D57860: [analyzer] Validate checker option names and values

whisperity added inline comments. Comment at: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp:325 + std::string FullOption = + (llvm::Twine() + FullName + ":" + OptionName).str(); + baloghadamsoftware wrote: > Is this the most efficient way to concatenate `

[PATCH] D59465: [analyzer] Add example plugin for checker option handling

whisperity added a comment. @Szelethus I know the dependent patch D59464 will move `examples/analyzer-plugin` to `test/Analysis/plugins/...`, but this patch still seems to affect `examples/`. Are you sure this is the right diff? Because you are adding brand new

[PATCH] D57922: [analyzer] Insert checker options into AnalyzerOption::ConfigTable

whisperity added a comment. I think this is good. Patch still marked as //Needs review// for some reason. 😦 Can we look up this `blocking review` thing? Perhaps this could be marked ready to roll once the dependency patch is ironed out. Comment at: lib/StaticAnalyzer/Frontend

[PATCH] D57858: [analyzer] Add a new frontend flag to display all checker options

whisperity added a comment. @dcoughlin How would removing the `USAGE` part of the dump and keeping only the list of options and their formatted help sound? That way, this option will not invite the user to directly call the analyzer. In D57858#1432714 ,

[PATCH] D61260: [clang-tidy] Extend bugprone-sizeof-expression to check sizeof(pointers to structures)

whisperity added inline comments. Comment at: test/clang-tidy/bugprone-sizeof-expression.cpp:196 typedef const MyStruct TMyStruct; + typedef const MyStruct *PMyStruct; While I trust Clang and the matchers to unroll the type and still match, I'd prefer also

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length

whisperity added a comment. I have checked the results, thank you for uploading them, they look solid to me, although I'm not exactly a developer for these projects, without full understanding of what and where allocates and true path-sensitive analysis and memory modelling, they look good. (E.

[PATCH] D50488: [Analyzer] Checker for non-determinism caused by sorting of pointer-like elements

whisperity added a comment. Hey @mgrang! Let's see some results on some projects and answer NoQ's comment, then I think we can put this in for all the world to see. https://reviews.llvm.org/D50488 ___ cfe-commits mailing list cfe-commits@lists.llvm

[PATCH] D52390: [analyzer] Add StackSizeChecker to StaticAnalyzer

whisperity added a comment. One final nit apart from a few in-line comments (most of them are just arguing with @Szelethus anyways 😛): KB vs. KiB (remember how a 2 TB hard drive appears as 1.8 TB on your machine? Because it's TiB!) is one of my pet peeve

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length

whisperity added a subscriber: baloghadamsoftware. whisperity added inline comments. Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:1038 + + SmallString<128> NewAddNullTermExprStr; + NewAddNullTermExprStr = "\n"; aaron.ballman wrote: > This sh

[PATCH] D52742: [analyzer][WIP] Add macro expansions to the plist output

whisperity added a subscriber: gamesh411. whisperity added a comment. Your code looks good, just minor comments going on inline. Trying to think of more cases to test for, in case someone generously misuses FLMs, as seen here in an example if you scroll down a bit: https://gcc.gnu.org/onlinedocs

[PATCH] D52742: [analyzer][PlistMacroExpansion] Part 1.: New expand-macros flag

whisperity added inline comments. Comment at: test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist:6 + clang_version +clang version 8.0.0 (http://mainstream.inf.elte.hu/Szelethus/clang 85a6dda64587a5a18482f091cbcf020fbd3ec1dd) (https://github.com/llvm-mirr

[PATCH] D52742: [analyzer][PlistMacroExpansion] Part 1.: New expand-macros flag

whisperity added inline comments. Comment at: test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist:346 + + /home/eumakri/Documents/2codechecker_dev_env/llvm/tools/clang/test/Analysis/plist-macros-with-expansion.cpp + Same here, as @xaza

[PATCH] D50488: [Analyzer] Checker for non-determinism caused by sorting of pointer-like elements

whisperity added a comment. Yeah, it seems upstream has moved away due to @Szelethus' implementation of a much more manageable "checker dependency" system. You most likely will have to rebase your patch first, then check what you missed which got added to other merged, existing checkers. Repo

[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

whisperity added a comment. The Python code here still uses `mangled name` in their wording. Does this mean this patch is yet to be updated with the USR management in the parent patch? Comment at: tools/scan-build-py/libscanbuild/analyze.py:165 +with open(filename,

[PATCH] D66333: [analyzer] NonNullParamChecker and CStringChecker parameter number in checker message

whisperity added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp:195-196 + llvm::raw_svector_ostream OS(SBuf); + OS << "Null pointer passed as an argument to a 'nonnull' "; + OS << Idx << llvm::getOrdinalSuffix(Idx) << " parameter"; + ---

[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

whisperity added a comment. @NoQ Do you reckon these tests files are too long? Perhaps the one about this inheritance, that inheritance, diamond inheritance, etc. could be split into multiple files. Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:317 +def Ctor

[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

whisperity requested changes to this revision. whisperity added subscribers: gsd, dkrupp, o.gyorgy. whisperity added a comment. This revision now requires changes to proceed. Sorry, one comment has gone missing meanwhile, I'm still getting used to this interface and hit //Submit// early. =

[PATCH] D45407: [StaticAnalyzer] Added notes to the plist output

whisperity added a comment. @NoQ The problem with emitting notes as events is that we lose the information that the node was a `node`. How does Xcode behave with these notes? Does it ignore them, or can read them from the command-line output of the analyser? Repository: rC Clang https://rev

[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

whisperity added a comment. In https://reviews.llvm.org/D45532#1068700, @Szelethus wrote: > In https://reviews.llvm.org/D45532#1068647, @dkrupp wrote: > > > This bug report also mentions assignment operator. But for that a warning > > may be not so useful. In that case the members of the assigne

[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

whisperity requested changes to this revision. whisperity added a comment. This revision now requires changes to proceed. There is something that came up in my mind: Consider a construct like this: class A { A() { memset(X, 0, 10 * sizeof(int)); } int X[10]; }; I

[PATCH] D45095: [clang-tidy] Align usage of ClangTool interface with new VFS injection

whisperity added inline comments. Comment at: clang-tidy/tool/CMakeLists.txt:19 clangBasic + clangFrontend clangTidy ilya-biryukov wrote: > whisperity wrote: > > ilya-biryukov wrote: > > > Why do we need an extra dependency? > > In the current state of the

[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

whisperity added a comment. @george.karpenkov @NoQ `bugprone.` as a category sounds nice. It also nicely corresponds to the Clang-Tidy `bugprone-` category. It would not be nice to further fragment the "top levels" of checker categories. I can say with confidence that CodeChecker does not break

[PATCH] D46081: [analyzer] Expand conversion check to check more expressions for overflow and underflow

whisperity added a comment. While I understand extending the analyzer to cover more is a good approach, there is `-Wconversion` which seemingly covers this -- or at least the trivial case(?): #include #include void foo(unsigned x) { printf("%u\n", x); } int main() {

[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

whisperity added a comment. Two minor comments. With regards to the function naming, the problem with incremental counts is that they get out of sync, like they did with `fXpY` and such, and if someone wants to keep the count incremental down the file, adding any new test will result in an unn

[PATCH] D45094: [LibTooling] Make interface of VFS injection into ClangTool more user-friendly

whisperity added a comment. Pinging this as the talk has stalled. https://reviews.llvm.org/D45094 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32747: [Analyzer] Iterator Checker - Part 3: Invalidation check, first for (copy) assignments

whisperity added inline comments. Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:92 + // Whether iterator is valid + bool Valid; + Seeing that in line 106 you consider this record immutable, you might want to add a `const` on this field too. ===

[PATCH] D32845: [Analyzer] Iterator Checker - Part 4: Mismatched iterator checker for function parameters

whisperity added inline comments. Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:317 +def MismatchedIteratorChecker : Checker<"MismatchedIterator">, + HelpText<"Check for use of iterators of different containers where iterators of the same container are expecte

[PATCH] D32859: [Analyzer] Iterator Checker - Part 5: Move Assignment of Containers

whisperity added inline comments. Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:1061 + // first reassign all iterator positions to the new container which + // are not past the container (thus not greater or equal to the + // current "end"

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

whisperity created this revision. whisperity added reviewers: aaron.ballman, alexfh, Eugene.Zelenko, JonasToth, NoQ, Szelethus, xazax.hun, baloghadamsoftware, Charusso. whisperity added a project: clang-tools-extra. Herald added subscribers: cfe-commits, ormris, kbarton, mgorny, nemanjai. Herald a

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

whisperity added a comment. Herald added a subscriber: wuzish. A few interesting "true positive" findings: - F10568770: AnalysisDeclContext.cpp_9aaed563ddd9ebd73fdd228c2883b8e7.plist.html (Such cases with many `bool`s are being discussed on enhancing type s

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

whisperity added reviewers: Szelethus, baloghadamsoftware. whisperity edited subscribers, added: baloghadamsoftware, NoQ; removed: Szelethus. whisperity added a comment. @Szelethus and @baloghadamsoftware are colleagues to me whom are far more knowledgeable about check development and I want the

[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 "llvm/Support/ErrorHandli

[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`, `chec

[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? Repositor

[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 erase(

[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: > mgor

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

[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 index_error_code::invocation_list_file_not_foun

[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] 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(); atomic

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

[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 https:

[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: clang/docs/analyzer/user-docs/CrossTranslationUnit.rs

[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: D

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

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

[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; +type

[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 'expe

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

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

[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/ https://reviews.llvm.org

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

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

  1   2   3   4   5   6   >