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 (103 w, 4 d)

Recent Activity

Mon, Jul 29

r.stahl committed rG0e074fa0fcbe: doc: Fix Google C++ Style Guide link. (authored by r.stahl).
doc: Fix Google C++ Style Guide link.
Mon, Jul 29, 7:55 AM
r.stahl committed rL367219: doc: Fix Google C++ Style Guide link..
doc: Fix Google C++ Style Guide link.
Mon, Jul 29, 7:39 AM

Jul 12 2019

r.stahl committed rGf625a8a250b3: [clang-format][tests] Explicitly specify style in some tests (authored by r.stahl).
[clang-format][tests] Explicitly specify style in some tests
Jul 12 2019, 8:58 AM
r.stahl committed rL365909: [clang-format][tests] Explicitly specify style in some tests.
[clang-format][tests] Explicitly specify style in some tests
Jul 12 2019, 8:58 AM
r.stahl closed D61001: [clang-format][tests] Explicitly specify style in some tests.
Jul 12 2019, 8:58 AM · Restricted Project

Jul 10 2019

r.stahl added inline comments to D61001: [clang-format][tests] Explicitly specify style in some tests.
Jul 10 2019, 1:15 AM · Restricted Project
r.stahl added reviewers for D61001: [clang-format][tests] Explicitly specify style in some tests: MyDeveloperDay, krasimir.

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.

Jul 10 2019, 1:05 AM · Restricted Project

Apr 23 2019

r.stahl added a comment to D61002: <<Replace this line with your revision title> [analyzer][CrossTU][NFC] Fix sanitizer test failure.

@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 :)

Apr 23 2019, 5:07 AM · Restricted Project
r.stahl abandoned D61002: <<Replace this line with your revision title> [analyzer][CrossTU][NFC] Fix sanitizer test failure.

Was already done: https://github.com/llvm-mirror/clang/commit/bacdda22396c39181aa0e641182e01a0b3cf43ea

Apr 23 2019, 5:06 AM · Restricted Project
r.stahl created D61002: <<Replace this line with your revision title> [analyzer][CrossTU][NFC] Fix sanitizer test failure.
Apr 23 2019, 5:02 AM · Restricted Project
r.stahl created D61001: [clang-format][tests] Explicitly specify style in some tests.
Apr 23 2019, 4:45 AM · Restricted Project
r.stahl committed rG850361f6c1db: [analyzer][CrossTU] Extend CTU to VarDecls with initializer (authored by r.stahl).
[analyzer][CrossTU] Extend CTU to VarDecls with initializer
Apr 23 2019, 4:04 AM
r.stahl committed rL358968: [analyzer][CrossTU] Extend CTU to VarDecls with initializer.
[analyzer][CrossTU] Extend CTU to VarDecls with initializer
Apr 23 2019, 4:04 AM
r.stahl committed rC358968: [analyzer][CrossTU] Extend CTU to VarDecls with initializer.
[analyzer][CrossTU] Extend CTU to VarDecls with initializer
Apr 23 2019, 4:04 AM
r.stahl closed D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer.
Apr 23 2019, 4:04 AM · Restricted Project

Apr 22 2019

r.stahl updated the diff for D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer.

@xazax.hun good point and that actually fixes a bug since that branch should also do the deep check. Added that to the tests.

Apr 22 2019, 8:59 AM · Restricted Project

Apr 9 2019

r.stahl added inline comments to D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer.
Apr 9 2019, 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
Apr 9 2019, 4:16 AM · Restricted Project

Apr 8 2019

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

Apr 8 2019, 1:58 AM

Apr 5 2019

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.

Apr 5 2019, 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.

Apr 5 2019, 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