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

r.stahl added a comment. While testing this I stumbled upon a crash with the following test case: inc.h #define BASE ((int*)0) void foo(); main.c: #include "inc.h" void moo() { int a = BASE[0]; foo(); } other.c #include "inc.h" void foo() { int a =

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

r.stahl added a comment. In a similar case, static inline functions are an issue. inc.h void foo(); static inline void wow() { int a = *(int*)0; } main.c #include "inc.h" void moo() { foo(); wow(); } other.c #include "inc.h" void foo() { wow();

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

r.stahl added a comment. 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 > - Absolute paths are handled correctly by the library I think there

[PATCH] D37478: [analyzer] Implement pointer arithmetic on constants

r.stahl updated this revision to Diff 118182. r.stahl marked an inline comment as done. r.stahl edited the summary of this revision. r.stahl added a comment. Herald added a subscriber: szepet. addressed review comments. updated summary. https://reviews.llvm.org/D37478 Files:

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

r.stahl created this revision. This fixes importing of CaseStmts, because the importer was not importing its SubStmt. A test was added and the test procedure was adjusted to also dump the imported Decl, because otherwise the bug would not be detected. https://reviews.llvm.org/D38943 Files:

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

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

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

r.stahl added a comment. If all is good, I will need someone to commit this for me please. Comment at: unittests/AST/ASTImporterTest.cpp:100 + // This might catch other cases. + Imported->dump(ToNothing); xazax.hun wrote: > r.stahl wrote: > > xazax.hun

[PATCH] D37478: [analyzer] Implement pointer arithmetic on constants

r.stahl created this revision. Herald added a subscriber: eraman. The "Multiplicand" variable in SimpleSValBuilder::evalBinOpLN was always initialized to zero, causing all pointer arithmetic on constant values to be no-ops. This fixes two FIXMEs in existing tests. The added tests were all

[PATCH] D37478: [analyzer] Implement pointer arithmetic on constants

r.stahl added a comment. To be honest I was quite surprised that this change in behavior didn't cause more test failures, because for detecting null dereferences the old behavior is definitely more useful. Since it did not, I was convinced that this change is desired. We use the analyzer for

[PATCH] D37478: [analyzer] Implement pointer arithmetic on constants

r.stahl updated this revision to Diff 114126. r.stahl added a comment. addressed the review comments https://reviews.llvm.org/D37478 Files: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp test/Analysis/explain-svals.cpp test/Analysis/inlining/inline-defensive-checks.c

[PATCH] D37478: [analyzer] Implement pointer arithmetic on constants

r.stahl added a comment. Since I do not have commit access, it would be nice if someone committed this for me. Thanks! https://reviews.llvm.org/D37478 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

r.stahl accepted this revision. r.stahl added a comment. This revision is now accepted and ready to land. I'm gonna go ahead and approve this now, because I reported the issue. Note that I'm not a regular contributor, yet! Comment at: lib/CrossTU/CrossTranslationUnit.cpp:99

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

r.stahl created this revision. r.stahl added reviewers: NoQ, dcoughlin, xazax.hun, george.karpenkov. Herald added subscribers: cfe-commits, a.sidorin, rnkovacs, szepet. The existing CTU mechanism imports FunctionDecls where the definition is available in another TU. This patch extends that to

[PATCH] D46823: [analyzer] const init: handle non-explicit cases more accurately

r.stahl created this revision. r.stahl added reviewers: NoQ, xazax.hun, george.karpenkov. Herald added subscribers: cfe-commits, a.sidorin, rnkovacs, szepet. If the access is out of bounds, return UndefinedVal. If it is missing an explicit init, return the implicit zero value it must have.

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

r.stahl updated this revision to Diff 146609. r.stahl marked an inline comment as done. r.stahl added a comment. - added tests - updated doc and var naming - addressed review comment https://reviews.llvm.org/D46421 Files: include/clang/CrossTU/CrossTranslationUnit.h

[PATCH] D46633: [analyzer] add range check for InitList lookup

r.stahl created this revision. r.stahl added reviewers: alexfh, NoQ, george.karpenkov. Herald added subscribers: cfe-commits, a.sidorin, szepet, xazax.hun. Fixes issue introduced by https://reviews.llvm.org/rC331556. Closes bug: https://bugs.llvm.org/show_bug.cgi?id=37357 Repository: rC

[PATCH] D46115: [ASTImporter] properly import SrcLoc of Attr

r.stahl updated this revision to Diff 145641. r.stahl marked 6 inline comments as done. https://reviews.llvm.org/D46115 Files: include/clang/AST/ASTImporter.h lib/AST/ASTImporter.cpp test/Import/attr/Inputs/S.cpp test/Import/attr/test.cpp Index: test/Import/attr/Inputs/S.cpp

[PATCH] D46115: [ASTImporter] properly import SrcLoc of Attr

r.stahl added inline comments. Comment at: include/clang/AST/ASTImporter.h:137 +/// +/// \returns the equivalent attribute in the "to" context, or NULL if an +/// error occurred. a.sidorin wrote: > nullptr I tried to stay consistent with the other

[PATCH] D46115: [ASTImporter] properly import SrcLoc of Attr

r.stahl updated this revision to Diff 145660. r.stahl added a comment. Didn't see that overload, thanks! I need someone to commit this for me. https://reviews.llvm.org/D46115 Files: include/clang/AST/ASTImporter.h lib/AST/ASTImporter.cpp test/Import/attr/Inputs/S.cpp

[PATCH] D46115: [ASTImporter] properly import SrcLoc of Attr

r.stahl updated this revision to Diff 145486. r.stahl edited the summary of this revision. r.stahl added a comment. full patch with test https://reviews.llvm.org/D46115 Files: include/clang/AST/ASTImporter.h lib/AST/ASTImporter.cpp test/Import/attr/Inputs/S.cpp

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

r.stahl added a comment. Tests and doc fixes pending. First I'm interested in your thoughts to this change. It allows to bind more symbolic values to constants if they have been defined and initialized in another TU: use.c: extern int * const p; extern struct S * const s; def.c: int

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

r.stahl updated this revision to Diff 146789. r.stahl marked 3 inline comments as done. r.stahl edited the summary of this revision. r.stahl added a comment. Herald added subscribers: whisperity, mgorny. I looked through the original patches and found quite a few more occurrences of "function

[PATCH] D46823: [analyzer] const init: handle non-explicit cases more accurately

r.stahl updated this revision to Diff 146809. r.stahl marked 2 inline comments as done. r.stahl added a comment. updated test https://reviews.llvm.org/D46823 Files: lib/StaticAnalyzer/Core/RegionStore.cpp test/Analysis/initialization.c Index: test/Analysis/initialization.c

[PATCH] D46823: [analyzer] const init: handle non-explicit cases more accurately

r.stahl added inline comments. Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1650 + +// If there is a list, but no init, it must be zero. +if (i >= InitList->getNumInits()) NoQ wrote: > NoQ wrote: > > Would this work correctly if

[PATCH] D46940: [ASTImporter] make sure that ACtx::getParents still works

r.stahl updated this revision to Diff 147268. r.stahl marked 2 inline comments as done. r.stahl added a comment. addressed review comments https://reviews.llvm.org/D46940 Files: include/clang/AST/ASTContext.h lib/AST/ASTImporter.cpp unittests/AST/ASTImporterTest.cpp Index:

[PATCH] D46940: [ASTImporter] make sure that ACtx::getParents still works

r.stahl added inline comments. Comment at: lib/AST/ASTImporter.cpp:6755 + auto ToE = cast_or_null(Import(cast(FromE))); + if (ToE) +ToContext.invalidateParents(); a.sidorin wrote: > We already do the invalidation in Import(Stmt), so it looks redundant

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

r.stahl added inline comments. Comment at: include/clang/CrossTU/CrossTranslationUnit.h:114 + llvm::Expected + getCrossTUDefinition(const VarDecl *VD, StringRef CrossTUDir, + StringRef IndexName); xazax.hun wrote: > I wonder if we need

[PATCH] D46940: [ASTImporter] make sure that ACtx::getParents still works

r.stahl created this revision. r.stahl added reviewers: a.sidorin, klimek. Herald added subscribers: cfe-commits, martong. If an AST node is imported after a call to getParents in the ToCtx, it was no longer possible to retrieve its parents since the old results were cached. Valid types for

[PATCH] D46940: [ASTImporter] make sure that ACtx::getParents still works

r.stahl added inline comments. Comment at: lib/AST/ASTImporter.cpp:6776 + // been invalidated to avoid repeatedly calling this. + ToContext.invalidateParents(); + martong wrote: > Can an `Expr` has a parent too? If yes, why not invalidate the parents in >

[PATCH] D47698: [ASTImporter] import macro source locations

r.stahl updated this revision to Diff 151138. r.stahl added a comment. remove stray include Repository: rC Clang https://reviews.llvm.org/D47698 Files: lib/AST/ASTImporter.cpp unittests/AST/ASTImporterTest.cpp Index: unittests/AST/ASTImporterTest.cpp

[PATCH] D47698: [ASTImporter] import macro source locations

r.stahl created this revision. r.stahl added reviewers: a.sidorin, klimek, martong. Herald added subscribers: cfe-commits, rnkovacs. Implement full import of macro expansion info with spelling and expansion locations. Repository: rC Clang https://reviews.llvm.org/D47698 Files:

[PATCH] D47698: [ASTImporter] import macro source locations

r.stahl added a comment. The split-token case should be covered by this, because it takes the correct TokenLen and the isTokenRange flag. Other than that the split-token ExpansionInfo is indistinguishable. What about loaded source locations? Do they need to be handled specifically or does

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

r.stahl updated this revision to Diff 149959. r.stahl added a comment. - add missing AnalysisConsumer diff - only collect const qualified vardecls in the extdef index and adjust test https://reviews.llvm.org/D46421 Files: include/clang/Basic/DiagnosticCrossTUKinds.td

[PATCH] D46823: [analyzer] const init: handle non-explicit cases more accurately

This revision was automatically updated to reflect the committed changes. Closed by commit rC333417: [analyzer] const init: handle non-explicit cases more accurately (authored by r.stahl, committed by ). Repository: rC Clang https://reviews.llvm.org/D46823 Files:

[PATCH] D47450: [ASTImporter] Use InjectedClassNameType at import of templated record.

r.stahl accepted this revision. r.stahl added a comment. LGTM! I'm not really too confident to approve changes yet, but with a second opinion it should be fine. Comment at: lib/AST/ASTImporter.cpp:2139 + CXXRecordDecl *Injected = nullptr; + for (NamedDecl

[PATCH] D46823: [analyzer] const init: handle non-explicit cases more accurately

r.stahl updated this revision to Diff 148809. r.stahl added a comment. Added FIXME tests. I know my example didn't exercise your scenario, but it was the only one I could think of where returning zero would have been straight up wrong. Note that I default initialized `a` to 3 there, so `sarr`

[PATCH] D47450: [ASTImporter] Use InjectedClassNameType at import of templated record.

r.stahl added inline comments. Comment at: lib/AST/ASTImporter.cpp:2139 + CXXRecordDecl *Injected = nullptr; + for (NamedDecl *Found : D2CXX->noload_lookup(Name)) { +auto *Record = dyn_cast(Found); balazske wrote: > r.stahl wrote: >

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

r.stahl added a comment. In https://reviews.llvm.org/D46421#1119098, @xazax.hun wrote: > Sorry for the limited activity. Unfortunately, I have very little time > reviewing patches lately. Thanks for getting around to it! > I think we need to answer the following questions: > > - Does this

[PATCH] D46940: [ASTImporter] make sure that ACtx::getParents still works

r.stahl added a comment. @rsmith do you have a chance to take a look or assign someone else? https://reviews.llvm.org/D46940 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47698: [ASTImporter] import macro source locations

r.stahl updated this revision to Diff 152633. r.stahl marked 5 inline comments as done. r.stahl added a comment. improved code quality; added nested macro test. it "works", but is disabled because it revealed another bug: the function end location is not imported. will send a patch

[PATCH] D47698: [ASTImporter] import macro source locations

r.stahl marked an inline comment as done. r.stahl added inline comments. Comment at: lib/AST/ASTImporter.cpp:7058 +const SrcMgr::ExpansionInfo = FromSLoc.getExpansion(); +SourceLocation ToSpLoc = Import(FromEx.getSpellingLoc()); +SourceLocation ToExLocS =

[PATCH] D47698: [ASTImporter] import macro source locations

r.stahl added a comment. In https://reviews.llvm.org/D47698#1140629, @thakis wrote: > This code is live when reading pchs, correct? Does this have any measurable > perf impact on deserializing pchs for, say, Cocoa.h or Windows.h? I don't know for sure, but it should be used - yes. I have not

[PATCH] D26054: Use `getFileLoc()` instead of `getSpellingLoc()` in the ASTImporter

r.stahl added a comment. Herald added a subscriber: martong. I encountered an issue caused by this change. In the scenario as shown below the call to getFileLoc causes the startLoc of the BinaryOp to be after the endLoc, because the LHS (`s->a`) is located in the marco argument while the RHS

[PATCH] D26054: Use `getFileLoc()` instead of `getSpellingLoc()` in the ASTImporter

r.stahl added a comment. It puts everything at the start of the marco expansion. `-FunctionDecl 0x12f5218 prev 0x12f4fa0 line:12:5 used foo 'int ()' `-CompoundStmt 0x12f5600 |-DeclStmt 0x12f5508 | `-VarDecl

[PATCH] D46115: [ASTImporter] properly import SrcLoc of Attr

r.stahl added a comment. Maybe this is a user error of CrossTU, but it seemed to import a FuncDecl with attributes, causing the imported FuncDecl to have all those attributes twice. That's why I thought merging would maybe make sense. However I did not encounter any issue with the duplicate

[PATCH] D45774: [analyzer] cover more cases where a Loc can be bound to constants

r.stahl updated this revision to Diff 145156. r.stahl marked 3 inline comments as done. r.stahl added a comment. addressed the comments, thanks! If it looks good, please commit for me. https://reviews.llvm.org/D45774 Files: lib/StaticAnalyzer/Core/RegionStore.cpp

[PATCH] D45774: [analyzer] cover more cases where a Loc can be bound to constants

r.stahl added inline comments. Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1723 +if (const InitListExpr *InitList = dyn_cast(Init)) { + if (const Expr *FieldInit = InitList->getInit(FD->getFieldIndex())) { +if (Optional V =

[PATCH] D47698: [ASTImporter] import macro source locations

r.stahl updated this revision to Diff 152436. r.stahl marked 5 inline comments as done. r.stahl added a comment. addressed review comment, but there is nothing like FullSourceRange to improve everything Repository: rC Clang https://reviews.llvm.org/D47698 Files: lib/AST/ASTImporter.cpp

[PATCH] D48474: [analyzer][ctu] fix unsortable diagnostics

r.stahl created this revision. r.stahl added reviewers: xazax.hun, NoQ, dcoughlin. Herald added subscribers: cfe-commits, mikhail.ramalho, a.sidorin, rnkovacs, szepet. Herald added a reviewer: george.karpenkov. In the provided test case the PathDiagnostic compare function was not able to find a

[PATCH] D48474: [analyzer][ctu] fix unsortable diagnostics

r.stahl added a comment. In https://reviews.llvm.org/D30691#879838, @xazax.hun wrote: > In https://reviews.llvm.org/D30691#878830, @r.stahl wrote: > > > For my purposes I replaced the return statement of the > > compareCrossTUSourceLocs function with: > > > > return XL.getFileID() <

[PATCH] D48941: [ASTImporter] import FunctionDecl end locations

r.stahl updated this revision to Diff 154215. r.stahl marked 2 inline comments as done. r.stahl added a comment. Alright, but then I would suggest to pass an invalid loc to the constructors instead to make it more explicit and save a map lookup in the import function. Repository: rC Clang

[PATCH] D46940: [ASTImporter] make sure that ACtx::getParents still works

r.stahl added a comment. @rsmith Yes, this should generally better be handled outside of the ASTContext. That would be out of scope of this patch. Is it fine to proceed with this one for now? For my usage scenario it actually is convenient this way. In my checker I use getParents, but if I

[PATCH] D48941: [ASTImporter] import FunctionDecl end locations

This revision was automatically updated to reflect the committed changes. Closed by commit rC336523: [ASTImporter] import FunctionDecl end locations (authored by r.stahl, committed by ). Changed prior to commit: https://reviews.llvm.org/D48941?vs=154215=154549#toc Repository: rC Clang

[PATCH] D48941: [ASTImporter] import FunctionDecl end locations

r.stahl created this revision. r.stahl added reviewers: martong, a.sidorin, balazske, xazax.hun. Herald added subscribers: cfe-commits, rnkovacs. On constructors that do not take the end source location, it was not imported. Fixes test from https://reviews.llvm.org/D47698 /

[PATCH] D48941: [ASTImporter] import FunctionDecl end locations

r.stahl added a comment. In https://reviews.llvm.org/D47698#1141871, @r.stahl wrote: > improved code quality; added nested macro test. it "works", but is disabled > because it revealed another bug: the function end location is not imported. > will send a patch Related to this. Repository:

[PATCH] D47698: [ASTImporter] import macro source locations

This revision was automatically updated to reflect the committed changes. r.stahl marked an inline comment as done. Closed by commit rC336269: [ASTImporter] import macro source locations (authored by r.stahl, committed by ). Changed prior to commit:

[PATCH] D48474: [analyzer][ctu] fix unsortable diagnostics

This revision was automatically updated to reflect the committed changes. Closed by commit rC336275: [analyzer][ctu] fix unsortable diagnostics (authored by r.stahl, committed by ). Changed prior to commit: https://reviews.llvm.org/D48474?vs=152433=154107#toc Repository: rC Clang

[PATCH] D45564: [analyzer] Fix null deref in AnyFunctionCall::getRuntimeDefinition

r.stahl added a comment. I encountered this with a construct like this: struct S { void (*fp)(); }; int main() { struct S s; s.fp(); } Repository: rC Clang https://reviews.llvm.org/D45564 ___ cfe-commits mailing

[PATCH] D45564: [analyzer] Fix null deref in AnyFunctionCall::getRuntimeDefinition

r.stahl created this revision. r.stahl added reviewers: xazax.hun, dcoughlin, a.sidorin, george.karpenkov. Herald added subscribers: cfe-commits, rnkovacs, szepet. In https://reviews.llvm.org/D30691 code was added to getRuntimeDefinition that does not handle the case when FD==nullptr.

[PATCH] D45564: [analyzer] Fix null deref in AnyFunctionCall::getRuntimeDefinition

r.stahl updated this revision to Diff 142381. r.stahl edited the summary of this revision. r.stahl added a comment. addressed review comments. I created a new test because certain checkers would cause early exits in the engine (because of undefined func ptr) and not cause the crash. Since I

[PATCH] D45774: [analyzer] cover more cases where a Loc can be bound to constants

r.stahl created this revision. r.stahl added reviewers: NoQ, dcoughlin, xazax.hun. Herald added subscribers: cfe-commits, a.sidorin, rnkovacs, szepet, mehdi_amini. Herald added a reviewer: george.karpenkov. - SValBuilder::getConstantVal stopped processing an initialization if there are

[PATCH] D45774: [analyzer] cover more cases where a Loc can be bound to constants

r.stahl added a comment. Not sure how to proceed about these: - CXXDynamicCastExpr: I don't think evalCast would handle this correctly, does it? - ObjCBridgedCastExpr: I don't know ObjectiveC - CastKinds for floating point: Floats cannot yet be handled by the analyzer, right? Repository:

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

r.stahl added inline comments. Herald added a subscriber: martong. Comment at: cfe/trunk/unittests/AST/ASTImporterTest.cpp:100 + // This traverses the AST to catch certain bugs like poorly or not + // implemented subtrees. a.sidorin wrote: > I just saw this

[PATCH] D46115: [ASTImporter] properly import SrcLoc of Attr

r.stahl created this revision. r.stahl added reviewers: NoQ, dcoughlin, xazax.hun, george.karpenkov. Herald added subscribers: cfe-commits, martong, a.sidorin, rnkovacs. The ASTImporter was failing to import the SourceLocation field of Attrs. Repository: rC Clang

[PATCH] D46115: [ASTImporter] properly import SrcLoc of Attr

r.stahl added a comment. This is unfinished. Posting to make you aware of the issue. There are other occurances of imported attrs without importing the srcloc in: - VisitIndirectFieldDecl - VisitAttributedStmt So I was thinking this would suggest to add a public API `ASTImporter::Import(Attr

[PATCH] D47698: [ASTImporter] import macro source locations

r.stahl added inline comments. Comment at: lib/AST/ASTImporter.cpp:7058 +const SrcMgr::ExpansionInfo = FromSLoc.getExpansion(); +SourceLocation ToSpLoc = Import(FromEx.getSpellingLoc()); +SourceLocation ToExLocS = Import(FromEx.getExpansionLocStart());

[PATCH] D55701: [analyzer] Pass the correct loc Expr from VisitIncDecOp to evalStore

r.stahl created this revision. r.stahl added reviewers: NoQ, dcoughlin, george.karpenkov. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. The LocationE parameter of evalStore is documented as "The location

[PATCH] D55701: [analyzer] Pass the correct loc Expr from VisitIncDecOp to evalStore

r.stahl added a comment. cfe-dev thread with short discussion: http://lists.llvm.org/pipermail/cfe-dev/2018-April/057562.html Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55701/new/ https://reviews.llvm.org/D55701

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

r.stahl added a comment. Herald added subscribers: dkrupp, donat.nagy, Szelethus, mikhail.ramalho, baloghadamsoftware. Please let me know if there is anything else that should be addressed. Note that even though this diff is quite large, most is just renaming things to stay consistent.

[PATCH] D56441: [analyzer][CrossTU][NFC] Generalize to external definitions instead of external functions

r.stahl updated this revision to Diff 181073. r.stahl added a comment. addressed review comments Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56441/new/ https://reviews.llvm.org/D56441 Files: include/clang/Basic/DiagnosticCrossTUKinds.td

[PATCH] D56441: [analyzer][CrossTU][NFC] Generalize to external definitions instead of external functions

This revision was automatically updated to reflect the committed changes. Closed by commit rL350852: [analyzer][CrossTU][NFC] Generalize to external definitions instead of external… (authored by r.stahl, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D55701: [analyzer] Pass the correct loc Expr from VisitIncDecOp to evalStore

This revision was automatically updated to reflect the committed changes. Closed by commit rC350528: [analyzer] Pass the correct loc Expr from VisitIncDecOp to evalStore (authored by r.stahl, committed by ). Changed prior to commit: https://reviews.llvm.org/D55701?vs=180487=180489#toc

[PATCH] D55701: [analyzer] Pass the correct loc Expr from VisitIncDecOp to evalStore

r.stahl added a comment. I tried adding isGLValue to evalStore and the test suite didn't complain. For evalLoad (on BoundEx) it failed in pretty much every test. Should the evalStore assert also go into trunk? Since the analyzer behavior itself remains unchanged, I don't believe any other

[PATCH] D55701: [analyzer] Pass the correct loc Expr from VisitIncDecOp to evalStore

r.stahl updated this revision to Diff 180487. r.stahl added a comment. rebased Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55701/new/ https://reviews.llvm.org/D55701 Files: lib/StaticAnalyzer/Core/ExprEngineC.cpp

[PATCH] D56441: [analyzer][CrossTU][NFC] Generalize to external definitions instead of external functions

r.stahl created this revision. r.stahl added reviewers: NoQ, xazax.hun, martong, a.sidorin. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, rnkovacs, szepet, baloghadamsoftware, whisperity, mgorny. Herald added a reviewer: george.karpenkov. Herald added a

[PATCH] D56441: [analyzer][CrossTU][NFC] Generalize to external definitions instead of external functions

r.stahl added a comment. Thanks for the quick response! Will wait a couple days to see if someone else has something to add. But I think something is wrong here... When trying to "arc diff" I got the following error: Exception Command failed with error #1! COMMAND svn propget

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

r.stahl updated this revision to Diff 181263. r.stahl marked 12 inline comments as done. r.stahl added a comment. Strip name changes (see D56441 ); addressed review comments In my old version I seemed to get away with the tests, but they failed after rebasing.

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

r.stahl added inline comments. Comment at: lib/CrossTU/CrossTranslationUnit.cpp:120 +} +template static bool hasDefinition(const T *D) { + const T *Unused; martong wrote: > `hasDefinitionOrInit` ? I simply made it `hasBodyOrInit` now to be as clear as possible

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

r.stahl added a comment. Okay so I would suggest to go ahead and commit this. Rebased it succeeds without modification. Still leaves the open problems with the redecls. Should I add the failing test from https://reviews.llvm.org/D46421#1375147 in the same commit marked as expected to fail? Or

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

r.stahl marked 3 inline comments as done. r.stahl added a comment. In D46421#1456171 , @xazax.hun wrote: > My last set of comments are also unresolved. Most of them are minor nits, but > I would love to get rid of the code duplication between

[PATCH] D46940: [ASTImporter] make sure that ACtx::getParents still works

r.stahl abandoned this revision. r.stahl added a comment. Herald added a reviewer: shafik. ASTContext::getParents should not be used this way. This use-case is solved by function-scoped ParentMaps or AnalysisDeclContext::getParentMap. Discussion:

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

r.stahl added inline comments. Comment at: lib/CrossTU/CrossTranslationUnit.cpp:255 + // in the other unit it has none. + if (LangTo.CPlusPlus11 != LangFrom.CPlusPlus11 || + LangTo.CPlusPlus14 != LangFrom.CPlusPlus14 || This is likely to become a bug in

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

r.stahl added a comment. Thanks for the comments! All good points. Nice idea with the constructor, but that can probably happen in a follow up patch. In D46421#1380619 , @xazax.hun wrote: > I know you probably tired of me making you split all the

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

r.stahl added a comment. In D46421#1374807 , @NoQ wrote: > At the same time, i don't have any test cases for the actual change in > behavior that such canonicalization causes. If the test case that you had in > mind is indeed demonstrating this problem,

[PATCH] D61001: [clang-format][tests] Explicitly specify style in some tests

This revision was automatically updated to reflect the committed changes. r.stahl marked an inline comment as done. Closed by commit rGf625a8a250b3: [clang-format][tests] Explicitly specify style in some tests (authored by r.stahl). Changed prior to commit:

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

This revision was automatically updated to reflect the committed changes. Closed by commit rC358968: [analyzer][CrossTU] Extend CTU to VarDecls with initializer (authored by r.stahl, committed by ). Changed prior to commit: https://reviews.llvm.org/D46421?vs=196075=196207#toc Repository: rC

[PATCH] D61002: <[analyzer][CrossTU][NFC] Fix sanitizer test failure

r.stahl added a comment. @xazax.hun Yes, I planned to just commit. Set you as Subscriber not Reviewer in Arc. Was just a bit confused why it fails at first :) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61002/new/ https://reviews.llvm.org/D61002

[PATCH] D61001: [clang-format][tests] Explicitly specify style in some tests

r.stahl created this revision. r.stahl added reviewers: djasper, klimek. Herald added a project: clang. Herald added a subscriber: cfe-commits. This fixes broken tests when doing an out-of-source build that picks up a random .clang-format on the file system due to the default "file" style.

[PATCH] D61002: <[analyzer][CrossTU][NFC] Fix sanitizer test failure

r.stahl created this revision. Herald added subscribers: cfe-commits, Charusso, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware. Herald added a project: clang. Missing break/fallthrough Repository: rC Clang https://reviews.llvm.org/D61002 Files:

[PATCH] D61002: <[analyzer][CrossTU][NFC] Fix sanitizer test failure

r.stahl abandoned this revision. r.stahl added a comment. Was already done: https://github.com/llvm-mirror/clang/commit/bacdda22396c39181aa0e641182e01a0b3cf43ea Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61002/new/ https://reviews.llvm.org/D61002

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

r.stahl updated this revision to Diff 194285. r.stahl marked 4 inline comments as done. r.stahl added a comment. - addressed review comments - created cross_tu::containsConst - fixed bug that unions were not exported - added TODO test for constructor case Repository: rC Clang CHANGES SINCE

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

r.stahl added inline comments. Comment at: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:357 + return true; +if (!RTy->hasConstFields()) + return true; xazax.hun wrote: > I wonder what would happen with types that has const fields

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

r.stahl updated this revision to Diff 196075. r.stahl added a comment. @xazax.hun good point and that actually fixes a bug since that branch should also do the deep check. Added that to the tests. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D46421/new/

[PATCH] D61001: [clang-format][tests] Explicitly specify style in some tests

r.stahl added reviewers: MyDeveloperDay, krasimir. r.stahl added a comment. This should be trivial enough to just commit, but I'd just be more comfortable if someone looked at it before, because this is my first commit in this area of clang. Repository: rC Clang CHANGES SINCE LAST ACTION

[PATCH] D61001: [clang-format][tests] Explicitly specify style in some tests

r.stahl marked 2 inline comments as done. r.stahl added inline comments. Comment at: test/Format/language-detection.cpp:2 // RUN: grep -Ev "// *[A-Z0-9_]+:" %s \ -// RUN: | clang-format -style=llvm -assume-filename=foo.js \ // RUN: | FileCheck -strict-whitespace

[PATCH] D124621: [Analyzer] Fix assumptions about const field with member-initializer

r.stahl accepted this revision. r.stahl added a comment. I can confirm the issue with my patch, so this piece of code needs to be removed. As long as the following test still succeeds, this looks good to me. Back then, the analyzer was not able to cover that case without that addition.