Page MenuHomePhabricator

steveire (Stephen Kelly)
User

Projects

User does not belong to any projects.

User Details

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

Recent Activity

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

Feb 3 2019

steveire accepted D57649: [ASTDump] Add a flag indicating whether a CXXThisExpr is implicit.
Feb 3 2019, 9:51 AM · Restricted Project
steveire added a comment to D57649: [ASTDump] Add a flag indicating whether a CXXThisExpr is implicit.

I have no objection to this, but I wonder whether all state accessible from all nodes should be part of the AST dump. Where do you think the line is? Is there anything else missing currently from other nodes?

Feb 3 2019, 9:50 AM · Restricted Project
steveire committed rC352989: [AST] Extract ASTNodeTraverser class from ASTDumper.
[AST] Extract ASTNodeTraverser class from ASTDumper
Feb 3 2019, 6:08 AM
steveire committed rL352989: [AST] Extract ASTNodeTraverser class from ASTDumper.
[AST] Extract ASTNodeTraverser class from ASTDumper
Feb 3 2019, 6:08 AM
steveire closed D57472: [AST] Extract ASTDumpTraverser class from ASTDumper.
Feb 3 2019, 6:08 AM · Restricted Project, Restricted Project

Feb 1 2019

steveire added inline comments to D57472: [AST] Extract ASTDumpTraverser class from ASTDumper.
Feb 1 2019, 11:33 AM · Restricted Project, Restricted Project

Jan 31 2019

steveire updated the diff for D57472: [AST] Extract ASTDumpTraverser class from ASTDumper.

Update

Jan 31 2019, 2:41 PM · Restricted Project, Restricted Project
steveire added inline comments to D57472: [AST] Extract ASTDumpTraverser class from ASTDumper.
Jan 31 2019, 2:40 PM · Restricted Project, Restricted Project
steveire committed rC352804: [ASTDump] Make template specialization tests more exact.
[ASTDump] Make template specialization tests more exact
Jan 31 2019, 2:28 PM
steveire committed rL352804: [ASTDump] Make template specialization tests more exact.
[ASTDump] Make template specialization tests more exact
Jan 31 2019, 2:28 PM
steveire closed D57502: [ASTDump] Make template specialization tests more exact.
Jan 31 2019, 2:28 PM
steveire created D57502: [ASTDump] Make template specialization tests more exact.
Jan 31 2019, 2:17 AM

Jan 30 2019

steveire added inline comments to D57472: [AST] Extract ASTDumpTraverser class from ASTDumper.
Jan 30 2019, 1:57 PM · Restricted Project, Restricted Project
steveire updated the diff for D57472: [AST] Extract ASTDumpTraverser class from ASTDumper.

Fix comment

Jan 30 2019, 1:51 PM · Restricted Project, Restricted Project
steveire created D57472: [AST] Extract ASTDumpTraverser class from ASTDumper.
Jan 30 2019, 1:49 PM · Restricted Project, Restricted Project
steveire committed rL352676: [ASTDump] Inline traverse methods into class.
[ASTDump] Inline traverse methods into class
Jan 30 2019, 1:49 PM
steveire committed rC352676: [ASTDump] Inline traverse methods into class.
[ASTDump] Inline traverse methods into class
Jan 30 2019, 1:49 PM
steveire committed rL352663: [ASTDump] Make method definition order matches declaration order.
[ASTDump] Make method definition order matches declaration order
Jan 30 2019, 12:07 PM
steveire committed rC352663: [ASTDump] Make method definition order matches declaration order.
[ASTDump] Make method definition order matches declaration order
Jan 30 2019, 12:07 PM
steveire committed rC352661: [ASTDump] Re-arrange method declarations to group Visit together.
[ASTDump] Re-arrange method declarations to group Visit together
Jan 30 2019, 12:03 PM
steveire committed rL352661: [ASTDump] Re-arrange method declarations to group Visit together.
[ASTDump] Re-arrange method declarations to group Visit together
Jan 30 2019, 12:03 PM
steveire committed rC352657: [ASTDump] Rename methods which are conceptually Visits.
[ASTDump] Rename methods which are conceptually Visits
Jan 30 2019, 11:51 AM
steveire committed rL352657: [ASTDump] Rename methods which are conceptually Visits.
[ASTDump] Rename methods which are conceptually Visits
Jan 30 2019, 11:51 AM
steveire committed rC352656: [ASTDump] NFC: Inline vestigial methods.
[ASTDump] NFC: Inline vestigial methods
Jan 30 2019, 11:41 AM
steveire committed rL352656: [ASTDump] NFC: Inline vestigial methods.
[ASTDump] NFC: Inline vestigial methods
Jan 30 2019, 11:41 AM
steveire added a comment to D57419: [ASTDump] Move Decl node dumping to TextNodeDumper.

Needs rebasing for rL352631.

Jan 30 2019, 11:33 AM
steveire committed rC352655: [ASTDump] Move Decl node dumping to TextNodeDumper.
[ASTDump] Move Decl node dumping to TextNodeDumper
Jan 30 2019, 11:32 AM
steveire committed rL352655: [ASTDump] Move Decl node dumping to TextNodeDumper.
[ASTDump] Move Decl node dumping to TextNodeDumper
Jan 30 2019, 11:32 AM
steveire closed D57419: [ASTDump] Move Decl node dumping to TextNodeDumper.
Jan 30 2019, 11:32 AM

Jan 29 2019

steveire abandoned D56959: [AST] NFC: Introduce new class GenericSelectionExpr::Association.

An alternative was written and committed.

Jan 29 2019, 3:24 PM
steveire created D57419: [ASTDump] Move Decl node dumping to TextNodeDumper.
Jan 29 2019, 3:14 PM
steveire committed rL352558: NFC: Move GenericSelectionExpr dump to NodeDumper.
NFC: Move GenericSelectionExpr dump to NodeDumper
Jan 29 2019, 2:58 PM
steveire committed rC352558: NFC: Move GenericSelectionExpr dump to NodeDumper.
NFC: Move GenericSelectionExpr dump to NodeDumper
Jan 29 2019, 2:58 PM
steveire closed D56961: NFC: Move GenericSelectionExpr dump to NodeDumper.
Jan 29 2019, 2:58 PM
steveire committed rL352552: NFC: Implement GenericSelectionExpr::Association dump with Visitor.
NFC: Implement GenericSelectionExpr::Association dump with Visitor
Jan 29 2019, 2:23 PM
steveire committed rC352552: NFC: Implement GenericSelectionExpr::Association dump with Visitor.
NFC: Implement GenericSelectionExpr::Association dump with Visitor
Jan 29 2019, 2:23 PM
steveire closed D56960: NFC: Implement GenericSelectionExpr::Association dump with Visitor.
Jan 29 2019, 2:22 PM
steveire added a comment to D57106: [AST] Introduce GenericSelectionExpr::Association.

This was subsequently reverted. Is there a status update? Rebasing and committing D56959 would unblock traverser work and would allow this to progress separately in parallel.

Jan 29 2019, 2:00 AM · Restricted Project

Jan 26 2019

steveire added a comment to D57104: [AST] Pack GenericSelectionExpr.

I was just working on this this afternoon to show you the difference when it is split up. Looks like you committed this meanwhile.

Jan 26 2019, 8:09 AM · Restricted Project

Jan 25 2019

steveire added a comment to D57104: [AST] Pack GenericSelectionExpr.

Cleanup the patch by factoring out the NFC changes.

@steveire This should be much clearer now.

Jan 25 2019, 6:12 AM · Restricted Project

Jan 24 2019

steveire added a comment to D57104: [AST] Pack GenericSelectionExpr.

I highly recommend this 9 minute video if this is new to you or you haven't seen it before: https://youtu.be/qpdYRPL3SVE?t=103

I would like to add an additional meta-comment here, but please don't take this in a bad way. I am wondering
about the usefulness and the productivity of the "if this is new to you" in what I am assuming is a discussion
between professionals. I will be happy to address any further technical comments regarding the code itself.

Jan 24 2019, 2:55 PM · Restricted Project
steveire added a comment to D57104: [AST] Pack GenericSelectionExpr.

There's definitely a better possible ordering in two commits:

  1. Introduce ::Create and port to it
  2. Use trailing objects, taking advantage of the fact that ::Create exists.

    That would make it clear in the future to other people because both commits would be cleaner, both commit messages would say what the commit does, and neither commit would have the noise of the other change.

    Not splitting this commit makes it less reviewable to people who are not around today.
Jan 24 2019, 1:18 PM · Restricted Project
steveire added a comment to D57104: [AST] Pack GenericSelectionExpr.

There's definitely a better possible ordering in two commits:

Jan 24 2019, 1:11 PM · Restricted Project
steveire added a comment to D57104: [AST] Pack GenericSelectionExpr.

Thanks, but I don't see the factored out changes in the repo. Are you going to commit those first?

Jan 24 2019, 11:54 AM · Restricted Project