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

2018-11-22 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. Herald added a subscriber: baloghadamsoftware. I'll commit tomorrow, when I have time to keep an eye out for buildbots. Thanks! https://reviews.llvm.org/D33672 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:/

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

2018-11-24 Thread Umann Kristóf via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC347513: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker (authored by Szelethus, committed by ). Changed prior to commit: https://reviews.llvm.org/D33672?vs=172516&id=175154#toc

[PATCH] D54823: [analyzer][MallocChecker][NFC] Document and reorganize some functions

2018-11-24 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus marked 3 inline comments as done. Szelethus added inline comments. Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1087 if (FD->getKind() == Decl::Function) { -initIdentifierInfo(C.getASTContext()); +MemFunctionInfo.initIdentifierInfo(C.getASTContex

[PATCH] D54438: [analyzer][WIP] Reimplement dependencies between checkers

2018-11-24 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. Hmmm, I was wrong a little bit: I realized that if I tinker with with `initializeManager` and `getEnabledCheckers` just a bit more, register checkers straight away, don't need to collect them first, and this would make sure that dependencies are registered before the

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

2018-11-25 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. I think the reason why the printed message was either along the lines of "this is 0" and "this is non-0", is that we don't necessarily know what constraint solver we're using, and adding more non-general code `BugReporter` is most probably a bad approach. How about we

[PATCH] D54437: [analyzer][NFC] Merge ClangCheckerRegistry to CheckerRegistry

2018-11-26 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. Polite ping :) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54437/new/ https://reviews.llvm.org/D54437 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin

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

2018-11-26 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. Polite ping. :) I'd be most comfortable landing D53692 together with this patch. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53280/new/ https://reviews.llvm.org/D53280

[PATCH] D51866: [analyzer][UninitializedObjectChecker] New flag to ignore guarded uninitialized fields

2018-11-26 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. > @george.karpenkov Matching macros is a very non-trivial job, how would you > feel if we shipped this patch as-is, and maybe leave a TODO about adding > macro `assert`s down the line? The only solution I saw in clang-tidy was to match binary expressions as a heurist

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

2018-11-27 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. Thanks! I'll commit around friday (with the requested fixes) to leave enough time for everyone to object. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53280/new/ https://reviews.llvm.org/D53280 _

[PATCH] D54401: [analyzer] Prefer returns values to out-params in CheckerRegistry.cpp

2018-11-27 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. Ping^2 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54401/new/ https://reviews.llvm.org/D54401 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54436: [analyzer][NFC] Move CheckerRegistry from the Core directory to Frontend

2018-11-27 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. Ping^2 Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54436/new/ https://reviews.llvm.org/D54436 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman

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

2018-11-28 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. In D54757#1311468 , @donat.nagy wrote: > **Macros:** > > The current implementation of the check only looks at the preprocessed code, > therefore it detects the situations where the duplication is created by > macros. I added s

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

2018-11-28 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus updated this revision to Diff 175718. Szelethus added a comment. Herald added a subscriber: gamesh411. - Fixed a crash where no arguments (**not** empty arguments) were supplied to `__VA_ARGS__`. - Rebased to master. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52986/new/ ht

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

2018-11-28 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus marked 2 inline comments as done. Szelethus added inline comments. Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:884 + // even if we lex a tok::comma and ParanthesesDepth == 1. + const IdentifierInfo *__VA_ARGS__II = PP.getIdentifierInfo("__VA_ARGS__"); + -

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

2018-11-28 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus marked 2 inline comments as done. Szelethus added inline comments. Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:884 + // even if we lex a tok::comma and ParanthesesDepth == 1. + const IdentifierInfo *__VA_ARGS__II = PP.getIdentifierInfo("__VA_ARGS__"); + -

[PATCH] D51866: [analyzer][UninitializedObjectChecker] New flag to ignore guarded uninitialized fields

2018-11-28 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus requested review of this revision. Szelethus added a comment. This revision is now accepted and ready to land. Changes are no longer planned for the reasons explained in my earlier comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D51866/new/ https://reviews.llvm.org/D518

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

2018-11-28 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus marked 2 inline comments as done. Szelethus added inline comments. Comment at: lib/Driver/ToolChains/Clang.cpp:2348 + // Enable compatilibily mode to avoid analyzer-config related errors. + CmdArgs.push_back("-analyzer-config-compatibility-mode=true"); + -

[PATCH] D55051: [Analyzer] [HOTFIX!] SValBuilder crash when `aggressive-binary-operation-simplification` enabled

2018-11-29 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. Have you seen a crash resulting from this? Is supplying a test case feasable? I know that some of these errors are extremely hard to reproduce. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55051/new/ https://reviews.llvm.org/D55051

[PATCH] D55051: [Analyzer] [HOTFIX!] SValBuilder crash when `aggressive-binary-operation-simplification` enabled

2018-11-29 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. In D55051#1312666 , @baloghadamsoftware wrote: > In D55051#1312660 , @Szelethus wrote: > > > Have you seen a crash resulting from this? Is supplying a test case > > feasable? I know that

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

2018-11-29 Thread Umann Kristóf via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC347888: [analyzer][PlistMacroExpansion] Part 4.: Support for __VA_ARGS__ (authored by Szelethus, committed by ). Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52986/new/ h

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

2018-11-29 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added a comment. In D54557#1313382 , @NoQ wrote: > In D54557#1301896 , @Szelethus wrote: > > > Wasn't the point of this patch to turn off part of this checkers > > functi

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

2018-11-29 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus marked 2 inline comments as done. Szelethus added a comment. Thanks! Comment at: lib/Frontend/CompilerInvocation.cpp:363 + A->claim(); Opts.Config[key] = val; } NoQ wrote: > Maybe we can eventually turn this into an array and address

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

2018-11-29 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. In D53692#1313956 , @NoQ wrote: > In D53692#1293781 , @NoQ wrote: > > > In D53692#1293778 , @Szelethus > > wrote: > > > > > Did you know that clan

[PATCH] D55125: [clang-tidy] Fix a false positive in misc-redundant-expression check

2018-11-30 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. @JonasToth this is the `Lexer` based expression equality check I talked about in D54757#1311516 . The point of this patch is that the definition is a macro sure could be build dependent, but the macro name is not, so it wouldn

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

2018-11-30 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: test/Analysis/ctu-main.c:3-5 +// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-pch -o %t/ctudir2/ctu-other.c.ast %S/Inputs/ctu-other.c +// RUN: cp %S/Inputs/externalFnMap2.txt %t/ctudir2/externalFnMap.txt +// RUN: %clang_cc1 -trip

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

2018-11-30 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. Also, maybe it'd be worth making a CTU directory under `test/Analysis` for CTU related test files. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55131/new/ https://reviews.llvm.org/D55131

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

2018-11-30 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus requested changes to this revision. Szelethus added a comment. This revision now requires changes to proceed. The idea is great! I think this should rather be an -analyzer-config flag, since the actual analysis changes with a new output (refer to `AnalyzerOption`'s doxygen comments).

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

2018-11-30 Thread Umann Kristóf via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL348025: [analyzer][PlistMacroExpansion] Part 5.: Support for # and ## (authored by Szelethus, committed by ). Herald added subscribers: llvm-commits, gamesh411. Changed prior to commit: https://reviews.

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

2018-11-30 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. Hah. Do we support CTU for other languages, like ObjC and ObjC++? Can this be an issue there? Comment at: lib/CrossTU/CrossTranslationUnit.cpp:59 +} +} + `// end of namespace llvm` Comment at: lib/CrossTU/CrossTran

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

2018-11-30 Thread Umann Kristóf via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC348031: [analyzer] Evaluate all non-checker config options before analysis (authored by Szelethus, committed by ). Changed prior to commit: https://reviews.llvm.org/D53692?vs=172856&id=176183#toc Repos

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

2018-11-30 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus marked 6 inline comments as done. Szelethus added inline comments. Comment at: lib/Frontend/CompilerInvocation.cpp:367 - parseAnalyzerConfigs(Opts, Diags); + if (Opts.ShouldEmitErrorsOnInvalidConfigValue) +parseAnalyzerConfigs(Opts, &Diags); xaz

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

2018-11-30 Thread Umann Kristóf via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Szelethus marked an inline comment as done. Closed by commit rC348038: [analyzer] Emit an error for invalid -analyzer-config inputs (authored by Szelethus, committed by ). Changed prior to commit: https://reviews.llvm.org

[PATCH] D55125: [clang-tidy] Fix a false positive in misc-redundant-expression check

2018-11-30 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a subscriber: donat.nagy. Szelethus added a comment. I see your point, but here's why I think it isn't a bug: I like to see macros as `constexpr` variables, and if I used those instead, I personally wouldn't like to get a warning just because they have the same value. In C, silen

[PATCH] D54401: [analyzer] Prefer returns values to out-params in CheckerRegistry.cpp

2018-12-01 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a reviewer: alexfh. Szelethus added a comment. *advanced summoning* CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54401/new/ https://reviews.llvm.org/D54401 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://list

[PATCH] D55125: [clang-tidy] Fix a false positive in misc-redundant-expression check

2018-12-02 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. Yup, I agree with everything that was said. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55125/new/ https://reviews.llvm.org/D55125 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cg

[PATCH] D54438: [analyzer][WIP] Reimplement dependencies between checkers

2018-12-02 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. Okay. I was wrong, and you were right. `MallocChecker` seriously suffers from overcrowding. Especially with the addition of `InnerPointerChecker`, it should be split up, but doing that is very much avoidable for the purposes of this effort while still achieving every

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

2018-12-03 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. Also, `AnalyzerOptions.def` was recently clan-formatted, feel free to run it again after the changes you make in it. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55135/new/ https://reviews.llvm.org/D55135 __

[PATCH] D51886: [analyzer][UninitializedObjectChecker] Using the new const methods of ImmutableList

2018-09-17 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. Ping :) Repository: rC Clang https://reviews.llvm.org/D51886 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51866: [analyzer][UninitializedObjectChecker][WIP] New flag to ignore guarded uninitialized fields

2018-09-17 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. @george.karpenkov @xazax.hun, is this how you imagined checking whether an access is guarded? Repository: rC Clang https://reviews.llvm.org/D51866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.ll

[PATCH] D51866: [analyzer][UninitializedObjectChecker][WIP] New flag to ignore guarded uninitialized fields

2018-09-17 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. I played around with it, and this seemed a bit cleaner. Since I would only match against the definition of the record (and the definition of its methods), the matcher would need to look like this: `stmt(hasDescendant(blahBlah(...)))`, and I'd only be able to retrieve

[PATCH] D52219: [analyzer] (1/n) Support pointee mutation analysis in ExprMutationAnalyzer.

2018-09-19 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: lib/Analysis/ExprMutationAnalyzer.cpp:86 +// - Pointer to non-const +// - Pointer-like type with `operator*` returning non-const reference +bool isPointeeMutable(const Expr *Exp, const ASTContext &Context) { Didn't you

[PATCH] D52390: [analyzer] StackSizeChecker

2018-09-22 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. Hi! Always great to see a new checker! I've started working in this project little over half a year ago, so I don't claim to be an expert, read my remarks as such! It'll be some time before I go through the entire code, but so far here are the things that caught my e

[PATCH] D52390: [analyzer] StackSizeChecker

2018-09-22 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added subscribers: rnkovacs, baloghadamsoftware. Szelethus added a comment. I think the idea for a checker like this is great! I left some inline comments, but most of them are minor nits. I have some general remarks to make however: - Your code lacks comments, especially a nice docume

[PATCH] D52390: [analyzer] StackSizeChecker

2018-09-22 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: lib/StaticAnalyzer/Checkers/StackSizeChecker.cpp:104 + ProgramStateRef State = C.getState(); + auto StackLevels = State->get(); + if (length(StackLevels) != countPredecessors(C)) Szelethus wrote: > It isn't obvious

[PATCH] D52390: [analyzer] StackSizeChecker

2018-09-22 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: lib/StaticAnalyzer/Checkers/StackSizeChecker.cpp:19 +#include "clang/AST/StmtCXX.h" +#include "clang/StaticAnalyzer/Checkers/StackUsageMeasuringVisitor.h" +#include "clang/StaticAnalyzer/Core/AnalyzerOptions.h" Szeleth

[PATCH] D51886: [analyzer][UninitializedObjectChecker] Using the new const methods of ImmutableList

2018-09-23 Thread Umann Kristóf via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC342834: [analyzer][UninitializedObjectChecker] Using the new const methods of… (authored by Szelethus, committed by ). Repository: rC Clang https://reviews.llvm.org/D51886 Files: lib/StaticAnalyzer/

[PATCH] D52423: [analyzer] Make ConversionChecker load StdCLibraryFunctionsChecker

2018-09-24 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus edited reviewers, added: NoQ; removed: dergachev.a. Szelethus added a comment. Cool! Comment at: test/Analysis/conversion.c:141 -// false positives.. +// old false positives.. I think this comment is no longer relevant ^-^ Repository: rC Clang

[PATCH] D52423: [analyzer] Make ConversionChecker load StdCLibraryFunctionsChecker

2018-09-24 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: test/Analysis/conversion.c:158 extern int dostuff (void); int falsePositive2() { int c, n; And this one Repository: rC Clang https://reviews.llvm.org/D52423 ___

[PATCH] D52390: [analyzer] StackSizeChecker

2018-09-24 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. > Thanks for all your detailed and helpful input, I will make sure to go over > all the comments and answer them, but it will take some time. Cheers! I can't emphasize enough however that I might be wrong on what I've said, or say in this comment. > It was my introdu

[PATCH] D52390: [analyzer] StackSizeChecker

2018-09-24 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. One important thing I forgot, that you cant rely on `ProgramState` due to tidy's constraints. Btw, how do you plan to make this into a tidy checker? To me it seems like it would amplify the already existing false positive issues (if I understand your currect way of th

[PATCH] D52502: [Lex] TokenConcatenation now takes const Preprocessor

2018-09-25 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus created this revision. Szelethus added a reviewer: dblaikie. Herald added a subscriber: cfe-commits. Repository: rC Clang https://reviews.llvm.org/D52502 Files: include/clang/Lex/TokenConcatenation.h lib/Lex/TokenConcatenation.cpp Index: lib/Lex/TokenConcatenation.cpp =

[PATCH] D52502: [Lex] TokenConcatenation now takes const Preprocessor

2018-09-25 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. Thanks! > any particular motivation for this? I'm currently trying to include macro expansions in the Static Analyzer's plist output, and I only managed to make this happen by some `Token` handling hackery, including printing them. I saw this trick in `lib/Rewrite/H

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

2018-09-26 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: include/clang/StaticAnalyzer/Checkers/StackUsageMeasuringVisitor.h:102-111 + bool hasEmptyFlaggedUsageStored(const Stmt *S) const { +auto iter = StmtSubtreeUsages.find(S); +return iter != StmtSubtreeUsages.end() && +

[PATCH] D52502: [Lex] TokenConcatenation now takes const Preprocessor

2018-09-27 Thread Umann Kristóf via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL343204: [Lex] TokenConcatenation now takes const Preprocessor (authored by Szelethus, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D52502?vs

[PATCH] D52502: [Lex] TokenConcatenation now takes const Preprocessor

2018-09-27 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h:82-84 + // TODO: This destructor shouldn't be virtual, but breaks buildbots with + // -Werror -Wnon-virtual-dtor. + virtual ~FieldNode() = default; ---

[PATCH] D51531: [analyzer][UninitializedObjectChecker] Uninit regions are only reported once

2018-10-01 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. Ping ^-^ https://reviews.llvm.org/D51531 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2018-10-01 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:175 + + if (RepresentsUntilExp >= sizeof(unsigned long long)*8) { return false; How about `AC.getSizeType(AC.UnsignedLongLongTy))`? Comment at:

[PATCH] D52735: [analyzer][NFC] Refactor functions in PlistDiDiagnostics to take Preproc as parameter

2018-10-01 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus created this revision. Szelethus added reviewers: NoQ, george.karpenkov, dcoughlin. Herald added subscribers: cfe-commits, mikhail.ramalho, a.sidorin, szepet, xazax.hun, whisperity. This is patch is a preparation for the proposed inclusion of macro expansions in the plist output. Also

[PATCH] D52735: [analyzer][NFC] Refactor functions in PlistDiagnostics to take Preproc as parameter

2018-10-01 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus updated this revision to Diff 167776. Szelethus retitled this revision from "[analyzer][NFC] Refactor functions in PlistDiDiagnostics to take Preproc as parameter" to "[analyzer][NFC] Refactor functions in PlistDiagnostics to take Preproc as parameter". Szelethus added a comment. Oops.

[PATCH] D52735: [analyzer][NFC] Refactor functions in PlistDiagnostics to take Preproc as parameter

2018-10-01 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. Would you be okay with me clang-formatting this file? I'm aware of a few people doing some work with this file, but the diff didn't seem too bad to cause any serious rebasing nightmares. https://reviews.llvm.org/D52735 _

[PATCH] D52735: [analyzer][NFC] Refactor functions in PlistDiagnostics to take Preproc as parameter

2018-10-01 Thread Umann Kristóf via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL343511: [analyzer][NFC] Refactor functions in PlistDiagnostics to take Preproc as… (authored by Szelethus, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://revie

[PATCH] D52735: [analyzer][NFC] Refactor functions in PlistDiagnostics to take Preproc as parameter

2018-10-01 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. Ah, okay, I'm fine with that. https://reviews.llvm.org/D52735 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2018-10-01 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus created this revision. Szelethus added reviewers: NoQ, george.karpenkov, xazax.hun, rnkovacs, dcoughlin, MTC, dkrupp. Herald added subscribers: cfe-commits, mikhail.ramalho, a.sidorin, szepet, whisperity. This is the implementation of the inclusion of macro expansions into the plist o

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

2018-10-01 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a reviewer: whisperity. Szelethus added a comment. We need your all-seeing hawk eye! Repository: rC Clang https://reviews.llvm.org/D52742 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailma

[PATCH] D52746: [clang-query] Add single-letter 'q' alias for 'quit'

2018-10-01 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. I'd personally really enjoy this feature. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52746 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

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

2018-10-01 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a subscriber: whisperity. Szelethus added inline comments. Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:175 + + if (RepresentsUntilExp >= sizeof(unsigned long long)*8) { return false; Szelethus wrote: > How about `AC.getSize

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

2018-10-02 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:175 + + if (RepresentsUntilExp >= sizeof(unsigned long long)*8) { return false; NoQ wrote: > Szelethus wrote: > > Szelethus wrote: > > > How about `AC.getSizeType

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

2018-10-02 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus updated this revision to Diff 167924. Szelethus added a comment. - Fixes according to @whisperity's comments - New test cases for - commas in brackets, - commas in braces, - macro arguments left empty. > (The whole thing, however, is generally disgusting. I'd've expected the > Pr

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

2018-10-02 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus marked 13 inline comments as done. Szelethus added inline comments. Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:292 + + // Output the location. + FullSourceLoc L = P.getLocation().asLocation(); whisperity wrote: > the location of what? Th

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

2018-10-02 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:755 +if (It->is(tok::l_paren)) { + while ((++It)->isNot(tok::r_paren)) { +assert(It->isNot(tok::eof) && donat.nagy wrote: > I fear that this doe

[PATCH] D52787: [analyzer][NFC] Refactor functions in PlistDiagnostics to take AnalyzerOptions as parameter

2018-10-02 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus created this revision. Szelethus added reviewers: george.karpenkov, NoQ. Herald added subscribers: cfe-commits, mikhail.ramalho, a.sidorin, szepet, xazax.hun, whisperity. I intend to add a new flag `macro-expnasions-as-events`, and unfortunately I'll only be able to convert the macro p

[PATCH] D52787: [analyzer][NFC] Refactor functions in PlistDiagnostics to take AnalyzerOptions as parameter

2018-10-02 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:306-307 + const FIDMap& FM, + const Preprocessor &PP, + AnalyzerOptions &AnOpts) { + ReportPiece(o, P, FM, PP, AnOpts, /*in

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

2018-10-02 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus updated this revision to Diff 167992. Szelethus retitled this revision from "[analyzer][WIP] Add macro expansions to the plist output" to "[analyzer][PlistMacroExpansion] Part 1.: New expand-macros flag". Szelethus edited the summary of this revision. Szelethus removed reviewers: xazax.

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

2018-10-02 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus created this revision. Szelethus added reviewers: george.karpenkov, NoQ, rnkovacs, dkrupp, whisperity, martong, baloghadamsoftware. Herald added subscribers: cfe-commits, mikhail.ramalho, a.sidorin, szepet, xazax.hun. Szelethus added a dependency: D52742: [analyzer][PlistMacroExpansion]

[PATCH] D52787: [analyzer][NFC] Refactor functions in PlistDiagnostics to take AnalyzerOptions as parameter

2018-10-02 Thread Umann Kristóf via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC343620: [analyzer][NFC] Refactor functions in PlistDiagnostics to take AnalyzerOptions… (authored by Szelethus, committed by ). Repository: rC Clang https://reviews.llvm.org/D52787 Files: lib/Stati

[PATCH] D52787: [analyzer][NFC] Refactor functions in PlistDiagnostics to take AnalyzerOptions as parameter

2018-10-02 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. I'll probably do something with this parameter spaghetti, it bothers me too, but maybe in a separate patch :) Repository: rC Clang https://reviews.llvm.org/D52787 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

2018-10-03 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus marked 3 inline comments as done. Szelethus 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 85a6dda64587a5a18482f091cbcf

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

2018-10-04 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus updated this revision to Diff 168315. https://reviews.llvm.org/D52790 Files: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h lib/StaticAnalyzer/Core/AnalyzerOptions.cpp lib/StaticAnalyzer/Core/PlistDiagnostics.cpp test/Analysis/Inputs/expected-plists/plist-macros-with-expan

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

2018-10-04 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus updated this revision to Diff 168317. Szelethus added a comment. Changed the plist output. https://reviews.llvm.org/D52790 Files: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h lib/StaticAnalyzer/Core/AnalyzerOptions.cpp lib/StaticAnalyzer/Core/BugReporter.cpp lib/StaticA

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

2018-10-04 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus updated this revision to Diff 168319. https://reviews.llvm.org/D52790 Files: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h lib/StaticAnalyzer/Core/AnalyzerOptions.cpp lib/StaticAnalyzer/Core/PlistDiagnostics.cpp test/Analysis/Inputs/expected-plists/plist-macros-with-expan

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

2018-10-04 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus updated this revision to Diff 168320. Szelethus marked an inline comment as done. https://reviews.llvm.org/D52742 Files: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h lib/StaticAnalyzer/Core/AnalyzerOptions.cpp lib/StaticAnalyzer/Core/BugReporter.cpp lib/StaticAnalyzer/Cor

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

2018-10-04 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus updated this revision to Diff 168334. https://reviews.llvm.org/D52794 Files: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist test/Analysis/plist-macros-with-expansion.cpp Index: test/Analysis/plist-macros-wit

[PATCH] D52957: [analyzer] Teach CallEvent about C++17 aligned new.

2018-10-06 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:929 + // number of arguments is always the same as the number of parameters. + unsigned getNumImplicitArgs() const { +return getOriginExpr()->passAlignment() ? 2 : 1; --

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

2018-10-07 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. It'd also be good to have an entry in `www/analyzer/alpha_checks.html`. We've been neglecting it for long enough :/. Repository: rC Clang https://reviews.llvm.org/D52390 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D52969: [analyzer][www] Added the missing alpha.cplusplus checkers to the webpage

2018-10-07 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus created this revision. Szelethus added reviewers: NoQ, george.karpenkov, baloghadamsoftware, rnkovacs. Herald added subscribers: cfe-commits, mikhail.ramalho, a.sidorin, szepet, xazax.hun, whisperity. Repository: rC Clang https://reviews.llvm.org/D52969 Files: www/analyzer/alpha_c

[PATCH] D51866: [analyzer][UninitializedObjectChecker][WIP] New flag to ignore guarded uninitialized fields

2018-10-07 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus updated this revision to Diff 168592. Szelethus added a comment. Reimplemented with AST matchers. @george.karpenkov I know you wanted to test this feature for a while, but sadly I've been busy with the macro expansion related projects, I hope it's still relevant. https://reviews.llvm

[PATCH] D51531: [analyzer][UninitializedObjectChecker] Uninit regions are only reported once

2018-10-07 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. Another possibility could be to gather `CXXConstructorDecl`, and emit one warning per ctor, but it would be waaay to drastic. Wouldn't a global set be too? https://reviews.llvm.org/D51531 ___ cfe-commits mailing list cfe

[PATCH] D52969: [analyzer][www] Added the missing alpha.cplusplus checkers to the webpage

2018-10-07 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus updated this revision to Diff 168595. Szelethus edited the summary of this revision. https://reviews.llvm.org/D52969 Files: www/analyzer/alpha_checks.html www/analyzer/available_checks.html Index: www/analyzer/available_checks.html ==

[PATCH] D52969: [analyzer][www] Update alpha_checks.html

2018-10-07 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus updated this revision to Diff 168597. Szelethus retitled this revision from "[analyzer][www] Added the missing alpha.cplusplus checkers to the webpage" to "[analyzer][www] Update alpha_checks.html". Herald added a subscriber: jfb. https://reviews.llvm.org/D52969 Files: www/analyzer/

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

2018-10-08 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. I love the idea. I got a couple more: Is `-analyzer-checker=core` included in all `RUN:` lines? Is the description in `Checkers.td` clear to a regular user? Is `Checkers.td` and the CMakeLists file alphabetically sorted? Repository: rC Clang https://reviews.llvm.or

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

2018-10-08 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus created this revision. Szelethus added reviewers: george.karpenkov, xazax.hun, NoQ, rnkovacs, baloghadamsoftware. Herald added subscribers: cfe-commits, mikhail.ramalho, a.sidorin, szepet, whisperity. Thanks to @donat.nagy for spotting this! Repository: rC Clang https://reviews.ll

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

2018-10-08 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. LGTM! I can think of a couple more, like - making sure that `clang-format` was run on the new files, - not forgetting header guards, - making sure that variables and functions only intended for use within an `assert` call are handled in a way that release build bots wo

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

2018-10-08 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus created this revision. Szelethus added reviewers: xazax.hun, NoQ, george.karpenkov, rnkovacs, baloghadamsoftware. Herald added subscribers: cfe-commits, mikhail.ramalho, a.sidorin, szepet, whisperity. >From what I can see, this should be the last patch needed to replicate macro >argum

[PATCH] D51866: [analyzer][UninitializedObjectChecker] New flag to ignore guarded uninitialized fields

2018-10-08 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp:519 + +if (FirstAccess->getBeginLoc() < FirstGuard->getBeginLoc()) + return true; xazax.hun wrote: > I am not sure if this is a r

[PATCH] D52993: [analyzer][www] Add more useful links

2018-10-08 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus created this revision. Szelethus added reviewers: NoQ, george.karpenkov, xazax.hun, dcoughlin. Herald added subscribers: cfe-commits, mikhail.ramalho, a.sidorin, rnkovacs, szepet, whisperity. This bothered me for a while, but finding literature about static analysis is very difficult.

[PATCH] D52969: [analyzer][www] Update alpha_checks.html

2018-10-09 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: www/analyzer/alpha_checks.html:450 - -Variable Argument Alpha Checkers - NoQ wrote: > I think these are missing on the available checkers list, so we should move > rather than delete. Indeed, I intend to update `avaib

[PATCH] D52993: [analyzer][www] Add more useful links

2018-10-09 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus updated this revision to Diff 168767. Szelethus added a comment. Merged the two sections. https://reviews.llvm.org/D52993 Files: www/analyzer/checker_dev_manual.html Index: www/analyzer/checker_dev_manual.html === ---

[PATCH] D52993: [analyzer][www] Add more useful links

2018-10-09 Thread Umann Kristóf via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC344031: [analyzer][www] Add more useful links (authored by Szelethus, committed by ). Repository: rC Clang https://reviews.llvm.org/D52993 Files: www/analyzer/checker_dev_manual.html Index: www/an

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

2018-04-12 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. Thank you for all your comments so far! I'll probably only be able to update the diff tomorrow (with me being in the GMT + 1 timezone). > That's a lot of code, so it'll take me some time to understand what's going > on here. You might be able to help me by the large p

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

2018-04-12 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: test/Analysis/ctor-uninitialized-member-inheritance.cpp:24 + : NonPolymorphicLeft1(int{}) { +y = 420; +z = 420; Szelethus wrote: > whisperity wrote: > > The literal `420` is repeated //everywhere// in this

<    1   2   3   4   5   6   >