Re: [PATCH] D14286: ASTImporter: expressions, pt.1

a.sidorin added a comment. Serge Pavlov: I'll enable tests from you patch (http://reviews.llvm.org/D14224) after all node types your patch supports will be supported. If you're agree, of course. Repository: rL LLVM http://reviews.llvm.org/D14286 _

Re: [PATCH] D17446: ASTMatchers: add new statement/decl matchers

a.sidorin removed rL LLVM as the repository for this revision. a.sidorin updated this revision to Diff 48503. a.sidorin added a comment. Fix issues pointed by Aaron. http://reviews.llvm.org/D17446 Files: docs/LibASTMatchersReference.html include/clang/ASTMatchers/ASTMatchers.h include/cla

[PATCH] D19979: [analyzer] ScopeContext - initial implementation

a.sidorin created this revision. a.sidorin added reviewers: zaks.anna, dcoughlin, bshastry. a.sidorin added a subscriber: cfe-commits. This patch enables ScopeContext to track variable lifetime. It is RFC mostly since the work on its dependencies is still not finished and it lacks some tests. Pr

Re: [PATCH] D14326: ASTImporter: expressions, pt.2

a.sidorin updated the summary for this revision. a.sidorin added a reviewer: spyffe. a.sidorin removed rL LLVM as the repository for this revision. a.sidorin updated this revision to Diff 56682. a.sidorin added a comment. - Some code cleanup - Add tests not present in http://reviews.llvm.org/D1428

Re: [PATCH] D14326: ASTImporter: expressions, pt.2

a.sidorin marked 7 inline comments as done. Comment at: lib/AST/ASTImporter.cpp:5859 @@ +5858,3 @@ + Expr *ToQueried = Importer.Import(E->getQueriedExpression()); + if (!ToQueried) +return nullptr; I usually prefer allocation with required size at the start

Re: [PATCH] D14326: ASTImporter: expressions, pt.2

a.sidorin updated this revision to Diff 56860. a.sidorin added a comment. Add error checks; replace `NULL` with `nullptr`. http://reviews.llvm.org/D14326 Files: include/clang/AST/ASTImporter.h include/clang/AST/DeclFriend.h lib/AST/ASTImporter.cpp test/ASTMerge/Inputs/class3.cpp test/

Re: [PATCH] D14326: ASTImporter: expressions, pt.2

a.sidorin marked 4 inline comments as done. Comment at: lib/AST/ASTImporter.cpp:5527 @@ -5353,3 +5526,3 @@ FoundD, - /*FIXME:TemplateArgs=*/nullptr); + /*Templ

Re: [PATCH] D20118: Add support for injected class names and constructor initializers in C++

a.sidorin added a comment. I' d like to have some tests for this. CXXCtorInitializers can be tested with D14224 -like stuff or with ASTMatchers (ASTImporterTest.cpp). Some code samples may be found in CXX/Sema tests, you just need to port/simplify them. ===

Re: [PATCH] D14326: ASTImporter: expressions, pt.2

a.sidorin updated this revision to Diff 57796. a.sidorin marked an inline comment as done. a.sidorin added a comment. Add some basic tests for ExpressionTraitExpr and ArrayTypeTraitExpr. http://reviews.llvm.org/D14326 Files: include/clang/AST/ASTImporter.h include/clang/AST/DeclFriend.h l

Re: [PATCH] D15031: CFG: Add CFGElement for automatic variables that leave the scope

a.sidorin added a subscriber: a.sidorin. a.sidorin added a comment. Matthias, Could you take a look at http://reviews.llvm.org/D16403? It looks like a duplicating work but it should cover not Objective-C only. http://reviews.llvm.org/D15031 ___ cf

Re: [PATCH] D14286: ASTImporter: expressions, pt.1

a.sidorin updated this revision to Diff 53389. a.sidorin added a comment. Update to current master; Fix a memory leak introduced in cf8ccff5: an array is allocated in ImportArray and never freed. http://reviews.llvm.org/D14286 Files: include/clang/AST/Type.h lib/AST/ASTImporter.cpp unitt

Re: [PATCH] D14326: ASTImporter: expressions, pt.2

a.sidorin added a comment. Ping? http://reviews.llvm.org/D14326 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D14326: ASTImporter: expressions, pt.2

a.sidorin marked 5 inline comments as done. a.sidorin added a comment. Thank you for your comments! And sorry for late response after pinging. Comment at: lib/AST/ASTImporter.cpp:2518-2519 @@ +2517,4 @@ +Importer.Import(D->getMessage())); + if (!Message) +return nul

Re: [PATCH] D14326: ASTImporter: expressions, pt.2

a.sidorin marked 2 inline comments as done. Comment at: lib/AST/ASTImporter.cpp:3363 @@ +3362,3 @@ + + // Determine whether we've already imported this decl. + // FriendDecl is not a NamedDecl so we cannot use localUncachedLookup. a.sidorin wrote: > sepavloff wr

Re: [PATCH] D14326: ASTImporter: expressions, pt.2

a.sidorin updated this revision to Diff 62474. a.sidorin added a comment. Fix some issues pointed by Serge Pavlov. Serge: putting a check of FriendDecls to `IsStructurallyEquivalent()` seems like a good idea for me, but we need some additional changes for it. `CXXRecodeDecl::getFirstDecl()` is

Re: [PATCH] D14277: [Analyzer] Make referenced SymbolMetadata live even if its region is dead

a.sidorin added a comment. Gabor: Ok,will do. Repository: rL LLVM http://reviews.llvm.org/D14277 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D21978: [analyzer] Add LocationContext to SymbolMetadata

a.sidorin created this revision. a.sidorin added reviewers: zaks.anna, dcoughlin. a.sidorin added a subscriber: cfe-commits. Currently, SymbolMetadata are distinguished by statement and block count. However, in case of inlining, block counts may be the same every time function is called because

Re: [PATCH] D21978: [analyzer] Add LocationContext to SymbolMetadata

a.sidorin added a comment. In http://reviews.llvm.org/D21978#476200, @NoQ wrote: > I also never noticed that the block count gets reset on every stack frame, > that's pretty unobvious. Yes, that was surprising for me too. > Uhm, and it's already there in SymbolConjured. Anyway, fixing the blo

[PATCH] D22090: [analyzer] Add more FileIDs to PlistDiagnostic map

a.sidorin created this revision. a.sidorin added reviewers: zaks.anna, dcoughlin. a.sidorin added a subscriber: cfe-commits. Some FileIDs that may be used by PlistDiagnostics are not added while building a list of pieces. This leads to assertion violation in `GetFID()` function. This patch tries

Re: [PATCH] D22374: [analyzer] Copy and move constructors - ExprEngine extended for "almost trivial" copy and move constructors

a.sidorin added inline comments. Comment at: test/Analysis/ctor.mm:177 @@ -176,3 +176,3 @@ Inner(const Inner &Other) -: x(Other.x), y(Other.y) // expected-warning {{undefined}} +: x(Other.x), y(Other.y) // no-warning { Seems like we m

Re: [PATCH] D14326: ASTImporter: expressions, pt.2

a.sidorin added a comment. > I don't think this small improvement in Importer is worth invasive changes in > other components. Thanks, Serge. Is it OK to commit? https://reviews.llvm.org/D14326 ___ cfe-commits mailing list cfe-commits@lists.llvm.

Re: [PATCH] D14286: ASTImporter: expressions, pt.1

a.sidorin added a comment. Serge: Sorry, that's because I forgot to add a dependence. There should be a patch implementing these matchers. Repository: rL LLVM http://reviews.llvm.org/D14286 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D17446: ASTMatchers: add new statement/decl matchers

a.sidorin updated this revision to Diff 49727. a.sidorin marked 15 inline comments as done. a.sidorin added a comment. Add more comments; resolve issues pointed on review. http://reviews.llvm.org/D17446 Files: docs/LibASTMatchersReference.html include/clang/ASTMatchers/ASTMatchers.h inclu

Re: [PATCH] D17446: ASTMatchers: add new statement/decl matchers

a.sidorin updated this revision to Diff 50119. a.sidorin added a comment. Improved doxygen comments. http://reviews.llvm.org/D17446 Files: docs/LibASTMatchersReference.html include/clang/ASTMatchers/ASTMatchers.h include/clang/ASTMatchers/ASTMatchersInternal.h lib/ASTMatchers/Dynamic/Re

Re: [PATCH] D14286: ASTImporter: expressions, pt.1

a.sidorin removed rL LLVM as the repository for this revision. a.sidorin updated this revision to Diff 50140. a.sidorin added a comment. Stylish fixes. Serge: ASTMatcher review is complete so there should be no more build problems. Could you try again? Also, I'd prefer to add your changes in the

Re: [PATCH] D14286: ASTImporter: expressions, pt.1

a.sidorin added a comment. Serge: BTW, tests for CXXBoolLiteralExpr are presented in this patch. There is no test //exactly// for it but at least one test uses bool literal and matches it successfully with cxxBoolLiteral() matcher. http://reviews.llvm.org/D14286

Re: [PATCH] D16873: Refactor MemRegionManager::getVarRegion to call two new functions, improving readability

a.sidorin added a comment. Alexander, could you update status of this review? http://reviews.llvm.org/D16873 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D14286: ASTImporter: expressions, pt.1

a.sidorin updated this revision to Diff 50741. a.sidorin added a comment. An attempt to fix ParenListExpr test on Windows http://reviews.llvm.org/D14286 Files: include/clang/AST/Type.h lib/AST/ASTImporter.cpp unittests/AST/ASTImporterTest.cpp unittests/AST/CMakeLists.txt Index: unittes

Re: [PATCH] D14286: ASTImporter: expressions, pt.1

a.sidorin updated this revision to Diff 51027. a.sidorin added a comment. Serge, thank you for help! GNUNullExpr was 'long' on my platform. I weakened a matcher to check only if it has integer type. ParenListExpr issue was also successfully reproduced with 'fdelayed-template-parsing' and fixed.

Re: [PATCH] D14286: ASTImporter: expressions, pt.1

a.sidorin added a comment. Is this patch OK to submit? http://reviews.llvm.org/D14286 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D15794: CheckerContext::isCLibraryFunction(): small refactoring; NFC

a.sidorin created this revision. a.sidorin added reviewers: zaks.anna, xazax.hun, dcoughlin. a.sidorin added a subscriber: cfe-commits. a.sidorin set the repository for this revision to rL LLVM. Use getRedeclContext() instead of a manually written loop; fix a comment. Repository: rL LLVM http:

Re: [PATCH] D15410: AnalysisConsumer: use canonical decl for both lookup and store of visited decls

a.sidorin removed rL LLVM as the repository for this revision. a.sidorin updated this revision to Diff 43723. a.sidorin added a comment. Added a comment. http://reviews.llvm.org/D15410 Files: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp test/Analysis/inlining/analysis-order.c Index: te

Re: [PATCH] D15410: AnalysisConsumer: use canonical decl for both lookup and store of visited decls

a.sidorin updated this revision to Diff 43726. a.sidorin added a comment. C++11-fy adding loop. (Sorry for the noise.) http://reviews.llvm.org/D15410 Files: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp test/Analysis/inlining/analysis-order.c Index: test/Analysis/inlining/analysis-order

[PATCH] D16063: [Analyzer] Use a wider integer type for an array index

a.sidorin created this revision. a.sidorin added reviewers: zaks.anna, dcoughlin, xazax.hun. a.sidorin added a subscriber: cfe-commits. a.sidorin set the repository for this revision to rL LLVM. Currently, a default index type in CSA is 'int'. However, this assumption seems to be incorrect for 64

Re: [PATCH] D14277: [Analyzer] Make referenced SymbolMetadata live even if its region is dead

a.sidorin added a comment. Ping? Repository: rL LLVM http://reviews.llvm.org/D14277 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D16063: [Analyzer] Use a wider integer type for an array index

a.sidorin added a comment. In http://reviews.llvm.org/D16063#323462, @xazax.hun wrote: > How is this array index type used? This type is used to make all the symbolic values for indices to have the same integer type (in SValBuilder). In http://reviews.llvm.org/D16063#323462, @xazax.hun wrote:

Re: [PATCH] D16063: [Analyzer] Use a wider integer type for an array index

a.sidorin removed rL LLVM as the repository for this revision. a.sidorin updated this revision to Diff 44606. a.sidorin added a comment. Fix a test. http://reviews.llvm.org/D16063 Files: include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h test/Analysis/index-type.c Index: test/An

Re: [PATCH] D16063: [Analyzer] Use a wider integer type for an array index

a.sidorin added a comment. LongLongTy is initialized as a built-in type in ASTContext and, as I understand, is always available. It is not 64-bit on some platforms (like TCE), but these platforms don't have a wider type too. http://reviews.llvm.org/D16063 ___

[PATCH] D16215: ASTMatchers: enable hasBody() matcher for FunctionDecls

a.sidorin created this revision. a.sidorin added reviewers: klimek, aaron.ballman. a.sidorin added a subscriber: cfe-commits. Herald added a subscriber: klimek. This patch enables matching for FunctionDecls that have bodies. http://reviews.llvm.org/D16215 Files: include/clang/ASTMatchers/ASTMa

Re: [PATCH] D16215: ASTMatchers: enable hasBody() matcher for FunctionDecls

a.sidorin updated this revision to Diff 44975. a.sidorin added a comment. Update comments and documentation. http://reviews.llvm.org/D16215 Files: docs/LibASTMatchersReference.html include/clang/ASTMatchers/ASTMatchers.h unittests/ASTMatchers/ASTMatchersTest.cpp unittests/ASTMatchers/Dy

Re: [PATCH] D16215: ASTMatchers: enable hasBody() matcher for FunctionDecls

a.sidorin updated this revision to Diff 45232. a.sidorin added a comment. Match only FunctionDecls which are definitions and ignore redeclarations without bodies. http://reviews.llvm.org/D16215 Files: docs/LibASTMatchersReference.html include/clang/ASTMatchers/ASTMatchers.h include/clang

Re: [PATCH] D16215: ASTMatchers: enable hasBody() matcher for FunctionDecls

a.sidorin updated this revision to Diff 45267. a.sidorin added a comment. Fix issues pointed on review. http://reviews.llvm.org/D16215 Files: docs/LibASTMatchersReference.html include/clang/ASTMatchers/ASTMatchers.h include/clang/ASTMatchers/ASTMatchersInternal.h unittests/ASTMatchers/A

Re: [PATCH] D16215: ASTMatchers: enable hasBody() matcher for FunctionDecls

a.sidorin updated this revision to Diff 45360. a.sidorin added a comment. Replace NULL with nullptr. http://reviews.llvm.org/D16215 Files: docs/LibASTMatchersReference.html include/clang/ASTMatchers/ASTMatchers.h include/clang/ASTMatchers/ASTMatchersInternal.h unittests/ASTMatchers/ASTM

Re: [PATCH] D14277: [Analyzer] Make referenced SymbolMetadata live even if its region is dead

a.sidorin added a comment. > - What happens when you take a string's length, test it against something, > throw it away, then remeasure the string? In this case we'd be okay because > CStringChecker still hangs on to the symbol in its map until the region is > invalidated. We cannot throw out

Re: [PATCH] D14277: [Analyzer] Make referenced SymbolMetadata live even if its region is dead

a.sidorin added a comment. Sorry, I misunderstood your first question. This case is OK because CStringChecker handles invalidation and shouldn't track invalidated regions until their lengths become known. Test strlen_global() (string.c:106) should confirm this. Repository: rL LLVM http://r

Re: [PATCH] D14286: ASTImporter: expressions, pt.1

a.sidorin removed rL LLVM as the repository for this revision. a.sidorin updated this revision to Diff 40769. a.sidorin added a comment. Herald added a subscriber: klimek. Seems like I have found a way to test ASTImporter. What about some unit-tests? A sample test using AST matcher is attached. I

Re: [PATCH] D14286: ASTImporter: expressions, pt.1

a.sidorin updated this revision to Diff 40775. a.sidorin added a comment. Add language-related arguments for compilation. http://reviews.llvm.org/D14286 Files: include/clang/ASTMatchers/ASTMatchers.h lib/AST/ASTImporter.cpp unittests/AST/ASTImporterTest.cpp unittests/AST/CMakeLists.txt

Re: [PATCH] D14286: ASTImporter: expressions, pt.1

a.sidorin added a comment. I understand your position. However, we cannot check that we don't import `BreakStmt` as `ContinueStmt` using just ASTMerge without any verification. As I understand, unit tests is what we need because we need to check that the overall imported type/expression is cor

[PATCH] D15410: AnalysisConsumer: use canonical decl for both lookup and store of visited decls

a.sidorin created this revision. a.sidorin added reviewers: zaks.anna, xazax.hun, dcoughlin. a.sidorin added a subscriber: cfe-commits. a.sidorin set the repository for this revision to rL LLVM. Due to redeclarations, the function may have different declarations used in CallExpr and in the defini

Re: [PATCH] D15410: AnalysisConsumer: use canonical decl for both lookup and store of visited decls

a.sidorin updated this revision to Diff 42691. a.sidorin added a comment. Replaced CHECK with CHECK-NEXT Repository: rL LLVM http://reviews.llvm.org/D15410 Files: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp test/Analysis/inlining

Re: [PATCH] D13126: New static analyzer checker for loss of sign/precision

a.sidorin added a subscriber: a.sidorin. Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:41 @@ +40,3 @@ +const Stmt *Parent = PM.getParent(Cast); +if (!Parent) + return; Parent should always exist for an implicit cast. May be it's better

Re: [PATCH] D13126: New static analyzer checker for loss of sign/precision

a.sidorin added inline comments. Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:44 @@ +43,3 @@ + +const BinaryOperator *B = dyn_cast(Parent); +if (!B) Note that InitExprs of DeclStmts are not binary operators. So you will not get a warning

Re: [PATCH] D15410: AnalysisConsumer: use canonical decl for both lookup and store of visited decls

a.sidorin added a comment. Thank you. I'm not familiar with Objective-C/C++ issues so I missed that. I'll take an another look. BTW, maybe it is better to use definition decl instead of canonical? Repository: rL LLVM http://reviews.llvm.org/D15410

Re: [PATCH] D15410: AnalysisConsumer: use canonical decl for both lookup and store of visited decls

a.sidorin updated this revision to Diff 43674. a.sidorin marked an inline comment as done. a.sidorin added a comment. Canonicalize the inserted declarations in a single place; honor ObjCMethodDecls. Repository: rL LLVM http://reviews.llvm.org/D15410 Files: lib/StaticAnalyzer/Frontend/Analy

<    1   2