Page MenuHomePhabricator

r.stahl (Rafael Stahl)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 23 2017, 7:34 AM (86 w, 4 d)

Recent Activity

Tue, Apr 9

r.stahl added inline comments to D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer.
Tue, Apr 9, 4:17 AM · Restricted Project
r.stahl updated the diff for D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer.
  • addressed review comments
  • created cross_tu::containsConst
  • fixed bug that unions were not exported
  • added TODO test for constructor case
Tue, Apr 9, 4:16 AM · Restricted Project

Mon, Apr 8

r.stahl abandoned D46940: [ASTImporter] make sure that ACtx::getParents still works.

ASTContext::getParents should not be used this way. This use-case is solved by function-scoped ParentMaps or AnalysisDeclContext::getParentMap. Discussion: http://lists.llvm.org/pipermail/cfe-dev/2019-April/061944.html

Mon, Apr 8, 1:58 AM

Fri, Apr 5

r.stahl added a comment to D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer.

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 ClangExtDefMapGen and the Clang Static Analyzer regarding when do we consider a variable worth to import. Otherwise the patch looks good to me.

Fri, Apr 5, 9:14 AM · Restricted Project
r.stahl added a comment to D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer.

Okay so I would suggest to go ahead and commit this. Rebased it succeeds without modification.

Fri, Apr 5, 6:28 AM · Restricted Project

Feb 8 2019

r.stahl added inline comments to D57906: [CTU] Do not allow different CPP dialects in CTU.
Feb 8 2019, 7:33 AM · Restricted Project

Feb 1 2019

r.stahl added a comment to D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer.

Thanks for the comments! All good points. Nice idea with the constructor, but that can probably happen in a follow up patch.

Feb 1 2019, 8:30 AM · Restricted Project

Jan 29 2019

r.stahl added a comment to D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer.
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, i'd love to have it. If it turns out that your test case doesn't allow us to demonstrate the problem without CTU, then probably it has something to do with ASTImporter accidentally canonicalizing the the declaration in DeclRefExpr more rarely than the vanilla AST.

Jan 29 2019, 2:13 AM · Restricted Project

Jan 11 2019

r.stahl added inline comments to D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer.
Jan 11 2019, 6:57 AM · Restricted Project
r.stahl updated the diff for D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer.

Strip name changes (see D56441); addressed review comments

Jan 11 2019, 6:57 AM · Restricted Project

Jan 10 2019

r.stahl committed rC350852: [analyzer][CrossTU][NFC] Generalize to external definitions instead of external….
[analyzer][CrossTU][NFC] Generalize to external definitions instead of external…
Jan 10 2019, 9:48 AM
r.stahl committed rL350852: [analyzer][CrossTU][NFC] Generalize to external definitions instead of external….
[analyzer][CrossTU][NFC] Generalize to external definitions instead of external…
Jan 10 2019, 9:48 AM
r.stahl closed D56441: [analyzer][CrossTU][NFC] Generalize to external definitions instead of external functions.
Jan 10 2019, 9:47 AM
r.stahl updated the diff for D56441: [analyzer][CrossTU][NFC] Generalize to external definitions instead of external functions.

addressed review comments

Jan 10 2019, 8:37 AM

Jan 8 2019

r.stahl added a comment to D56441: [analyzer][CrossTU][NFC] Generalize to external definitions instead of external functions.

Thanks for the quick response! Will wait a couple days to see if someone else has something to add.

Jan 8 2019, 7:38 AM
r.stahl created D56441: [analyzer][CrossTU][NFC] Generalize to external definitions instead of external functions.
Jan 8 2019, 7:05 AM

Jan 7 2019

r.stahl added a comment to D55701: [analyzer] Pass the correct loc Expr from VisitIncDecOp to evalStore.

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?

Jan 7 2019, 7:11 AM
r.stahl committed rL350528: [analyzer] Pass the correct loc Expr from VisitIncDecOp to evalStore.
[analyzer] Pass the correct loc Expr from VisitIncDecOp to evalStore
Jan 7 2019, 7:10 AM
r.stahl committed rC350528: [analyzer] Pass the correct loc Expr from VisitIncDecOp to evalStore.
[analyzer] Pass the correct loc Expr from VisitIncDecOp to evalStore
Jan 7 2019, 7:10 AM
r.stahl closed D55701: [analyzer] Pass the correct loc Expr from VisitIncDecOp to evalStore.
Jan 7 2019, 7:10 AM
r.stahl updated the diff for D55701: [analyzer] Pass the correct loc Expr from VisitIncDecOp to evalStore.

rebased

Jan 7 2019, 7:08 AM

Dec 14 2018

r.stahl added a comment to D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer.

Please let me know if there is anything else that should be addressed.

Dec 14 2018, 4:00 AM · Restricted Project
r.stahl added a comment to D55701: [analyzer] Pass the correct loc Expr from VisitIncDecOp to evalStore.

cfe-dev thread with short discussion: http://lists.llvm.org/pipermail/cfe-dev/2018-April/057562.html

Dec 14 2018, 3:44 AM
r.stahl created D55701: [analyzer] Pass the correct loc Expr from VisitIncDecOp to evalStore.
Dec 14 2018, 3:42 AM

Jul 11 2018

r.stahl added a comment to D46940: [ASTImporter] make sure that ACtx::getParents still works.

@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?

Jul 11 2018, 4:08 AM

Jul 9 2018

r.stahl committed rC336527: [ASTImporter] fix test failure corrected by fixed func end locs.
[ASTImporter] fix test failure corrected by fixed func end locs
Jul 9 2018, 2:08 AM
r.stahl committed rL336527: [ASTImporter] fix test failure corrected by fixed func end locs.
[ASTImporter] fix test failure corrected by fixed func end locs
Jul 9 2018, 2:07 AM
r.stahl committed rL336523: [ASTImporter] import FunctionDecl end locations.
[ASTImporter] import FunctionDecl end locations
Jul 9 2018, 1:45 AM
r.stahl committed rC336523: [ASTImporter] import FunctionDecl end locations.
[ASTImporter] import FunctionDecl end locations
Jul 9 2018, 1:45 AM
r.stahl closed D48941: [ASTImporter] import FunctionDecl end locations.
Jul 9 2018, 1:45 AM

Jul 5 2018

r.stahl updated the diff for D48941: [ASTImporter] import FunctionDecl end locations.

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.

Jul 5 2018, 6:03 AM

Jul 4 2018

r.stahl added a comment to D48941: [ASTImporter] import FunctionDecl end locations.

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

Jul 4 2018, 8:17 AM
r.stahl created D48941: [ASTImporter] import FunctionDecl end locations.
Jul 4 2018, 8:14 AM
r.stahl committed rL336275: [analyzer][ctu] fix unsortable diagnostics.
[analyzer][ctu] fix unsortable diagnostics
Jul 4 2018, 7:18 AM
r.stahl committed rC336275: [analyzer][ctu] fix unsortable diagnostics.
[analyzer][ctu] fix unsortable diagnostics
Jul 4 2018, 7:17 AM
r.stahl closed D48474: [analyzer][ctu] fix unsortable diagnostics.
Jul 4 2018, 7:17 AM
r.stahl committed rL336269: [ASTImporter] import macro source locations.
[ASTImporter] import macro source locations
Jul 4 2018, 6:39 AM
r.stahl committed rC336269: [ASTImporter] import macro source locations.
[ASTImporter] import macro source locations
Jul 4 2018, 6:38 AM
r.stahl closed D47698: [ASTImporter] import macro source locations.
Jul 4 2018, 6:38 AM

Jun 26 2018

r.stahl added a comment to D46940: [ASTImporter] make sure that ACtx::getParents still works.

@rsmith do you have a chance to take a look or assign someone else?

Jun 26 2018, 6:50 AM

Jun 25 2018

r.stahl added inline comments to D47698: [ASTImporter] import macro source locations.
Jun 25 2018, 1:20 AM
r.stahl updated the diff for D47698: [ASTImporter] import macro source locations.

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

Jun 25 2018, 1:18 AM

Jun 22 2018

r.stahl added a comment to D47698: [ASTImporter] import macro source locations.

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?

Jun 22 2018, 7:50 AM
r.stahl updated the diff for D47698: [ASTImporter] import macro source locations.

addressed review comment, but there is nothing like FullSourceRange to improve everything

Jun 22 2018, 1:43 AM
r.stahl added a comment to D48474: [analyzer][ctu] fix unsortable diagnostics.

For my purposes I replaced the return statement of the compareCrossTUSourceLocs function with:

return XL.getFileID() < YL.getFileID();

A more correct fix would create only one unique diagnostic for both cases.

Thank you for the report, I could reproduce this after modifying the null dereference checker. My fear of using file IDs is that I don't know whether they are stable. So in subsequent runs, different paths might be chosen by the analyzer and this could be confusing to the user. I will think about a workaround that both stable and solves this assertion.

I see two possible ways to do the proper fix. One is to check explicitly for this case when the same function appears in multiple translation units. A better approach would be to have the ASTImporter handle this case. I think the proper fix is better addressed in a separate patch.

Jun 22 2018, 1:24 AM
r.stahl created D48474: [analyzer][ctu] fix unsortable diagnostics.
Jun 22 2018, 1:19 AM

Jun 20 2018

r.stahl added inline comments to D47698: [ASTImporter] import macro source locations.
Jun 20 2018, 4:24 AM

Jun 13 2018

r.stahl updated the diff for D47698: [ASTImporter] import macro source locations.

remove stray include

Jun 13 2018, 6:02 AM

Jun 5 2018

r.stahl updated the diff for D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer.
  • add missing AnalysisConsumer diff
  • only collect const qualified vardecls in the extdef index and adjust test
Jun 5 2018, 5:47 AM · Restricted Project

Jun 4 2018

r.stahl added a comment to D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer.

Sorry for the limited activity. Unfortunately, I have very little time reviewing patches lately.

Jun 4 2018, 9:06 AM · Restricted Project
r.stahl added a comment to D47698: [ASTImporter] import macro source locations.

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.

Jun 4 2018, 1:59 AM
r.stahl created D47698: [ASTImporter] import macro source locations.
Jun 4 2018, 1:52 AM

May 29 2018

r.stahl committed rL333417: [analyzer] const init: handle non-explicit cases more accurately.
[analyzer] const init: handle non-explicit cases more accurately
May 29 2018, 7:18 AM
r.stahl committed rC333417: [analyzer] const init: handle non-explicit cases more accurately.
[analyzer] const init: handle non-explicit cases more accurately
May 29 2018, 7:18 AM
r.stahl closed D46823: [analyzer] const init: handle non-explicit cases more accurately.
May 29 2018, 7:18 AM
r.stahl committed rC333396: Testing commit access with whitespace change..
Testing commit access with whitespace change.
May 29 2018, 1:16 AM
r.stahl committed rL333396: Testing commit access with whitespace change..
Testing commit access with whitespace change.
May 29 2018, 1:16 AM

May 28 2018

r.stahl added inline comments to D47450: [ASTImporter] Use InjectedClassNameType at import of templated record..
May 28 2018, 7:23 AM
r.stahl updated the diff for D46823: [analyzer] const init: handle non-explicit cases more accurately.

Added FIXME tests.

May 28 2018, 6:48 AM
r.stahl accepted D47450: [ASTImporter] Use InjectedClassNameType at import of templated record..

I'm not really too confident to approve changes yet, but with a second opinion it should be fine.

May 28 2018, 6:30 AM

May 18 2018

r.stahl added inline comments to D46940: [ASTImporter] make sure that ACtx::getParents still works.
May 18 2018, 8:04 AM

May 17 2018

r.stahl updated the diff for D46940: [ASTImporter] make sure that ACtx::getParents still works.

addressed review comments

May 17 2018, 1:53 AM

May 16 2018

r.stahl added inline comments to D46940: [ASTImporter] make sure that ACtx::getParents still works.
May 16 2018, 7:59 AM
r.stahl created D46940: [ASTImporter] make sure that ACtx::getParents still works.
May 16 2018, 6:09 AM

May 15 2018

r.stahl added inline comments to D46823: [analyzer] const init: handle non-explicit cases more accurately.
May 15 2018, 6:44 AM
r.stahl updated the diff for D46823: [analyzer] const init: handle non-explicit cases more accurately.

updated test

May 15 2018, 6:43 AM
r.stahl added inline comments to D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer.
May 15 2018, 5:37 AM · Restricted Project
r.stahl updated the diff for D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer.

I looked through the original patches and found quite a few more occurrences of "function map" and renamed them - including the tool "clang-func-mapping". There is one comment "by using the 'clang-extdef-mapping' packaged with Xcode (on OS X)". Is this implicit from the install targets that the tool name changed or do I have to inform someone?

May 15 2018, 5:36 AM · Restricted Project

May 14 2018

r.stahl updated the diff for D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer.
  • added tests
  • updated doc and var naming
  • addressed review comment
May 14 2018, 7:45 AM · Restricted Project
r.stahl added a comment to D45774: [analyzer] cover more cases where a Loc can be bound to constants.
In D45774#1088453, @NoQ wrote:

Two questions for the future:

  • Should we skip the initializer binding for local variables (and fields/elements of local variables) entirely? Cause we can load from them anyway. And keeping the Store small might be good for performance.
May 14 2018, 3:13 AM
r.stahl created D46823: [analyzer] const init: handle non-explicit cases more accurately.
May 14 2018, 3:08 AM

May 9 2018

r.stahl created D46633: [analyzer] add range check for InitList lookup.
May 9 2018, 2:12 AM

May 8 2018

r.stahl updated the diff for D46115: [ASTImporter] properly import SrcLoc of Attr.

Didn't see that overload, thanks!

May 8 2018, 3:34 AM
r.stahl added inline comments to D46115: [ASTImporter] properly import SrcLoc of Attr.
May 8 2018, 12:54 AM
r.stahl updated the diff for D46115: [ASTImporter] properly import SrcLoc of Attr.
May 8 2018, 12:53 AM

May 7 2018

r.stahl updated the diff for D46115: [ASTImporter] properly import SrcLoc of Attr.

full patch with test

May 7 2018, 9:41 AM

May 4 2018

r.stahl added a comment to D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer.

Tests and doc fixes pending. First I'm interested in your thoughts to this change.

May 4 2018, 1:06 AM · Restricted Project
r.stahl created D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer.
May 4 2018, 1:05 AM · Restricted Project
r.stahl added inline comments to D45774: [analyzer] cover more cases where a Loc can be bound to constants.
May 4 2018, 12:26 AM
r.stahl updated the diff for D45774: [analyzer] cover more cases where a Loc can be bound to constants.

addressed the comments, thanks!

May 4 2018, 12:25 AM

May 3 2018

r.stahl added a comment to D46115: [ASTImporter] properly import SrcLoc of Attr.

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 attributes. Only the wrong source locations produced odd crashes.

May 3 2018, 9:08 AM
r.stahl added a comment to D26054: Use `getFileLoc()` instead of `getSpellingLoc()` in the ASTImporter.

It puts everything at the start of the marco expansion.

May 3 2018, 7:40 AM

May 2 2018

r.stahl added a comment to D26054: Use `getFileLoc()` instead of `getSpellingLoc()` in the ASTImporter.

I encountered an issue caused by this change.

May 2 2018, 9:28 AM

Apr 26 2018

r.stahl added a comment to D46115: [ASTImporter] properly import SrcLoc of Attr.

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
Apr 26 2018, 5:16 AM
r.stahl created D46115: [ASTImporter] properly import SrcLoc of Attr.
Apr 26 2018, 5:15 AM

Apr 18 2018

r.stahl added a comment to D45774: [analyzer] cover more cases where a Loc can be bound to constants.

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?
Apr 18 2018, 8:47 AM
r.stahl created D45774: [analyzer] cover more cases where a Loc can be bound to constants.
Apr 18 2018, 8:45 AM

Apr 13 2018

r.stahl updated the diff for D45564: [analyzer] Fix null deref in AnyFunctionCall::getRuntimeDefinition.

addressed review comments.

Apr 13 2018, 5:17 AM

Apr 12 2018

r.stahl added a comment to D45564: [analyzer] Fix null deref in AnyFunctionCall::getRuntimeDefinition.

I encountered this with a construct like this:

Apr 12 2018, 5:14 AM
r.stahl created D45564: [analyzer] Fix null deref in AnyFunctionCall::getRuntimeDefinition.
Apr 12 2018, 5:14 AM

Mar 27 2018

r.stahl added inline comments to D38943: [ASTImporter] import SubStmt of CaseStmt.
Mar 27 2018, 12:31 AM

Oct 26 2017

r.stahl accepted D38842: [CrossTU] Fix handling of Cross Translation Unit directory path.

I'm gonna go ahead and approve this now, because I reported the issue. Note that I'm not a regular contributor, yet!

Oct 26 2017, 1:54 AM

Oct 16 2017

r.stahl added a comment to D38943: [ASTImporter] import SubStmt of CaseStmt.

If all is good, I will need someone to commit this for me please.

Oct 16 2017, 8:47 AM
r.stahl updated the diff for D38943: [ASTImporter] import SubStmt of CaseStmt.

addressed review comment

Oct 16 2017, 8:45 AM
r.stahl added a comment to D38943: [ASTImporter] import SubStmt of CaseStmt.

Thanks for the fast response. See inline comment.

Oct 16 2017, 5:51 AM
r.stahl created D38943: [ASTImporter] import SubStmt of CaseStmt.
Oct 16 2017, 12:49 AM

Oct 9 2017

r.stahl added a comment to D37478: [analyzer] Implement pointer arithmetic on constants.

Since I do not have commit access, it would be nice if someone committed this for me. Thanks!

Oct 9 2017, 11:50 PM
r.stahl updated the diff for D37478: [analyzer] Implement pointer arithmetic on constants.

addressed review comments. updated summary.

Oct 9 2017, 12:18 AM

Oct 6 2017

r.stahl added a comment to D34512: Add preliminary Cross Translation Unit support library.
  • 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
Oct 6 2017, 8:24 AM

Sep 22 2017

r.stahl added a comment to D30691: [analyzer] Support for naive cross translational unit analysis.

In a similar case, static inline functions are an issue.

Sep 22 2017, 8:06 AM