Page MenuHomePhabricator

steveire (Stephen Kelly)
User

Projects

User does not belong to any projects.

User Details

User Since
Sep 5 2013, 6:50 AM (343 w, 4 d)

Recent Activity

Jan 28 2020

steveire updated the diff for D72531: Set traversal explicitly where needed in tests.

Rebase

Jan 28 2020, 4:59 AM · Restricted Project
steveire updated the diff for D72530: Set traversal explicitly where needed in clang-tidy.

Rebase

Jan 28 2020, 3:00 AM · Restricted Project

Jan 26 2020

steveire abandoned D73029: Extend ExprTraversal class with acending traversal.
Jan 26 2020, 11:18 AM · Restricted Project
steveire abandoned D73028: Extract ExprTraversal class from Expr.
Jan 26 2020, 11:18 AM · Restricted Project

Jan 25 2020

steveire added a comment to D73388: NFC: Implement AST node skipping in ParentMapContext.

This seems reasonable to me.

If you want to unify something about this and the Ignore* functions in Expr, maybe we could add a "classify" mechanism, so you could ask "what kind of node is this?" and get back an enumerated value that indicates whether it's semantic / syntactic / whatever. Then we could implement both this and the downward-skipping in terms of that.

Jan 25 2020, 3:18 AM · Restricted Project

Jan 24 2020

steveire retitled D73388: NFC: Implement AST node skipping in ParentMapContext from Implement AST node skipping in ParentMapContext to NFC: Implement AST node skipping in ParentMapContext.
Jan 24 2020, 4:18 PM · Restricted Project
steveire updated the diff for D73388: NFC: Implement AST node skipping in ParentMapContext.

constness

Jan 24 2020, 4:09 PM · Restricted Project
steveire added a comment to D73028: Extract ExprTraversal class from Expr.

The follow-up is here: https://reviews.llvm.org/D73029 .

I have the need to change the ascending traversal implemented in ASTContext.cpp with a map (populated while descending) to make it skip nodes too during parent traversal.

I didn't want to have descending traversal in Expr.cpp and ascending traversal in ASTContext.cpp as they would be likely to go out of sync, so moving this implementation here allows extending the purpose in the follow-up.

The ascending traversal should not be in the AST library at all. If tooling or static analysis wants this, that's fine, but navigating upwards is not something the AST supports, and we do not want any part of clang proper developing a reliance on having a parent map, so we're working on moving the parent map out of ASTContext (with a plan to eventually move it out of the AST library). See D71313 for related work in this area.

Thanks for the heads-up - I'm trying to reach the conclusion from this information.

Are you saying that

  1. this patch shouldn't be committed, so down-traversal-skipping should remain in Expr.cpp
  2. D73029 should be changed to implement the up-traversal-skipping directly in ParentMapContext.cpp

    Is that the right conclusion?
Jan 24 2020, 4:09 PM · Restricted Project
steveire created D73388: NFC: Implement AST node skipping in ParentMapContext.
Jan 24 2020, 4:09 PM · Restricted Project
steveire added a comment to D71313: [AST] Split parent map traversal logic into ParentMapContext.h.

Should this new class have some tests?

Jan 24 2020, 3:59 PM · Restricted Project, Restricted Project
steveire added a comment to D73028: Extract ExprTraversal class from Expr.

The follow-up is here: https://reviews.llvm.org/D73029 .

I have the need to change the ascending traversal implemented in ASTContext.cpp with a map (populated while descending) to make it skip nodes too during parent traversal.

I didn't want to have descending traversal in Expr.cpp and ascending traversal in ASTContext.cpp as they would be likely to go out of sync, so moving this implementation here allows extending the purpose in the follow-up.

The ascending traversal should not be in the AST library at all. If tooling or static analysis wants this, that's fine, but navigating upwards is not something the AST supports, and we do not want any part of clang proper developing a reliance on having a parent map, so we're working on moving the parent map out of ASTContext (with a plan to eventually move it out of the AST library). See D71313 for related work in this area.

Jan 24 2020, 1:53 PM · Restricted Project
steveire added a comment to D73028: Extract ExprTraversal class from Expr.

An addition to the API will be concerned with ascending through the AST in different traversal modes.

Ascending through the AST is not possibly by design. (For example, we share AST nodes between template instantiations, and statement nodes don't contain parent pointers.) Can you say more about what you're thinking of adding here?

Jan 24 2020, 12:36 PM · Restricted Project
steveire updated the diff for D73028: Extract ExprTraversal class from Expr.

Update

Jan 24 2020, 12:36 PM · Restricted Project
steveire updated the diff for D73028: Extract ExprTraversal class from Expr.

Update for review comments

Jan 24 2020, 12:36 PM · Restricted Project

Jan 20 2020

steveire created D73037: Add a way to set traversal mode in clang-query.
Jan 20 2020, 5:40 AM · Restricted Project
steveire updated the diff for D73029: Extend ExprTraversal class with acending traversal.

Format

Jan 20 2020, 4:35 AM · Restricted Project
steveire updated the diff for D73028: Extract ExprTraversal class from Expr.

Format

Jan 20 2020, 4:35 AM · Restricted Project
steveire created D73029: Extend ExprTraversal class with acending traversal.
Jan 20 2020, 4:07 AM · Restricted Project
steveire created D73028: Extract ExprTraversal class from Expr.
Jan 20 2020, 4:07 AM · Restricted Project

Jan 10 2020

steveire updated the diff for D72531: Set traversal explicitly where needed in tests.

Fix case

Jan 10 2020, 12:42 PM · Restricted Project
steveire updated the diff for D72530: Set traversal explicitly where needed in clang-tidy.

Fix case

Jan 10 2020, 12:42 PM · Restricted Project
steveire added a comment to D72233: Add a new AST matcher 'optionally'..

LGTM too. I described a more ad-hoc implementation previously: https://steveire.wordpress.com/2018/11/20/composing-ast-matchers-in-clang-tidy/ but I prefer this implementation.

Jan 10 2020, 12:33 PM
steveire created D72534: Change default traversal in AST Matchers to ignore invisible nodes.
Jan 10 2020, 11:56 AM · Restricted Project
steveire created D72532: Make the ExprMutationAnalyzer explicit about how it traverses the AST.
Jan 10 2020, 11:55 AM · Restricted Project
steveire created D72531: Set traversal explicitly where needed in tests.
Jan 10 2020, 11:55 AM · Restricted Project
steveire created D72530: Set traversal explicitly where needed in clang-tidy.
Jan 10 2020, 11:55 AM · Restricted Project

Dec 29 2019

steveire created D71977: Implement additional traverse() overloads.
Dec 29 2019, 12:41 PM · Restricted Project
steveire created D71976: Match code following lambdas when ignoring invisible nodes.
Dec 29 2019, 12:41 PM · Restricted Project

Dec 26 2019

steveire added inline comments to D71842: Allow newlines in AST Matchers in clang-query files.
Dec 26 2019, 9:47 AM · Restricted Project
steveire updated the diff for D71842: Allow newlines in AST Matchers in clang-query files.

Update

Dec 26 2019, 9:10 AM · Restricted Project
steveire added inline comments to D71842: Allow newlines in AST Matchers in clang-query files.
Dec 26 2019, 9:10 AM · Restricted Project

Dec 23 2019

steveire created D71842: Allow newlines in AST Matchers in clang-query files.
Dec 23 2019, 9:06 AM · Restricted Project

Dec 18 2019

steveire updated the diff for D71680: Customize simplified dumping and matching of LambdaExpr.

Update

Dec 18 2019, 4:23 PM · Restricted Project
steveire updated the diff for D71680: Customize simplified dumping and matching of LambdaExpr.

Update

Dec 18 2019, 4:23 PM · Restricted Project
steveire created D71680: Customize simplified dumping and matching of LambdaExpr.
Dec 18 2019, 2:58 PM · Restricted Project

Dec 10 2019

steveire added a comment to D70613: Add method to ignore invisible AST nodes.

It wasn't possible to add the const because of the return type.

Dec 10 2019, 1:42 PM · Restricted Project
steveire updated the diff for D70613: Add method to ignore invisible AST nodes.

Update

Dec 10 2019, 1:42 PM · Restricted Project

Dec 7 2019

steveire created D71166: Deprecate the hasDefaultArgument matcher.
Dec 7 2019, 10:25 AM · Restricted Project

Dec 6 2019

steveire closed D54406: Add matchDynamic convenience functions.

Committed in 2e8dc8590d8b412

Dec 6 2019, 4:07 PM · Restricted Project
steveire closed D62056: Use ASTDumper to dump the AST from clang-query.

Committed in b22d8ae7f436bfe63. I don't know why this review didn't automatically update.

Dec 6 2019, 3:48 PM · Restricted Project
steveire added a comment to D61837: Make it possible control matcher traversal kind with ASTContext.

I tried using DenseMap, but couldn't make it compile. I stuck with std::map for now.

Dec 6 2019, 3:21 PM · Restricted Project

Nov 22 2019

steveire created D70613: Add method to ignore invisible AST nodes.
Nov 22 2019, 12:16 PM · Restricted Project
steveire updated the diff for D61837: Make it possible control matcher traversal kind with ASTContext.

Rebase and update

Nov 22 2019, 9:58 AM · Restricted Project

Oct 14 2019

steveire committed rL374845: Add myself to the github migration.
Add myself to the github migration
Oct 14 2019, 4:30 PM

May 19 2019

steveire committed rL361117: Add a Visit overload for DynTypedNode to ASTNodeTraverser.
Add a Visit overload for DynTypedNode to ASTNodeTraverser
May 19 2019, 6:05 AM
steveire committed rC361117: Add a Visit overload for DynTypedNode to ASTNodeTraverser.
Add a Visit overload for DynTypedNode to ASTNodeTraverser
May 19 2019, 6:05 AM

May 17 2019

steveire added inline comments to D61837: Make it possible control matcher traversal kind with ASTContext.
May 17 2019, 7:45 AM · Restricted Project
steveire created D62065: Move dump method implementations to their respective class files.
May 17 2019, 7:37 AM · Restricted Project
steveire committed rC361034: Extract ASTDumper to a header file.
Extract ASTDumper to a header file
May 17 2019, 7:03 AM
steveire committed rL361034: Extract ASTDumper to a header file.
Extract ASTDumper to a header file
May 17 2019, 7:03 AM
steveire closed D61835: Extract ASTDumper to a header file.
May 17 2019, 7:03 AM · Restricted Project
steveire added a comment to D61835: Extract ASTDumper to a header file.

I'm going to investigate whether we can move the dump method implementations to their respective class files, and then look into a rename for this to StreamNodeDumper or so (name tbd).

May 17 2019, 7:02 AM · Restricted Project
steveire committed rC361033: Add a Visit overload for DynTypedNode to ASTNodeTraverser.
Add a Visit overload for DynTypedNode to ASTNodeTraverser
May 17 2019, 6:54 AM
steveire committed rL361033: Add a Visit overload for DynTypedNode to ASTNodeTraverser.
Add a Visit overload for DynTypedNode to ASTNodeTraverser
May 17 2019, 6:53 AM
steveire closed D61834: Add a Visit overload for DynTypedNode to ASTNodeTraverser.
May 17 2019, 6:53 AM · Restricted Project, Restricted Project
steveire added inline comments to D61834: Add a Visit overload for DynTypedNode to ASTNodeTraverser.
May 17 2019, 6:53 AM · Restricted Project, Restricted Project
steveire added inline comments to D61837: Make it possible control matcher traversal kind with ASTContext.
May 17 2019, 5:37 AM · Restricted Project
steveire updated the diff for D61834: Add a Visit overload for DynTypedNode to ASTNodeTraverser.

Update

May 17 2019, 1:47 AM · Restricted Project, Restricted Project
steveire created D62056: Use ASTDumper to dump the AST from clang-query.
May 17 2019, 1:40 AM · Restricted Project

May 16 2019

steveire updated the diff for D61834: Add a Visit overload for DynTypedNode to ASTNodeTraverser.

Add basic traverser test

May 16 2019, 3:55 PM · Restricted Project, Restricted Project
steveire added a comment to D61834: Add a Visit overload for DynTypedNode to ASTNodeTraverser.

What will be making use of/testing this new functionality?

Any code which has a DynTypedNode and wishes to traverse it.

I envisage this as a more-flexible DynTypedNode::dump that the user does not have to implement themselves in order to use the ASTNodeTraverser.

Do we currently have any such code that's using this functionality, though? I'm mostly concerned that this is dead code with no testing, currently. The functionality itself seems reasonable enough and the code looks correct enough, so if this is part of a series of planned changes, that'd be good information to have for the review.

May 16 2019, 1:35 PM · Restricted Project, Restricted Project
steveire added a comment to D61835: Extract ASTDumper to a header file.
  1. Anyone who wants traversal in the same way that dumping is done and who needs to call API on the instance which is provided by ASTNodeTraverser (which ASTDumper inherits) needs to use ASTDumper. For example my UI. See my EuroLLVM talk for more: https://steveire.wordpress.com/2019/04/30/the-future-of-ast-matching-refactoring-tools-eurollvm-and-accu/

Do they? Why is the ASTNodeTraverser insufficient?

It doesn't have the same behavior as ASTDumper, because ASTDumper "overrides" some Visit metthods.

I'm aware that they're different, but I may not have been sufficiently clear. I'm saying that the only public APIs I think a user should be calling are inherited from ASTNodeTraverser and not ASTDumper, so it is not required to expose ASTDumper directly, only a way to get an ASTNodeTraverser reference/pointer to an ASTDumper instance so that you get the correct virtual dispatch. e.g., ASTNodeTraverser *getSomethingThatActsLikeAnASTDumper() { return new ASTDumper; }

May 16 2019, 1:27 PM · Restricted Project
steveire updated the diff for D61837: Make it possible control matcher traversal kind with ASTContext.

Update

May 16 2019, 12:34 PM · Restricted Project
steveire added inline comments to D61837: Make it possible control matcher traversal kind with ASTContext.
May 16 2019, 12:34 PM · Restricted Project
steveire added a reviewer for D61835: Extract ASTDumper to a header file: klimek.
May 16 2019, 12:25 PM · Restricted Project
steveire added a comment to D61835: Extract ASTDumper to a header file.
  1. Anyone who wants traversal in the same way that dumping is done and who needs to call API on the instance which is provided by ASTNodeTraverser (which ASTDumper inherits) needs to use ASTDumper. For example my UI. See my EuroLLVM talk for more: https://steveire.wordpress.com/2019/04/30/the-future-of-ast-matching-refactoring-tools-eurollvm-and-accu/

Do they? Why is the ASTNodeTraverser insufficient?

May 16 2019, 12:25 PM · Restricted Project
steveire updated the diff for D61837: Make it possible control matcher traversal kind with ASTContext.

Update

May 16 2019, 11:49 AM · Restricted Project
steveire added inline comments to D61837: Make it possible control matcher traversal kind with ASTContext.
May 16 2019, 11:49 AM · Restricted Project
steveire added a comment to D61835: Extract ASTDumper to a header file.

The users of the follow-up patch https://reviews.llvm.org/D61837#change-x5mxz9Lpijjs need that 'correctness', but also need the public API from ASTNodeTraverser on the instance. (That patch also extends the public API for users).

I don't see anything in the follow-up patch that actually uses the ASTDumper class though, so I'm still a bit confused.

May 16 2019, 11:43 AM · Restricted Project
steveire committed rC360922: Update comments on enums.
Update comments on enums
May 16 2019, 11:01 AM
steveire committed rL360922: Update comments on enums.
Update comments on enums
May 16 2019, 11:01 AM
steveire added a comment to D61836: Move TraversalKind enum to ast_type_traits.

Thanks, I made the comment changes in a separate commit because otherwise this would cease to be a refactoring commit. Commits get too noisy if their content is modified "mid-flight".

May 16 2019, 11:01 AM · Restricted Project
steveire committed rL360920: Move TraversalKind enum to ast_type_traits.
Move TraversalKind enum to ast_type_traits
May 16 2019, 10:55 AM
steveire committed rC360920: Move TraversalKind enum to ast_type_traits.
Move TraversalKind enum to ast_type_traits
May 16 2019, 10:55 AM
steveire closed D61836: Move TraversalKind enum to ast_type_traits.
May 16 2019, 10:55 AM · Restricted Project
steveire added a comment to D61834: Add a Visit overload for DynTypedNode to ASTNodeTraverser.

What will be making use of/testing this new functionality?

May 16 2019, 10:52 AM · Restricted Project, Restricted Project
steveire added a comment to D61835: Extract ASTDumper to a header file.

I'm not certain where you're planning to go with this change (or is this the only change you're trying to make in this area?), so it's a bit hard to evaluate this patch. Can you explain a bit more about what you're ultimately trying to accomplish?

It might help if I had a better idea of which APIs you thought were ones that would help users (because my only real concern with this change is that the public interface for this class is rather unpleasant).

May 16 2019, 10:47 AM · Restricted Project
steveire added a reviewer for D61835: Extract ASTDumper to a header file: martong.
May 16 2019, 1:12 AM · Restricted Project
steveire added a reviewer for D61836: Move TraversalKind enum to ast_type_traits: martong.
May 16 2019, 1:12 AM · Restricted Project
steveire added a reviewer for D61837: Make it possible control matcher traversal kind with ASTContext: martong.
May 16 2019, 1:12 AM · Restricted Project
steveire added a reviewer for D61834: Add a Visit overload for DynTypedNode to ASTNodeTraverser: martong.
May 16 2019, 1:08 AM · Restricted Project, Restricted Project
steveire added reviewers for D61837: Make it possible control matcher traversal kind with ASTContext: ilya-biryukov, sammccall.
May 16 2019, 12:58 AM · Restricted Project
steveire added reviewers for D61836: Move TraversalKind enum to ast_type_traits: ilya-biryukov, sammccall.
May 16 2019, 12:58 AM · Restricted Project
steveire added reviewers for D61835: Extract ASTDumper to a header file: ilya-biryukov, sammccall.
May 16 2019, 12:58 AM · Restricted Project
steveire added reviewers for D61834: Add a Visit overload for DynTypedNode to ASTNodeTraverser: ilya-biryukov, sammccall.
May 16 2019, 12:58 AM · Restricted Project, Restricted Project

May 13 2019

steveire added a comment to D60847: [CMake] Enable policy CMP0056.

Indeed, the cmake_minimum_required command sets policies to new.

May 13 2019, 12:41 AM · Restricted Project

May 12 2019

steveire updated the diff for D61837: Make it possible control matcher traversal kind with ASTContext.

Format

May 12 2019, 2:24 PM · Restricted Project
steveire added inline comments to D60910: [WIP] Dumping the AST to JSON.
May 12 2019, 1:47 PM
steveire updated the diff for D61837: Make it possible control matcher traversal kind with ASTContext.

Format

May 12 2019, 1:44 PM · Restricted Project
steveire updated the diff for D61835: Extract ASTDumper to a header file.

Format

May 12 2019, 1:44 PM · Restricted Project
steveire updated the diff for D61836: Move TraversalKind enum to ast_type_traits.

Format

May 12 2019, 1:44 PM · Restricted Project
steveire updated the diff for D61834: Add a Visit overload for DynTypedNode to ASTNodeTraverser.

Format

May 12 2019, 1:43 PM · Restricted Project, Restricted Project
steveire added a comment to D61837: Make it possible control matcher traversal kind with ASTContext.

@klimek This includes a test for the memoization case you were interested in at EuroLLVM.

May 12 2019, 1:28 PM · Restricted Project
steveire created D61837: Make it possible control matcher traversal kind with ASTContext.
May 12 2019, 1:26 PM · Restricted Project
steveire added a comment to D61836: Move TraversalKind enum to ast_type_traits.

This is part of the work I demo'd at EuroLLVM for ignoring invisible AST nodes during AST Matching and dumping: http://ce.steveire.com/z/lHYwEH

May 12 2019, 1:25 PM · Restricted Project
steveire created D61836: Move TraversalKind enum to ast_type_traits.
May 12 2019, 1:18 PM · Restricted Project
steveire created D61835: Extract ASTDumper to a header file.
May 12 2019, 1:18 PM · Restricted Project
steveire created D61834: Add a Visit overload for DynTypedNode to ASTNodeTraverser.
May 12 2019, 1:15 PM · Restricted Project, Restricted Project

Apr 29 2019

steveire added a comment to D60910: [WIP] Dumping the AST to JSON.

Thanks for doing this! I'm glad you were able to do it without needing to change the traverser class. That's a good indicator.

Apr 29 2019, 3:15 PM

Feb 5 2019

steveire accepted D56850: [ASTMatchers][NFC] Add tests for assorted `CXXMemberCallExpr` matchers..

Sorry, this fell off my radar :). LGTM too.

Feb 5 2019, 1:12 PM · Restricted Project, Restricted Project