[PATCH] D34512: [libTooling] Add preliminary Cross Translation Unit support for libTooling

xazax.hun updated this revision to Diff 104848. xazax.hun added a comment. - Updated to compile with trunk - Minor style fixes - Proper diagnostic when the index format is wrong https://reviews.llvm.org/D34512 Files: include/clang/Basic/DiagnosticFrontendKinds.td

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

xazax.hun updated this revision to Diff 105053. xazax.hun added a comment. - Patch scan-build instead of using custom scripts - Rebase patch based on the proposed LibTooling CTU code https://reviews.llvm.org/D30691 Files: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h

[PATCH] D34512: [libTooling] Add preliminary Cross Translation Unit support for libTooling

xazax.hun added a comment. It looks like Richard approved libTooling as a dependency for clang on the mailing list (http://lists.llvm.org/pipermail/cfe-dev/2017-July/054536.html). If it is ok to have this code in libTooling (for now), I think we could start/continue the review of this patch.

[PATCH] D34512: [libTooling] Add preliminary Cross Translation Unit support for libTooling

xazax.hun added a comment. In https://reviews.llvm.org/D34512#800499, @klimek wrote: > In https://reviews.llvm.org/D34512#800490, @xazax.hun wrote: > > > It looks like Richard approved libTooling as a dependency for clang on the > > mailing list > >

[PATCH] D34512: Add preliminary Cross Translation Unit support library

xazax.hun added a comment. In https://reviews.llvm.org/D34512#800618, @klimek wrote: > +Richard as top-level code owner for new libs. > > For bikeshedding the name: I'd have liked libIndex, but that's already taken. > CrossTU doesn't seem too bad to me, too, though. Some brainstorming for the

[PATCH] D34512: Add preliminary Cross Translation Unit support library

xazax.hun updated this revision to Diff 105412. xazax.hun marked an inline comment as done. xazax.hun added a comment. - Fix a copy and paste error and removed an unintended change. https://reviews.llvm.org/D34512 Files: include/clang/Basic/DiagnosticFrontendKinds.td

[PATCH] D34512: Add preliminary Cross Translation Unit support library

xazax.hun updated this revision to Diff 105409. xazax.hun retitled this revision from "[libTooling] Add preliminary Cross Translation Unit support for libTooling" to "Add preliminary Cross Translation Unit support library". xazax.hun added a comment. - Move CrossTU functionality into its own

[PATCH] D34506: Factor out a functionality from `isBeforeInTranslationUnit`

xazax.hun added a comment. I created a new public API that is using a piece of code that was factored out from `isBeforeInTranslationUnit`. Using this new function it is possible to implement proper comparison of source locations within the Static Analyzer. What do you think?

[PATCH] D34506: Factor out a functionality from `isBeforeInTranslationUnit`

xazax.hun updated this revision to Diff 104167. xazax.hun retitled this revision from "Relax an assert in the comparison of source locations" to "Factor out a functionality from `isBeforeInTranslationUnit`". xazax.hun edited the summary of this revision. xazax.hun added a comment. - New

[PATCH] D34449: [clang-tidy] Enable inline variable definitions in headers.

xazax.hun added inline comments. Comment at: test/clang-tidy/misc-definitions-in-headers.hpp:1 -// RUN: %check_clang_tidy %s misc-definitions-in-headers %t +// RUN: %check_clang_tidy %s misc-definitions-in-headers %t -- -- -std=c++1z hokein wrote: > hokein

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

xazax.hun added inline comments. Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:31 +class VirtualCallChecker: public Checker { + mutable std::unique_ptr BT_CT; + mutable std::unique_ptr BT_DT; Could you find

[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width

xazax.hun added a comment. I only found two nits otherwise looks good to me. Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h:46 +bool exprComparesTo(SVal LHSVal, BinaryOperatorKind ComparisonOp, SVal RHSVal, +ProgramStateRef

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

xazax.hun updated this revision to Diff 96727. xazax.hun marked 5 inline comments as done. xazax.hun added a comment. - Updates according to review comments - Improvements to the python scripts https://reviews.llvm.org/D30691 Files: include/clang/AST/ASTContext.h include/clang/AST/Mangle.h

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

xazax.hun updated this revision to Diff 96377. xazax.hun marked an inline comment as done. xazax.hun added a comment. - Removed more unused code. - Remove the usages of posix API. - Changes according to some review comments. - Add a test to the clang-func-mapping tool.

[PATCH] D30489: [analyzer] catch out of bounds for VLA

xazax.hun added a comment. In https://reviews.llvm.org/D30489#728945, @danielmarjamaki wrote: > I would propose that I rename and cleanup RangeConstraintManager::uglyEval() > and add it. When I tested it, the Z3 does not seem to handle this. What do you mean by Z3 not handling this? I mean,

[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. It looks good to me but let's wait for Anna, NoQ, or Devin for the final word. Repository: rL LLVM https://reviews.llvm.org/D30295 ___

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

xazax.hun added inline comments. Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:622 +} +//===--===// +// Check: Use of 'sprintf', 'vsprintf', 'scanf', 'wscanf', 'fscanf', I

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

xazax.hun added a comment. In https://reviews.llvm.org/D35068#811437, @NoQ wrote: > It'd look good in clang-tidy (especially if extended to provide fixits), but > if Daniel is interested in having this feature in the analyzer (and picked by > clang-tidy from there), i wouldn't mind. > > I

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

xazax.hun added a comment. Even though it is not undefined behavior in C, it can still cause surprising behavior for the users. I think maybe putting it into the optin package instead of cplusplus is better. What do you think? https://reviews.llvm.org/D33672

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

xazax.hun added a comment. Even though it is not undefined behavior in C, it can still cause surprising behavior for the users. I think maybe putting it into the optin package instead of cplusplus is better. What do you think? https://reviews.llvm.org/D33672

[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width

xazax.hun added inline comments. Comment at: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp:126 + << BinaryOperator::getOpcodeStr(B->getOpcode()) + << "' expression is undefined due to shift count >= width of type"; + } else {

[PATCH] D36141: [StaticAnalyzer] Fix FP in UnreachableCodeChecker

xazax.hun accepted this revision. xazax.hun added reviewers: NoQ, dcoughlin. xazax.hun added a comment. This revision is now accepted and ready to land. LGTM! Repository: rL LLVM https://reviews.llvm.org/D36141 ___ cfe-commits mailing list

[PATCH] D36471: [StaticAnalyzer] Try to calculate arithmetic result when operand has a range of possible values

xazax.hun added a comment. Can't you reuse somehow some machinery already available to evaluate the arithmetic operators? Those should already handle most of your TODOs and overflows. Repository: rL LLVM https://reviews.llvm.org/D36471 ___

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

xazax.hun added a subscriber: NoQ. xazax.hun added a comment. @NoQ , @dcoughlin could either of you review this patch as well? The end of GSoC is closing and it would be awesome to be able to commit this before it ends. :) Comment at:

[PATCH] D33095: [analyzer] Avoid allocation in Std C function modelling.

xazax.hun added a comment. In https://reviews.llvm.org/D33095#752879, @NoQ wrote: > Is it an actual performance problem? Cause i think i did somehow test the > checker for performance regressions and it seemed all good. I did not measure it. Just spotted by skimming through the code. I think,

[PATCH] D33092: [analyzer] Add checker to model builtin functions

xazax.hun added a comment. In https://reviews.llvm.org/D33092#752039, @NoQ wrote: > Hmm, shouldn't this be part of `BuiltinFunctionChecker` aka > `core.builtin.BuiltinFunctions`? We already have `__builtin_assume_aligned` > here (though it doesn't seem to assume anything because that

[PATCH] D33092: [analyzer] Add checker to model builtin functions

xazax.hun updated this revision to Diff 98609. xazax.hun marked an inline comment as done. xazax.hun edited the summary of this revision. xazax.hun added a comment. - Move this to the right checker. https://reviews.llvm.org/D33092 Files: lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp

[PATCH] D33092: [analyzer] Add checker to model builtin functions

xazax.hun created this revision. Herald added subscribers: whisperity, mgorny. I added a checker to model builtin functions. Only one builtin function is modelled so far. The motivation behind using `__builtin_assume` from the analyzers point of view is to add assumptions. The conventional way

[PATCH] D33095: [analyzer] Avoid allocation in Std C function modelling.

xazax.hun created this revision. Herald added a subscriber: whisperity. Avoid an allocation in modelling Std C functions to improve the performance. Repository: rL LLVM https://reviews.llvm.org/D33095 Files: lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp Index:

[PATCH] D32751: [ASTImporter] Support new kinds of declarations (mostly Using*)

xazax.hun accepted this revision. xazax.hun added a reviewer: szepet. xazax.hun added a comment. This revision is now accepted and ready to land. LGTM! Adding Peter as a reviewer, as he is more experienced with the ASTImporter than me. https://reviews.llvm.org/D32751

[PATCH] D32751: [ASTImporter] Support new kinds of declarations (mostly Using*)

xazax.hun added a comment. Looks good for me, I only have a few questions. Could you reupload the diff with contexts? It might make the review easier for others. Comment at: lib/AST/ASTImporter.cpp:1311 + EmptyDecl *ToD = EmptyDecl::Create(Importer.getToContext(), DC, Loc);

[PATCH] D28953: [analyzer] Eliminate analyzer limitations on symbolic constraint generation

xazax.hun added a comment. Do you have a benchmark how this affects the performance and memory usage when the old constraint manager is used? I wonder if most of people are using the old one, it might make no sense to generate symbolic expressions that can not be solved anyway. Maybe the

[PATCH] D32350: [Analyzer] Exception checker for misuse: uncaught/noncompliant throws

xazax.hun added inline comments. Comment at: lib/StaticAnalyzer/Checkers/ExceptionMisuseChecker.cpp:105 + ReportExnSpec(ReportExnSpec) { + // In the beginning we have to parse list formatted options + SmallVector EnabledFuncsVec; All

[PATCH] D33191: [analyzer] ObjCGenerics: account for __kindof specifiers met along a chain of casts.

xazax.hun added a comment. In https://reviews.llvm.org/D33191#756174, @NoQ wrote: > @Gábor: I didn't want to bother you with this, but i'm not entirely sure > about how to deal with these false positives; since you're the original > author of this check, if you see anything obviously wrong

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

xazax.hun added a comment. I do not see any test cases for this patch. Could you add them as well? Are you sure that this representation is ok? How do you handle the following case? struct A { A() { X x; x.virtualMethod(); // this virtual call is ok } } int main() {

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

xazax.hun added a comment. Note that when you update the differential revision you need to upload the whole diff. Your diff now only contains the tests but not the code. In https://reviews.llvm.org/D34275#785189, @wangxindsb wrote: > > How do you handle the following case? > > > > struct A

[PATCH] D34277: [analyzer] Bump default performance thresholds?

xazax.hun added a comment. While I have no objections, I am wondering whether this is the good way to spend the performance budget. In particular, there are patches to generate more symbolic expressions, that could also degrade the performance (but fix some fixmes along the way).

[PATCH] D34502: [analyzer] Do not continue to analyze a path if the constraints contradict with builtin assume

xazax.hun created this revision. Herald added a subscriber: whisperity. This is how asserts are working right now. This way the semantics of __builtin_assume will be identical to asserts. I also moved the tests to another file. Repository: rL LLVM https://reviews.llvm.org/D34502 Files:

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

xazax.hun added inline comments. Comment at: docs/clang-tidy/checks/readability-redundant-keyword.rst:8 + +`extern` is redundant in function declarations + alexfh wrote: > Could you explain, why you think `extern` is redundant in function > declarations? Just

[PATCH] D34506: Relax an assert in the comparison of source locations

xazax.hun created this revision. Right now source locations from different translation units can not be compared. This is a problem for an upcoming feature in the Static Analyzer, the cross translation unit support (https://reviews.llvm.org/D30691). It would be great to be able to sort the

[PATCH] D34506: Relax an assert in the comparison of source locations

xazax.hun added a comment. Note that, it is not easy to add a test case for this patch without the https://reviews.llvm.org/D30691 being accepted. Repository: rL LLVM https://reviews.llvm.org/D34506 ___ cfe-commits mailing list

[PATCH] D34449: [Clang-tidy] Enable constexpr definitions in headers.

xazax.hun created this revision. xazax.hun added a project: clang-tools-extra. Herald added a subscriber: whisperity. Constexpr variable definitions should be ok in headers. https://stackoverflow.com/questions/34445336/constexpr-global-constants-in-a-header-file-and-odr Repository: rL LLVM

[PATCH] D34506: Relax an assert in the comparison of source locations

xazax.hun added a comment. In https://reviews.llvm.org/D34506#787971, @joerg wrote: > I don't think it is a good idea to make this function non-transitive. I think this is a good point. However, I am not entirely sure that it is transitive right now. Check the "Both are in built-in buffers,

[PATCH] D34512: [libTooling] Add preliminary Cross Translation Unit support for libTooling

xazax.hun updated this revision to Diff 103707. xazax.hun marked an inline comment as done. xazax.hun edited the summary of this revision. xazax.hun added a comment. - Added test for the USR dumping tool. - Added unit test for the CrossTranslationUnit class - Added documentation for the API entry

[PATCH] D34512: [libTooling] Add preliminary Cross Translation Unit support for libTooling

xazax.hun added a comment. Unfortunately, in order to make the Unit Test pass, I also had to modify the AST Importer. Are you ok with having that change as part of this patch (Aleksei might be a suitable person to review that part) or should I make a separate differential for that?

[PATCH] D34512: [libTooling] Add preliminary Cross Translation Unit support for libTooling

xazax.hun updated this revision to Diff 103710. xazax.hun added a comment. - Removed an unrelated change - Made the unit test more strict https://reviews.llvm.org/D34512 Files: include/clang/Tooling/CrossTranslationUnit.h lib/AST/ASTImporter.cpp lib/Tooling/CMakeLists.txt

[PATCH] D34512: [libTooling] Add preliminary Cross Translation Unit support for libTooling

xazax.hun added inline comments. Comment at: include/clang/Tooling/CrossTranslationUnit.h:53-58 + /// \p CrossTUDir directory, called \p IndexName. In case the declaration is + /// found in the index the corresponding AST file will be loaded and the + /// definition of the

[PATCH] D34512: [libTooling] Add preliminary Cross Translation Unit support for libTooling

xazax.hun updated this revision to Diff 103571. xazax.hun added a comment. - Add a tool to dump USRs for function definitions. It can be used to create index files. https://reviews.llvm.org/D34512 Files: include/clang/Tooling/CrossTranslationUnit.h lib/Tooling/CMakeLists.txt

[PATCH] D34512: [libTooling] Add preliminary Cross Translation Unit support for libTooling

xazax.hun created this revision. Herald added a subscriber: mgorny. This patch introduces a class that can help to build tools that require cross translation unit facilities. This class allows function definitions to be loaded from external AST files based on an index. In order to use this

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

xazax.hun added a comment. Some of the CTU related analyzer independent parts are being factored out. The review is ongoing here: https://reviews.llvm.org/D34512 Another small and independent part is under review here: https://reviews.llvm.org/D34506 https://reviews.llvm.org/D30691

[PATCH] D34449: [clang-tidy] Enable constexpr definitions in headers.

xazax.hun updated this revision to Diff 104111. xazax.hun marked 5 inline comments as done. xazax.hun added a comment. Herald added a subscriber: JDevlieghere. - Updates according to the reviews. https://reviews.llvm.org/D34449 Files: clang-tidy/misc/DefinitionsInHeadersCheck.cpp

[PATCH] D34506: Relax an assert in the comparison of source locations

xazax.hun added a comment. In https://reviews.llvm.org/D34506#791089, @akyrtzi wrote: > Comparing SourceLocations from different translation units is not meaningful > and my concern is that treating source locations like this can very easily > lead to errors where by mistake the code is

[PATCH] D33191: [analyzer] ObjCGenerics: account for __kindof specifiers met along a chain of casts.

xazax.hun added a comment. In https://reviews.llvm.org/D33191#758583, @NoQ wrote: > I think i found a relatively clean way of storing the kindof specifiers, > which is within the most-specialized type object itself. > > Like, if we see `Foo` being casted to `Foo<__kindof X, Y>` and in >

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

xazax.hun updated this revision to Diff 101695. xazax.hun edited the summary of this revision. xazax.hun added a comment. - Migrate to use USR instead of mangled names. Name mangling related changes are reverted. - Better error handling in some cases. - File paths containing spaces are now

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

xazax.hun marked 2 inline comments as done. xazax.hun added a comment. In https://reviews.llvm.org/D30691#731617, @zaks.anna wrote: > I agree that scan-build or scan-build-py integration is an important issue to > resolve here. What I envision is that users will just call scan-build and > pass

[PATCH] D30489: [analyzer] catch out of bounds for VLA

xazax.hun added a comment. In https://reviews.llvm.org/D30489#769916, @NoQ wrote: > An idea. I believe the safest way to find the bugs you mentioned would be to > replace extent-as-a-symbol with extent-as-a-trait. > > I.e., currently we construct `extent_$3{SymRegion{conj_$1{char *}}}`, assume

[PATCH] D32743: [clang-tidy] Add new cert-dcl21-cpp check.

xazax.hun updated this revision to Diff 97789. xazax.hun marked 21 inline comments as done. xazax.hun added a comment. - Fixes according to the review comments. https://reviews.llvm.org/D32743 Files: clang-tidy/cert/CERTTidyModule.cpp clang-tidy/cert/CMakeLists.txt

[PATCH] D32743: [clang-tidy] Add new cert-dcl21-cpp check.

xazax.hun updated this revision to Diff 97790. xazax.hun added a comment. - Fixed include order. https://reviews.llvm.org/D32743 Files: clang-tidy/cert/CERTTidyModule.cpp clang-tidy/cert/CMakeLists.txt clang-tidy/cert/PostfixOperatorCheck.cpp clang-tidy/cert/PostfixOperatorCheck.h

[PATCH] D32700: [clang-tidy] Add misc-suspicious-memset-usage check.

xazax.hun added inline comments. Comment at: clang-tidy/misc/SuspiciousMemsetUsageCheck.cpp:21 +void SuspiciousMemsetUsageCheck::registerMatchers(MatchFinder *Finder) { + const auto HasCtorOrDtor = + eachOf(hasMethod(cxxConstructorDecl(unless(anyOf( I

[PATCH] D32743: [clang-tidy] Add new cert-dcl21-cpp check.

xazax.hun added inline comments. Comment at: clang-tidy/cert/PostfixOperatorCheck.cpp:72 + + diag(Location, "return type of overloaded %0 is not a constant type") + << FuncDecl << FixItHint::CreateInsertion(Location, "const "); aaron.ballman wrote: >

[PATCH] D32743: [clang-tidy] Add new cert-dcl21-cpp check.

xazax.hun updated this revision to Diff 98097. xazax.hun marked 3 inline comments as done. xazax.hun added a comment. - Fix alphabetical ordering of files in cmake. - Let clang-format do its job. https://reviews.llvm.org/D32743 Files: clang-tidy/cert/CERTTidyModule.cpp

[PATCH] D32702: [analyzer] Fix 'Memory Error' bugtype capitalization.

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

[PATCH] D32702: [analyzer] Fix 'Memory Error' bugtype capitalization.

xazax.hun accepted this revision. xazax.hun added a comment. Even better :) https://reviews.llvm.org/D32702 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32743: [clang-tidy] Add new cert-dcl21-cpp check.

xazax.hun created this revision. xazax.hun added a project: clang-tools-extra. Herald added subscribers: whisperity, mgorny. This check flags postfix ``operator++`` and ``operator--`` declarations, where the return type is not a const value type. It also attempt to fix the error. This rule is

[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width

xazax.hun added inline comments. Comment at: lib/StaticAnalyzer/Core/CheckerHelpers.cpp:116 +return false; + ConstraintManager = State->getConstraintManager(); + ProgramStateRef StTrue, StFalse; danielmarjamaki wrote: > xazax.hun wrote: > > Any reason why

[PATCH] D32743: [clang-tidy] Add new cert-dcl21-cpp check.

xazax.hun updated this revision to Diff 98179. xazax.hun added a comment. - Do not do fixits for type aliases. https://reviews.llvm.org/D32743 Files: clang-tidy/cert/CERTTidyModule.cpp clang-tidy/cert/CMakeLists.txt clang-tidy/cert/PostfixOperatorCheck.cpp

[PATCH] D32743: [clang-tidy] Add new cert-dcl21-cpp check.

xazax.hun marked 4 inline comments as done. xazax.hun added inline comments. Comment at: test/clang-tidy/cert-dcl21-cpp.cpp:1 +// RUN: %check_clang_tidy %s cert-dcl21-cpp %t + aaron.ballman wrote: > xazax.hun wrote: > > aaron.ballman wrote: > > > alexfh wrote: >

[PATCH] D32743: [clang-tidy] Add new cert-dcl21-cpp check.

xazax.hun updated this revision to Diff 98162. xazax.hun added a comment. - Added more test cases and make them pass. https://reviews.llvm.org/D32743 Files: clang-tidy/cert/CERTTidyModule.cpp clang-tidy/cert/CMakeLists.txt clang-tidy/cert/PostfixOperatorCheck.cpp

[PATCH] D32743: [clang-tidy] Add new cert-dcl21-cpp check.

xazax.hun added inline comments. Comment at: test/clang-tidy/cert-dcl21-cpp.cpp:1 +// RUN: %check_clang_tidy %s cert-dcl21-cpp %t + aaron.ballman wrote: > alexfh wrote: > > As usual, please add tests with macros and templates with multiple > > instantiations.

[PATCH] D34449: [clang-tidy] Enable inline variable definitions in headers.

xazax.hun updated this revision to Diff 104360. xazax.hun marked 2 inline comments as done. xazax.hun retitled this revision from "[clang-tidy] Enable constexpr definitions in headers. " to "[clang-tidy] Enable inline variable definitions in headers. ". xazax.hun edited the summary of this

[PATCH] D34506: Factor out a functionality from `isBeforeInTranslationUnit`

xazax.hun updated this revision to Diff 104358. xazax.hun added a comment. - Removed the whitespace changes - Factored out one more condition https://reviews.llvm.org/D34506 Files: include/clang/Basic/SourceManager.h lib/Basic/SourceManager.cpp Index: lib/Basic/SourceManager.cpp

[PATCH] D34506: Factor out a functionality from `isBeforeInTranslationUnit`

xazax.hun added a comment. In https://reviews.llvm.org/D34506#792313, @akyrtzi wrote: > I'd prefer to avoid including whitespace-only changes (there are a couple of > lines in the diff with only whitespace change), otherwise LGTM! Great, thank you! If no one has objections I will commit this

[PATCH] D37897: [StaticAnalyzer] Fix ProgramState for static variables that are not written

xazax.hun added a comment. Out of curiosity, does the false positive disappear after making the static variables const? Repository: rL LLVM https://reviews.llvm.org/D37897 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D37897: [StaticAnalyzer] Fix ProgramState for static variables that are not written

xazax.hun added a comment. I have some comments and questions but maybe you do not want to address those until Devin, NoQ, or Anna approved the general directions. Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:107 +/** Recursively check if variable is changed in code. */

[PATCH] D34512: Add preliminary Cross Translation Unit support library

xazax.hun added a comment. Any further reviews? What are the criteria to accept this patch? Who or how many people should accept this? https://reviews.llvm.org/D34512 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D37978: [analyzer] Fix an assertion fail in VirtualCallChecker

xazax.hun created this revision. Herald added subscribers: baloghadamsoftware, whisperity. This patch attempts to fix PR34451. The `getBestDynamicClassType` call can only handle `RecordDecls` and pointers to `RecordDecls`. The code removed all the implicit casts, this includes the array to

[PATCH] D34512: Add preliminary Cross Translation Unit support library

xazax.hun updated this revision to Diff 116195. xazax.hun added a comment. - Unittests now creates temporary files at the correct location - Temporary files are also cleaned up when the process is terminated - Absolute paths are handled correctly by the library https://reviews.llvm.org/D34512

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

xazax.hun added a comment. Ping. 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] D37963: [analyzer] PthreadLock: Don't track dead regions.

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. Maybe it worth a comment why we do not want to clean up `LockSet`? Otherwise LGTM! https://reviews.llvm.org/D37963 ___ cfe-commits

[PATCH] D34512: Add preliminary Cross Translation Unit support library

xazax.hun added a comment. In https://reviews.llvm.org/D34512#877032, @dcoughlin wrote: > I'm OK with this going into the repo a is (although it is light on tests!), > as long as we have an agreement that you'll be OK with iteration on both the > interface and the implementation to handle

[PATCH] D37812: [analyzer] PthreadLock: Escape the pointers.

xazax.hun added inline comments. Comment at: lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp:594 +// it takes the mutex explicitly as an argument. +if (IsLibraryFunction && +std::find(ExplicitRegions.begin(), ExplicitRegions.end(), R) ==

[PATCH] D31538: [analyzer] MisusedMovedObjectChecker: Fix a false positive on state-resetting a base-class sub-object.

xazax.hun accepted this revision. xazax.hun added a comment. I think there was only one comment but that is already addressed in a dependent revision. So I think this one is good as is. https://reviews.llvm.org/D31538 ___ cfe-commits mailing list

[PATCH] D38674: [analyzer] MisusedMovedObjectChecker: More precise warning message

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

[PATCH] D31541: [analyzer] MisusedMovedObjectChecker: Add a printState() method.

xazax.hun accepted this revision. xazax.hun added a comment. LGTM! https://reviews.llvm.org/D31541 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38675: [analyzer] MisusedMovedObjectChecker: Moving the checker out of alpha state

xazax.hun added a comment. In https://reviews.llvm.org/D38675#891750, @danielmarjamaki wrote: > > However, the checker seems to work with a low false positive rate. (<15 on > > the LLVM, 6 effectively different) > > This does not sound like a low false positive rate to me. Could you describe

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

xazax.hun added a comment. Let's also summarize what do we have now and what do we want. > I also think this sounds good, though I'm not quite sure it can be done in > this patch. Anyway, we should definitely specify what we expect from > first-class citizen checks. Please correct & extend the

[PATCH] D38943: [ASTImporter] import SubStmt of CaseStmt

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LGTM with a nit. Comment at: unittests/AST/ASTImporterTest.cpp:100 + // This might catch other cases. + Imported->dump(ToNothing); I would

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

xazax.hun updated this revision to Diff 119141. xazax.hun marked 2 inline comments as done. xazax.hun added a comment. Herald added a subscriber: szepet. - Address review comments. https://reviews.llvm.org/D37437 Files: include/clang/StaticAnalyzer/Core/BugReporter/BugType.h

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

xazax.hun added inline comments. Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:308 if (StOutBound && !StInBound) { +if (!Filter.CheckCStringOutOfBounds) + return state; zaks.anna wrote: > This seems to be related to the change in the

[PATCH] D38943: [ASTImporter] import SubStmt of CaseStmt

xazax.hun added inline comments. Comment at: unittests/AST/ASTImporterTest.cpp:100 + // This might catch other cases. + Imported->dump(ToNothing); r.stahl wrote: > xazax.hun wrote: > > I would elaborate a bit more on the purpose of the code below. > I will

[PATCH] D38688: [clang-tidy] Misc redundant expressions checker updated for macros

xazax.hun added a comment. Thanks for working on this, these additions look very useful to me. Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:124 + case Stmt::CXXFunctionalCastExprClass: +return cast(Left)->getTypeAsWritten() == You could

[PATCH] D38728: [analyzer] Use the signature of the primary template for issue hash calculation

xazax.hun added inline comments. Comment at: lib/StaticAnalyzer/Core/IssueHash.cpp:39 + // primary template. + if (const FunctionDecl *InstantiatedFrom = + Target->getInstantiatedFromMemberFunction()) martong wrote: > Could we use here

[PATCH] D38728: [analyzer] Use the signature of the primary template for issue hash calculation

xazax.hun added a comment. In https://reviews.llvm.org/D38728#895669, @NoQ wrote: > I think it would be great to split them into two different patches, to be > able to easily see how the change in the hashing affects the tests (and maybe > revert easily if something goes wrong). So you would

[PATCH] D34512: Add preliminary Cross Translation Unit support library

xazax.hun added a comment. In https://reviews.llvm.org/D34512#890537, @r.stahl wrote: > In https://reviews.llvm.org/D34512#877643, @xazax.hun wrote: > > > - Unittests now creates temporary files at the correct location > > - Temporary files are also cleaned up when the process is terminated > >

[PATCH] D38842: [CrossTU] Fix handling of Cross Translation Unit directory path

xazax.hun created this revision. The function map generator tool always creates absolute path. The correct logic to determine whether a path should be handled as absolute depends on the value of the CrossTU directory. Added a unit test to avoid regressions. https://reviews.llvm.org/D38842

[PATCH] D38728: [analyzer] Use the signature of the primary template for issue hash calculation

xazax.hun updated this revision to Diff 118788. xazax.hun added a comment. - Rebase based on the dependent revision and minor cleanups https://reviews.llvm.org/D38728 Files: lib/StaticAnalyzer/Core/IssueHash.cpp test/Analysis/bug_hash_test.cpp test/Analysis/edges-new.mm Index:

[PATCH] D34512: Add preliminary Cross Translation Unit support library

xazax.hun updated this revision to Diff 114133. xazax.hun marked 8 inline comments as done. xazax.hun added a comment. - Address review comments. https://reviews.llvm.org/D34512 Files: include/clang/Basic/AllDiagnostics.h include/clang/Basic/CMakeLists.txt

[PATCH] D34512: Add preliminary Cross Translation Unit support library

xazax.hun added inline comments. Comment at: lib/CrossTU/CrossTranslationUnit.cpp:35 +namespace { +// FIXME: This class is will be removed after the transition to llvm::Error. +class IndexErrorCategory : public std::error_category { dcoughlin wrote: > Is this

[PATCH] D34512: Add preliminary Cross Translation Unit support library

xazax.hun added a comment. In https://reviews.llvm.org/D34512#856821, @benlangmuir wrote: > In https://reviews.llvm.org/D34512#856301, @xazax.hun wrote: > > > In https://reviews.llvm.org/D34512#856184, @dcoughlin wrote: > > > > > In either case, the important scenario I think we should support

[PATCH] D34512: Add preliminary Cross Translation Unit support library

xazax.hun updated this revision to Diff 113206. xazax.hun added a comment. - Added unit test to ensure the produced index format can be parsed. - Added further diagnostics. https://reviews.llvm.org/D34512 Files: include/clang/Basic/AllDiagnostics.h include/clang/Basic/CMakeLists.txt

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