[PATCH] D41451: Make DiagnosticIDs::getAllDiagnostics use std::vector

xazax.hun closed this revision. xazax.hun added a comment. Committed in https://reviews.llvm.org/rL321190 https://reviews.llvm.org/D41451 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm

[PATCH] D41444: [ASTImporterTest] Make testing under '-fdelayed-template-parsing' mandatory

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LGTM! Comment at: unittests/AST/ASTImporterTest.cpp:32 + +static RunOptions getRunOptionsForLanguage(Language Lang) { + ArgVector BasicArgs; I wonder

[PATCH] D41538: [analyzer] Fix some checker's output plist not containing the checker name #2

xazax.hun created this revision. xazax.hun added reviewers: NoQ, dcoughlin, george.karpenkov. Herald added subscribers: dkrupp, a.sidorin, rnkovacs, szepet, baloghadamsoftware, whisperity. Unfortunately, currently, the analyzer core sets the checker name after the constructor was already run. So

[PATCH] D37437: [analyzer] Fix some checker's output plist not containing the checker name

xazax.hun added a comment. Herald added subscribers: a.sidorin, rnkovacs. In case you do not like this solution, I uploaded an alternative approach: https://reviews.llvm.org/D41538 https://reviews.llvm.org/D37437 ___ cfe-commits mailing list cfe-co

[PATCH] D41538: [analyzer] Fix some checker's output plist not containing the checker name #2

xazax.hun updated this revision to Diff 128240. xazax.hun marked 5 inline comments as done. xazax.hun added a comment. - Address review comments. https://reviews.llvm.org/D41538 Files: include/clang/StaticAnalyzer/Core/BugReporter/BugType.h lib/StaticAnalyzer/Checkers/MallocChecker.cpp li

[PATCH] D41538: [analyzer] Fix some checker's output plist not containing the checker name #2

xazax.hun added inline comments. Comment at: include/clang/StaticAnalyzer/Core/BugReporter/BugType.h:54 +// initialization. +if (getCheckName().empty() && Checker) { + Check = Checker->getCheckName(); a.sidorin wrote: > Possibly I am missing the cont

[PATCH] D41538: [analyzer] Fix some checker's output plist not containing the checker name #2

xazax.hun updated this revision to Diff 128630. xazax.hun marked an inline comment as done. xazax.hun added a comment. - Address some review comments. https://reviews.llvm.org/D41538 Files: include/clang/StaticAnalyzer/Core/BugReporter/BugType.h lib/StaticAnalyzer/Checkers/MallocChecker.cpp

[PATCH] D41538: [analyzer] Fix some checker's output plist not containing the checker name #2

xazax.hun added inline comments. Comment at: include/clang/StaticAnalyzer/Core/BugReporter/BugType.h:51 StringRef getCategory() const { return Category; } - StringRef getCheckName() const { return Check.getName(); } + StringRef getCheckName() const { +// FIXME: This is a

[PATCH] D37437: [analyzer] Fix some checker's output plist not containing the checker name

xazax.hun added a comment. https://reviews.llvm.org/D41538 is superior and committed. https://reviews.llvm.org/D37437 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41538: [analyzer] Fix some checker's output plist not containing the checker name #2

xazax.hun added a comment. In https://reviews.llvm.org/D41538#969205, @sylvestre.ledru wrote: > It missed the 6.0 branching. Will you try to get it on this branch? > Thanks Sure! I will try to do so: https://reviews.llvm.org/rC321933 Repository: rC Clang https://reviews.llvm.org/D41538

[PATCH] D41815: [clang-tidy] implement check for goto

xazax.hun added a comment. A high level comment: while it is very easy to get all the gotos using grep, it is not so easy to get all the labels. So for this check to be complete, I think it would be useful to also find labels (possibly having a configuration option for that). Repository: rC

[PATCH] D41816: [analyzer] Model and check unrepresentable left shifts

xazax.hun added a comment. Overall looks good to me, one comment inline. I think it is good to have these checks to prevent the analyzer executing undefined behavior. Maybe this would make it more feasible to run the analyzer with ubsan :) In the future, it would be great to also look for these

[PATCH] D6550: ASTImporter: Fix missing SourceLoc imports

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. Great! Thanks! Repository: rC Clang https://reviews.llvm.org/D6550 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.

[PATCH] D41800: [analyzer] Use a custom program point for the check::NewAllocator callback.

xazax.hun accepted this revision. xazax.hun added a comment. LG! Comment at: include/clang/Analysis/ProgramPoint.h:592 + friend class ProgramPoint; + PostAllocatorCall() {} + static bool isKind(const ProgramPoint &Location) { Maybe `= default` is getting mor

[PATCH] D41406: [analyzer] Add a new checker callback, check::NewAllocator.

xazax.hun added a comment. Do you have a plan for the new false negatives when `c++-allocator-inlining` is on? Should the user mark allocation functions with attributes? Also, I was wondering if it would make sense to only have the PostCall callback return the casted memory region in these spec

[PATCH] D41799: [analyzer] PtrArithChecker: Update to use check::NewAllocator

xazax.hun added a comment. > (@xazax.hun - this is an alpha checker last touched by you, do you still have > plans for it?) I think this check could be very useful. Last I checked it was too noisy (e.g.: around 850 hits on LLVM), and I did plan to look into what are the main sources of false p

[PATCH] D41797: [analyzer] Suppress escape of this-pointer during construction.

xazax.hun added a comment. I am fine with suppressing the escape globally. I did see some code in the wild where the constructors registered the objects with a (global) map. But I think it is still easier to annotate code that does something unconventional than the other way around. Repository

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

xazax.hun added a reviewer: ilya-biryukov. xazax.hun added a subscriber: ilya-biryukov. xazax.hun added a comment. @ilya-biryukov Could you please provide some more details where the cyclic dependency is? I cannot reproduce the problem and usually cmake fails when there is a cyclic dependency a

[PATCH] D43928: [analyzer] Correctly measure array size in security.insecureAPI.strcpy

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

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

xazax.hun abandoned this revision. xazax.hun added a comment. Resubmitted in https://reviews.llvm.org/rL326439 Phabricator did not display the mailing list conversation. The point is, the circular dependency does not exist in the upstream version of clang. The reason is that CMake does not trac

[PATCH] D44100: [ASTImporter] Reorder fields after structure import is finished

xazax.hun added a comment. Hi Aleksei! Just a minor high level note. If https://reviews.llvm.org/D32947 will be accepted once, we will need to reorder friends as well. Or alternatively, we have to omit the check of friends in structural equivalence in https://reviews.llvm.org/D32947. Reposit

[PATCH] D43967: [ASTImporter] Add test helper Fixture

xazax.hun added a comment. I think having both kinds of tests might make sense. Overall, this looks good to me. Some nits inline. Comment at: unittests/AST/ASTImporterTest.cpp:143 +class Fixture : public ::testing::Test { + I do not like the name of this clas

[PATCH] D45517: [analyzer] False positive refutation with Z3

xazax.hun added inline comments. Comment at: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2382 +// Reset the solver +RefutationMgr.reset(); + } george.karpenkov wrote: > xazax.hun wrote: > > george.karpenkov wrote: > > > george.karpenkov wrote: > > >

[PATCH] D45517: [analyzer] False positive refutation with Z3

xazax.hun added a comment. In https://reviews.llvm.org/D45517#1116734, @george.karpenkov wrote: > @xazax.hun (I'll reply here to avoid scattering the conversation across many > subtrees) > > I was thinking about the optimization for not adding redundant constraints > some more, and I've decided

[PATCH] D45517: [analyzer] False positive refutation with Z3

xazax.hun added a comment. In https://reviews.llvm.org/D45517#1116770, @george.karpenkov wrote: > > I am not not sure that I got the idea what are you suggesting here. If we > > have the constraint of for example a symbol s > 10 and later on a path we > > discover s > 20, will we also deduplica

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

xazax.hun added a comment. Sorry for the limited activity. Unfortunately, I have very little time reviewing patches lately. I think we need to answer the following questions: - Does this change affect the analysis of the constructors of global objects? If so, how? - Do we want to import non-con

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

xazax.hun added inline comments. Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:223-225 + ExplodedNode *Node = Context.generateNonFatalErrorNode(Context.getState()); + if (!Node) +return; Szelethus wrote: > NoQ wrote: > > Szelethus w

[PATCH] D47671: [analyzer] Implement copy elision.

xazax.hun added a comment. So having an analyzer-config option would be useful for those codebases that are expected to be compiled with less advanced compilers that do not elide copies or not doing it aggressively enough. Maybe it is worth to ask on the mailing list of the community wants to h

[PATCH] D47671: [analyzer] Implement copy elision.

xazax.hun added a comment. In https://reviews.llvm.org/D47671#1122994, @george.karpenkov wrote: > > So having an analyzer-config option would be useful for those codebases > > that are expected to be compiled with less advanced compilers that do not > > elide copies or not doing it aggressively

[PATCH] D47671: [analyzer] Implement copy elision.

xazax.hun added a comment. Herald added a subscriber: mikhail.ramalho. Just for the record, there is a common example where relying on copy elision might bite and google do not recommend relying on it for correctness: https://abseil.io/tips/120 The main purpose of sharing is to add some more co

[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.

xazax.hun added a comment. Having C++ support would be awesome! Thanks for working on this! While I do agree matching is not trivial with qualified names, this problem is already solved with AST matchers. I wonder if using matchers or reusing the `HasNameMatcher` class would make this easier.

[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.

xazax.hun added inline comments. Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:32 check::PostCall> { - CallDescription CStrFn; + const llvm::SmallVector CStrFnFamily = { +{"std::basic_string::

[PATCH] D48285: [analyzer]{UninitializedObjectChecker] Added "NotesAsWarnings" flag

xazax.hun added a comment. I wonder if this could be done when plist is emitted generally, independent of any checks. Repository: rC Clang https://reviews.llvm.org/D48285 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.

[PATCH] D48521: [analyzer] Highlight STL object destruction in MallocChecker

xazax.hun added inline comments. Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:483 // Did not track -> released. Other state (allocated) -> released. - return (Stmt && (isa(Stmt) || isa(Stmt)) && - (S && S->isReleased()) && (!SPrev || !SPrev->

[PATCH] D48522: [analyzer] Highlight c_str() call in DanglingInternalBuffer checker

xazax.hun added a comment. Regarding the visitor: Maybe rather than looking at the AST, we should check the states, when we started to track the returned symbol? Using your current design you need to check for the AST twice. Once in the visitor and once in the check. Also, I wonder if this alw

[PATCH] D48522: [analyzer] Highlight c_str() call in DanglingInternalBuffer checker

xazax.hun added a comment. Looks better, thanks! Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:65 +if (Entry.second == Sym) + Found = true; + } Maybe early return here? https://reviews.llvm.org/D48522 _

[PATCH] D48521: [analyzer] Highlight container object destruction in MallocChecker

xazax.hun accepted this revision. xazax.hun added a comment. LGTM, given that the assert does not fire for the projects you tested on. https://reviews.llvm.org/D48521 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-

[PATCH] D48460: [analyzer] Fix invalidation on C++ const methods.

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LGTM! Repository: rC Clang https://reviews.llvm.org/D48460 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org

[PATCH] D48522: [analyzer] Highlight c_str() call in DanglingInternalBuffer checker

xazax.hun accepted this revision. xazax.hun added inline comments. Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:41 +// Tracked pointer to a buffer. +SymbolRef Sym; + I am fine with this as is, but I prefer self documenting cod

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

xazax.hun added a comment. I am not sure what to do about implcit checks. Those are probably should never be turned on or off by the user, but they should be on or off by default based on the set of checks the user enabled and the platform she is using. Thus, I am perfectly ok with the implicit

[PATCH] D52730: [analyzer] ConversionChecker: handle floating point

xazax.hun added inline comments. Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:164 +// double and possibly long double on some systems +RepresentsUntilExp = 53; break; + case 32: A link to the source of these number would be u

[PATCH] D48866: [clang-tidy] Add incorrect-pointer-cast checker

hgabii updated this revision to Diff 170360. hgabii added a comment. Warning messages changed. Tests updated. Comments changed based on the recommendations. Documentation refactored and reformatted. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48866 Files: clang-tidy/misc/C

[PATCH] D53483: [analyzer] Restrict AnalyzerOptions' interface so that non-checker objects have to be registered

xazax.hun added a comment. Overall looks good if the community agrees with the directions. Some comments inline. Comment at: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h:243 + /// specified. + StringRef getStringOption(StringRef Name, StringRef DefaultVal);

[PATCH] D53296: [analyzer] New flag to print all -analyzer-config options

xazax.hun added a comment. Overall looks good, minor comments inline. Comment at: lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp:166 + "\n\n"; + out << " clang [CLANG_OPTIONS] -analyzer-config OPTION1=VALUE, " + "-analyzer-config OPTION2=VALUE, ...\n

[PATCH] D53280: [analyzer] Emit a warning for unknown -analyzer-config options

xazax.hun added a comment. I agree with NoQ. Forward and backward compatibility might be important for CodeChecker as well. But I wonder if it make sense to have analyzer-config compatibility mode on a per config basis? E.g., if we have two configs: - One did not exist in earlier clang versions

[PATCH] D53277: [analyzer][NFC] Collect all -analyzer-config options in a .def file

xazax.hun added inline comments. Comment at: include/clang/StaticAnalyzer/Core/AnalyzerOptions.def:445 + +ANALYZER_OPTION_GEN_FN(StringRef, ModelPath, "model-path", "", "", getModelPath) + Should we explain the feature in the commad line path? The description

[PATCH] D53810: [analyzer][NFC] Refactor PlistDiagnostics to use a class instead of passing 9 parameters around

xazax.hun added inline comments. Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:82 -static void EmitRanges(raw_ostream &o, - const ArrayRef Ranges, - const FIDMap& FM, - const Preprocessor &PP, -

[PATCH] D52730: [analyzer] ConversionChecker: handle floating point

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LGTM! I only wonder if this should be on by default or guarded by a config option. I do not have strong feelings about any of the options though. Repository: rC Clang https://reviews

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

xazax.hun accepted this revision. xazax.hun added a comment. LGTM! Thanks, I think it is much easier to understand what is going on this way. https://reviews.llvm.org/D52742 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.o

[PATCH] D52794: [analyzer][PlistMacroExpansion] Part 2.: Retrieving the macro name and primitive expansion

xazax.hun added a comment. Please add a test case where a bug path goes through a macro definition and this macro is undefed at the end of the translation unit. Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:667 + +//===---

[PATCH] D56632: [analyzer] Track region liveness only through base regions.

xazax.hun added a comment. I really like all this detective work and it would be sad to have it forgotten. I would love to see some of your comments in the documentation of symbol reaper. More specifically: > When a memory region is live, all its sub-regions and super-regions are > automaticall

[PATCH] D56632: [analyzer] Track region liveness only through base regions.

xazax.hun accepted this revision. xazax.hun added a comment. Thanks, LGTM! It is interesting to see if we need to traverse all the super regions in `scanReachableSymbols`, but if we need to change something there, I would prefer that to be in a separate patch. If visiting the whole super region

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

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. Herald added subscribers: donat.nagy, Szelethus, rnkovacs, baloghadamsoftware. LGTM! Any objections to commit this? I think this is quiet coding guideline specific check which is useful

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

xazax.hun added a comment. In D35068#1361902 , @Szelethus wrote: > In D35068#1069880 , @koldaniel wrote: > > > I've evaluated this checker on LLVM+Clang, there were only a few (about 15) > > warnings, because of t

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

xazax.hun added a comment. To add an analogy, Clang Tidy will not require C++ Core Guidelines related checks to be evaluated on projects that are not following the guidelines as the results are meaningless for those projects. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D35068/new/ ht

[PATCH] D57230: [analyzer] Toning down invalidation a bit

xazax.hun created this revision. xazax.hun added reviewers: dergachev.a, george.karpenkov, Szelethus. xazax.hun added a project: clang. Herald added subscribers: gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, whisperity. This is a patch for the fo

[PATCH] D57230: [analyzer] Toning down invalidation a bit

xazax.hun marked 2 inline comments as done. xazax.hun added a comment. In D57230#1372275 , @NoQ wrote: > Could you share reproducible examples for these, probably in the form of > FIXME tests? Given that they are "regressions", they are easy to creduce do

[PATCH] D57230: [analyzer] Toning down invalidation a bit

xazax.hun updated this revision to Diff 183702. xazax.hun added a comment. - Added some tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57230/new/ https://reviews.llvm.org/D57230 Files: lib/StaticAnalyzer/Core/CallEvent.cpp test/Analysis/call-invalidation.cpp test/Analysis/cx

[PATCH] D57230: [analyzer] Toning down invalidation a bit

xazax.hun added a comment. In D57230#1372523 , @NoQ wrote: > In D57230#1372488 , @xazax.hun wrote: > > > In D57230#1372275 , @NoQ wrote: > > > > > > > > > > > Do you have suc

[PATCH] D57230: [analyzer] Toning down invalidation a bit

xazax.hun added a comment. I tried to creduce one file where the result differed and this is the result: typedef struct { int a; int b } c; d; e(c *f) { d < f->a; c g; h(&g.b); e(&g); } I think this the core idea is quite straightforward but this example is a bi

[PATCH] D57230: [analyzer] Toning down invalidation a bit

xazax.hun added a comment. Thanks for all the reviews. Do you have any preference about the spelling of the annotation mentioned in the description? There were two ideas so far: `uses_offsetof`, `may_use_offsetof` While I like those, I wonder if it is a good idea to have `offsetof` in the name

[PATCH] D53979: [analyzer][CTU] Correctly signal in the function index generation tool if there was an error

xazax.hun created this revision. xazax.hun added reviewers: NoQ, george.karpenkov, Szelethus, martong. Herald added subscribers: dkrupp, donat.nagy, arphaman, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, whisperity, mgorny. Do not always return 0 to the OS. This will make it

[PATCH] D53979: [analyzer][CTU] Correctly signal in the function index generation tool if there was an error

xazax.hun updated this revision to Diff 172134. xazax.hun added a reviewer: a.sidorin. xazax.hun added a comment. - Remove yet another dependency from the tool that is no longer used. https://reviews.llvm.org/D53979 Files: tools/clang-func-mapping/CMakeLists.txt tools/clang-func-mapping/Cla

[PATCH] D53692: [analyzer] Evaluate all non-checker config options before analysis

xazax.hun added inline comments. Comment at: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp:227 -CTUDir = getStringOption("ctu-dir", ""); -if (!llvm::sys::fs::is_directory(*CTUDir)) - CTUDir = ""; Szelethus wrote: > This check should and will be moved t

[PATCH] D52794: [analyzer][PlistMacroExpansion] Part 2.: Retrieving the macro name and primitive expansion

xazax.hun accepted this revision. xazax.hun added a comment. One question and one nit otherwise looks good. Feel free to commit once those are resolved without another round of reviews. Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:805 +// macro. +if (const

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

xazax.hun added inline comments. Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:685 const MacroInfo *MI = nullptr; + llvm::Optional Args; I wonder if the optional gives us any value here. An empty map could be just as great to represent that ther

[PATCH] D52986: [analyzer][PlistMacroExpansion] Part 4.: Support for __VA_ARGS__

xazax.hun accepted this revision. xazax.hun added a comment. One question otherwise looks good. Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:884 + // even if we lex a tok::comma and ParanthesesDepth == 1. + const IdentifierInfo *__VA_ARGS__II = PP.getIdentifierInf

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

xazax.hun added a comment. I also would like to see in a tool how this would look like as an event before committing :) Just a sanity check to make sure this feature makes sense. Could you post a screenshot of CodeChecker or any other tool using this feature? https://reviews.llvm.org/D52790

[PATCH] D53995: [analyzer] Drastically simplify the tblgen files used for checkers

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LGTM, but let's wait for @NoQ before committing. Repository: rC Clang https://reviews.llvm.org/D53995 ___ cfe-commits mailing list cfe-

[PATCH] D52984: [analyzer] Checker reviewer's checklist

xazax.hun updated this revision to Diff 172354. xazax.hun added a comment. This new version based on the bullets by NoQ. I also included some additional ones from other lists and added some new ones, e.g. the NamedDecl::getName will fail if the name of the decl is not a single token. I also reor

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

xazax.hun added a comment. I would love to see a test with deeper macro in macro expansion and larger number of arguments, with some of the arguments unused. Some minor nits inline, otherwise looks good. Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:831 const Ma

[PATCH] D52984: [analyzer] Checker reviewer's checklist

xazax.hun added inline comments. Comment at: www/analyzer/checker_dev_manual.html:719 + +User facing documentation is important for adoption! Make sure the check list updated +at the homepage of the analyzer. Also ensure that the description is good quality in -

[PATCH] D52984: [analyzer] Checker reviewer's checklist

xazax.hun updated this revision to Diff 173513. xazax.hun marked 11 inline comments as done. xazax.hun added a comment. - Move the checklist up before additional info in the HTML file. - Fix minor nits. - Add missing bullet points (thanks @Szelethus for noticing) I did not add any coding conventi

[PATCH] D52984: [analyzer] Checker reviewer's checklist

xazax.hun added inline comments. Comment at: www/analyzer/checker_dev_manual.html:719 + +User facing documentation is important for adoption! Make sure the check list updated +at the homepage of the analyzer. Also ensure that the description is good quality in -

[PATCH] D52984: [analyzer] Checker reviewer's checklist

xazax.hun updated this revision to Diff 173654. xazax.hun added a comment. - Use the term `checker` instead of `check`. https://reviews.llvm.org/D52984 Files: www/analyzer/checker_dev_manual.html Index: www/analyzer/checker_dev_manual.html

[PATCH] D54429: [analyzer] Creating standard shpinx documentation

xazax.hun added a comment. I do like the idea of moving the Clang Static Analyzer documentation to where the rest of the tools are documented. I believe the original reason the analyzer had a separate homepage is due to it was off by default in clang at the beginning and users downloaded it fro

[PATCH] D54556: [analyzer] MoveChecker Pt.1: Give MisusedMovedObject checker a more consistent name.

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. Looks good. Do we plan to detect problems other than use after move? Maybe it would be worth to synchronize with the tidy checker name use-after-move or is it going to cause more confusi

[PATCH] D54557: [analyzer] MoveChecker Pt.2: Restrict the warning to STL objects and locals.

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. It would be great to have a way to extend the list of (possibly non-stl) types to check. But I do understand that the analyzer does not have a great way to set such configuration options

[PATCH] D54557: [analyzer] MoveChecker Pt.2: Restrict the warning to STL objects and locals.

xazax.hun added a comment. In https://reviews.llvm.org/D54557#1300654, @NoQ wrote: > In https://reviews.llvm.org/D54557#1299736, @xazax.hun wrote: > > > It would be great to have a way to extend the list of (possibly non-stl) > > types to check. But I do understand that the analyzer does not hav

[PATCH] D48866: [clang-tidy] Add incorrect-pointer-cast checker

hgabii marked 9 inline comments as done. hgabii added a comment. I resolved all comments. Comment at: docs/ReleaseNotes.rst:60 +- New :doc:`misc-incorrect-pointer-cast + ` check Eugene.Zelenko wrote: > Will be good idea to rebase from trunk and use alphabeti

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

xazax.hun accepted this revision. xazax.hun added a reviewer: xazax.hun. xazax.hun added a comment. Some minor comment inline. Otherwise looks good. Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:879 +// If this token is the current macro's argument, we should exp

[PATCH] D53280: [analyzer] Emit an error for invalid -analyzer-config inputs

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. Overall looks good to me, some minor comments inline. Comment at: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h:231 #define ANALYZER_OPTION(TYPE, NAME, CMDFLAG,

[PATCH] D52984: [analyzer] Checker reviewer's checklist

xazax.hun updated this revision to Diff 176403. xazax.hun marked 4 inline comments as done. xazax.hun added a comment. - Addressed further comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52984/new/ https://reviews.llvm.org/D52984 Files: www/analyzer/checker_dev_manual.html I

[PATCH] D55129: [CTU] Eliminate race condition in CTU lit tests

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. After the review comment is resolved, the rest LGTM! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55129/new/ https://reviews.llvm.org/D55129

[PATCH] D55132: [CTU] Add asserts to protect invariants

xazax.hun accepted this revision. xazax.hun added inline comments. Comment at: lib/CrossTU/CrossTranslationUnit.cpp:249 CrossTranslationUnitContext::importDefinition(const FunctionDecl *FD) { + assert(FD->hasBody() && "Functions to be imported should have body."); + ---

[PATCH] D55133: [CTU] Add statistics

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. The code LGTM! I am not good at wordsmithing, but if the descriptions of the statistics are not clear enough, I agree that they should be rephrased. Repository: rC Clang CHANGES SINC

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

xazax.hun requested changes to this revision. xazax.hun added inline comments. This revision now requires changes to proceed. Comment at: include/clang/Basic/DiagnosticCrossTUKinds.td:19 + +def err_ctu_incompat_triple : Error< + "imported AST from '%0' had been generated for a d

[PATCH] D55135: [CTU][Analyzer]Add DisplayCTUProgress analyzer switch

xazax.hun added a comment. Having an analyzer config option makes sense. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55135/new/ https://reviews.llvm.org/D55135 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

xazax.hun added inline comments. Comment at: lib/CrossTU/CrossTranslationUnit.cpp:212 +// diagnostics. +Context.getDiagnostics().Report(diag::err_ctu_incompat_triple) +<< Unit->getMainFileName() << TripleTo.str() << TripleFrom.str(); martong wrote

[PATCH] D42682: [clang-tidy] Add io-functions-misused checker

hgabii updated this revision to Diff 176732. hgabii marked an inline comment as done. hgabii edited the summary of this revision. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D42682/new/ https://reviews.llvm.org/D42682 Files: clang-tidy/bugprone/Bu

[PATCH] D42682: [clang-tidy] Add io-functions-misused checker

hgabii marked an inline comment as done. hgabii added a comment. I moved to bugprone module and renamed to bugprone-io-functions. I added as a reference for cert-fio34-c. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D42682/new/ https://reviews.llvm.

[PATCH] D42682: [clang-tidy] Add io-functions-misused checker

hgabii updated this revision to Diff 176736. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D42682/new/ https://reviews.llvm.org/D42682 Files: clang-tidy/bugprone/BugproneTidyModule.cpp clang-tidy/bugprone/CMakeLists.txt clang-tidy/bugprone/IoFun

[PATCH] D55280: [CTU] Remove redundant error check

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LG! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55280/new/ https://reviews.llvm.org/D55280 ___ cfe-commi

[PATCH] D55388: [analyzer] MoveChecker Pt.8: Add checks for dereferencing a smart pointer after move.

xazax.hun added a comment. Hm. I wonder if it would also make sense to model e.g. the get method to return nullptr for moved from smart ptrs. This could help null dereference checker and also aid false path prunning. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D55135: [CTU][Analyzer]Add DisplayCTUProgress analyzer switch

xazax.hun accepted this revision. xazax.hun added a comment. While Static Analyzer is the only client of CTU library at the moment, we might have more in the future. I would not use the phrase `ANALYZE` in the log message. Once this is resolved the rest looks good. Repository: rC Clang CHA

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LG! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55134/new/ https://reviews.llvm.org/D55134 ___ cfe-commi

[PATCH] D55131: [CTU] Add more lit tests and better error handling

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LG! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55131/new/ https://reviews.llvm.org/D55131 ___ cfe-commi

[PATCH] D42682: [clang-tidy] Add io-functions-misused checker

hgabii updated this revision to Diff 177713. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D42682/new/ https://reviews.llvm.org/D42682 Files: clang-tidy/bugprone/BugproneTidyModule.cpp clang-tidy/bugprone/CMakeLists.txt clang-tidy/bugprone/IoFun

[PATCH] D42682: [clang-tidy] Add io-functions-misused checker

hgabii marked 3 inline comments as done. hgabii added inline comments. Comment at: clang-tidy/bugprone/IoFunctionsCheck.cpp:32 +has(cxxMemberCallExpr( +on(hasType(namedDecl(hasAnyName("istream", +callee(cxxMethodDecl(has

[PATCH] D57906: [CTU] Do not allow different CPP dialects in CTU

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LGTM! I think we should commit this as is for now but maybe adding a TODO comment to summarize the problem would be nice. Maybe we could have an isSameDialect or similar method within La

<    1   2   3   4   5   6   7   8   9   10   >