Page MenuHomePhabricator

steveire (Stephen Kelly)
User

Projects

User does not belong to any projects.

User Details

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

Recent Activity

May 11 2021

steveire added a comment to D102213: [ASTMatchers] Add forCallable(), a generalization of forFunction()..

I'm not certain what strong opinions I've voiced on the mailing list about that. Was it just that I think hasDescendant can lead to unexpected matches and a more-specific matcher should always be used when available (such as hasArgument)? I do think has, hasDescendant, hasAncestor etc have their uses and they get more useful with utilities like forFunction.

May 11 2021, 2:24 AM · Restricted Project
steveire edited reviewers for D102214: [clang-tidy] bugprone-infinite-loop: forFunction() -> forCallable()., added: steveire; removed: stephenkelly.
May 11 2021, 2:02 AM · Restricted Project
steveire edited reviewers for D102213: [ASTMatchers] Add forCallable(), a generalization of forFunction()., added: steveire; removed: stephenkelly.
May 11 2021, 2:02 AM · Restricted Project

May 10 2021

steveire added a reviewer for D102185: Widen `name` stencil to support `TypeLoc` nodes.: ymandel.

Adding Yitzhak as a reviewer. I notice that at least the name of a cxxBaseSpecifier is not supported and I don't know if that (or lack of typeloc support) is desired in the Transformer design.

May 10 2021, 4:11 PM · Restricted Project

May 6 2021

steveire accepted D101572: Make `hasTypeLoc` matcher support more node types..

This looks good to me. I tried running it in clang-query, which crashes with a matcher like

May 6 2021, 3:51 PM · Restricted Project

May 5 2021

steveire removed a reviewer for D101792: Bug 45973 - Inaccurate system requirements: stephenkelly.
May 5 2021, 1:25 PM · Restricted Project
steveire added a comment to D101572: Make `hasTypeLoc` matcher support more node types..

Please run llvm-project/clang/docs/tools/dump_ast_matchers.py to generate updated documentation for this (there is a urlopen(url) which slows things down, so I usually comment that out when running it).

May 5 2021, 12:56 PM · Restricted Project
steveire added a comment to D101792: Bug 45973 - Inaccurate system requirements.

Hi, thanks for this. I'm not sure there was enough assent on the mailing list or agreement on the direction, but it might indeed make sense to put a patch together to discuss it.

May 5 2021, 12:24 PM · Restricted Project

May 1 2021

steveire accepted D101614: [clang-tidy][NFC] Simplify a lot of bugprone-sizeof-expression matchers.

LGTM, but I think you could split it into 3 commits with a commit message saying what each is doing. "Simplify a lot of" doesn't say anything specific about what this patch does. It looks like you could split it into

May 1 2021, 8:07 AM · Restricted Project

Apr 30 2021

steveire added inline comments to D101614: [clang-tidy][NFC] Simplify a lot of bugprone-sizeof-expression matchers.
Apr 30 2021, 3:45 PM · Restricted Project
steveire added a comment to D101572: Make `hasTypeLoc` matcher support more node types..

Yes, the motivation for adding these is so I can use clang-transformer to refactor a bunch of constructor calls to call a static factory method instead.

// Before
ns::Foo x(1);
auto y = ns::Foo(1, 2);

// After
auto x = ns::Foo::Make(1);
auto y = ns::Foo::Make(1, 2);

That refactor only needs hasTypeInfo to work on these three node types, but for consistency's sake I'd be happy to add overloads for others as well. I looked at the types in the results of the grep you posted; I think it would make sense for hasTypeInfo to be able to handle the classes in the first and third groups; do you think that would be reasonable?

Pretty sure these should work with hasTypeInfo.

  • TypedefNameDecl (getTypeSourceInfo)
  • CXXBaseSpecifier (getTypeSourceInfo)
  • CXXCtorInitializer (getTypeSourceInfo)
  • ObjCPropertyDecl (getTypeSourceInfo)
  • ClassTemplateSpecializationDecl (getTypeAsWritten)
  • CompoundLiteralExpr (getTypeSourceInfo)
  • ExplicitCastExpr (getTypeInfoAsWritten)
  • CXXTemporaryObjectExpr (getTypeSourceInfo)
  • CXXUnresolvedConstructExpr (getTypeSourceInfo)
  • TemplateArgumentLoc (getTypeSourceInfo)

These could make sense, but no other matchers exist for nodes of these types:

  • VarTemplateSpecializationDecl (getTypeAsWritten)
  • OffsetOfExpr (getTypeSourceInfo)
  • ConvertVectorExpr (getTypeSourceInfo)
  • VAArgExpr (getWrittenTypeInfo)
  • CXXScalarValueInitExpr (getTypeSourceInfo)

These nodes have a method with a different name, but probably still make sense to use with hasTypeLoc.

  • BlockDecl (getSignatureAsWritten)
  • CXXNewExpr (getAllocatedTypeSourceInfo)

These all have a method that returns a TypeSourceInfo*, but seem like they're expressing something different and shouldn't work with hasTypeInfo.

  • EnumDecl (getIntegerTypeSourceInfo)
  • CXXRecordDecl (getLambdaTypeInfo) fails if not a lambda
  • FriendDecl (getFriendType)
  • ObjCMethodDecl (getReturnTypeSourceInfo)
  • ObjCInterfaceDecl (getSuperClassTInfo)
  • UnaryExprOrTypeTraitExpr (getArgumentTypeInfo) fails when arg is expr instead of type
Apr 30 2021, 1:28 PM · Restricted Project

Apr 29 2021

steveire requested review of D101589: [AST] Add Concept-related locations to node introspection.
Apr 29 2021, 4:21 PM · Restricted Project
steveire edited reviewers for D101572: Make `hasTypeLoc` matcher support more node types., added: steveire; removed: stephenkelly.
Apr 29 2021, 3:27 PM · Restricted Project
steveire added a reviewer for D101572: Make `hasTypeLoc` matcher support more node types.: stephenkelly.

According to

Apr 29 2021, 3:26 PM · Restricted Project
steveire added a comment to D100092: [clang-tidy] cppcoreguidelines-declare-loop-variable-in-the-initializer: a new check.

I implemented something like this recently too. The code I was trying to refactor was something like

Apr 29 2021, 1:50 PM · Restricted Project

Apr 27 2021

steveire updated the diff for D101346: [AST] Fix getExprLoc reported for cxxDependentScopeMemberExpr.

Fix windows build

Apr 27 2021, 2:56 PM · Restricted Project
steveire accepted D101365: [clang-query] Add check to prevent setting srcloc when no introspection is available..

LGTM!

Apr 27 2021, 2:46 PM · Restricted Project
steveire updated the diff for D101346: [AST] Fix getExprLoc reported for cxxDependentScopeMemberExpr.

Update test

Apr 27 2021, 2:20 AM · Restricted Project
steveire requested review of D101346: [AST] Fix getExprLoc reported for cxxDependentScopeMemberExpr.
Apr 27 2021, 1:32 AM · Restricted Project

Apr 23 2021

steveire updated the diff for D101049: [AST] Add DeclarationNameInfo to node introspection.

Update

Apr 23 2021, 8:28 AM · Restricted Project

Apr 22 2021

steveire updated the diff for D101049: [AST] Add DeclarationNameInfo to node introspection.

Update

Apr 22 2021, 3:05 PM · Restricted Project
steveire updated the diff for D93325: Add srcloc output to clang-query.

Update

Apr 22 2021, 5:24 AM · Restricted Project, Restricted Project
steveire requested review of D101054: [AST] Sort introspection results without instantiating other data.
Apr 22 2021, 5:24 AM · Restricted Project
steveire updated the diff for D101049: [AST] Add DeclarationNameInfo to node introspection.

Update

Apr 22 2021, 4:35 AM · Restricted Project
steveire requested review of D101049: [AST] Add DeclarationNameInfo to node introspection.
Apr 22 2021, 4:33 AM · Restricted Project
steveire added a comment to D98774: [AST] De-duplicate empty node introspection.

I think it is? https://llvm.org/docs/GettingStarted.html#software lists it at least.

Apr 22 2021, 4:32 AM · Restricted Project, Restricted Project

Apr 19 2021

steveire added a comment to D93325: Add srcloc output to clang-query.

@aaron.ballman ping?

Apr 19 2021, 2:12 PM · Restricted Project, Restricted Project
steveire updated the diff for D100712: [AST] Add NestedNameSpecifierLoc accessors to node introspection.

Update

Apr 19 2021, 1:59 PM · Restricted Project
steveire added inline comments to D100712: [AST] Add NestedNameSpecifierLoc accessors to node introspection.
Apr 19 2021, 1:26 PM · Restricted Project
steveire updated the diff for D100712: [AST] Add NestedNameSpecifierLoc accessors to node introspection.

Update

Apr 19 2021, 1:24 PM · Restricted Project

Apr 18 2021

steveire added inline comments to D100712: [AST] Add NestedNameSpecifierLoc accessors to node introspection.
Apr 18 2021, 3:19 PM · Restricted Project
steveire added inline comments to D100712: [AST] Add NestedNameSpecifierLoc accessors to node introspection.
Apr 18 2021, 10:42 AM · Restricted Project
steveire added inline comments to D100723: [AST] Fix comparison to of SourceRanges in container.
Apr 18 2021, 10:24 AM · Restricted Project
steveire added a comment to D100638: [AST][Introspection] Avoid creating temporary strings when comparing LocationCalls..

Your implementation is getting very complicated and it requires many comments.

Apr 18 2021, 6:35 AM · Restricted Project
steveire requested review of D100723: [AST] Fix comparison to of SourceRanges in container.
Apr 18 2021, 5:34 AM · Restricted Project
steveire accepted D100719: [Introspection] Dont emit json if unchanged..

Great, thanks!

Apr 18 2021, 3:25 AM · Restricted Project
steveire requested review of D100720: [AST] Update introspection API to use const-ref for copyable types.
Apr 18 2021, 3:13 AM · Restricted Project

Apr 17 2021

steveire requested review of D100712: [AST] Add NestedNameSpecifierLoc accessors to node introspection.
Apr 17 2021, 3:31 PM · Restricted Project
steveire added inline comments to D100516: [AST] Add TypeLoc support to node introspection.
Apr 17 2021, 3:28 PM · Restricted Project

Apr 16 2021

steveire updated the summary of D100688: [AST] Remove args from LocationCall.
Apr 16 2021, 2:32 PM · Restricted Project
steveire requested review of D100688: [AST] Remove args from LocationCall.
Apr 16 2021, 2:32 PM · Restricted Project
steveire added inline comments to D100638: [AST][Introspection] Avoid creating temporary strings when comparing LocationCalls..
Apr 16 2021, 5:15 AM · Restricted Project
steveire added inline comments to D100638: [AST][Introspection] Avoid creating temporary strings when comparing LocationCalls..
Apr 16 2021, 4:28 AM · Restricted Project
steveire updated the diff for D100516: [AST] Add TypeLoc support to node introspection.

Update

Apr 16 2021, 3:26 AM · Restricted Project
steveire updated the diff for D100516: [AST] Add TypeLoc support to node introspection.

Update

Apr 16 2021, 2:10 AM · Restricted Project

Apr 15 2021

steveire added inline comments to D100516: [AST] Add TypeLoc support to node introspection.
Apr 15 2021, 3:53 PM · Restricted Project
steveire updated the diff for D100516: [AST] Add TypeLoc support to node introspection.

Update

Apr 15 2021, 3:53 PM · Restricted Project
steveire accepted D100423: [AST] Add a print method to Introspection LocationCall.

Thanks!

Apr 15 2021, 6:32 AM · Restricted Project
steveire updated the summary of D100516: [AST] Add TypeLoc support to node introspection.
Apr 15 2021, 4:52 AM · Restricted Project
steveire updated the diff for D100516: [AST] Add TypeLoc support to node introspection.

Update

Apr 15 2021, 4:52 AM · Restricted Project
steveire requested review of D100548: [AST] Fix location call storage with common last-invocation.
Apr 15 2021, 4:30 AM · Restricted Project
steveire accepted D100530: [AST][Introspection] Add a check to detect if introspection is supported..
Apr 15 2021, 4:12 AM · Restricted Project

Apr 14 2021

steveire requested review of D100516: [AST] Add TypeLoc support to node introspection.
Apr 14 2021, 6:02 PM · Restricted Project
steveire requested changes to D100423: [AST] Add a print method to Introspection LocationCall.
Apr 14 2021, 3:54 PM · Restricted Project
steveire accepted D100343: [AST][NFC] Remove temporary ASTTU file from Introspection generation..
Apr 14 2021, 3:50 PM · Restricted Project
steveire added a comment to D100378: [AST] Use IntrusiveRefCntPtr for Introspection LocationCall..

Indeed, I just this evening discovered the need for this to be stable across runs for testing.

Apr 14 2021, 3:49 PM · Restricted Project
steveire accepted D100378: [AST] Use IntrusiveRefCntPtr for Introspection LocationCall..
Apr 14 2021, 3:49 PM · Restricted Project

Apr 13 2021

steveire added inline comments to D99231: [AST] Add introspection support for more base nodes.
Apr 13 2021, 3:37 PM · Restricted Project

Apr 12 2021

steveire updated the diff for D99231: [AST] Add introspection support for more base nodes.

Update

Apr 12 2021, 6:03 AM · Restricted Project
steveire added inline comments to D99231: [AST] Add introspection support for more base nodes.
Apr 12 2021, 4:33 AM · Restricted Project
steveire updated the diff for D99231: [AST] Add introspection support for more base nodes.

Update

Apr 12 2021, 4:33 AM · Restricted Project

Apr 11 2021

steveire updated the diff for D93325: Add srcloc output to clang-query.

Update

Apr 11 2021, 2:23 PM · Restricted Project, Restricted Project
steveire updated the diff for D93325: Add srcloc output to clang-query.

Update

Apr 11 2021, 3:47 AM · Restricted Project, Restricted Project
steveire updated the diff for D99231: [AST] Add introspection support for more base nodes.

Add locations for CXXBaseSpecifier

Apr 11 2021, 3:45 AM · Restricted Project

Apr 7 2021

steveire added a comment to D99231: [AST] Add introspection support for more base nodes.

@njames93 ping?

Apr 7 2021, 4:03 PM · Restricted Project

Apr 3 2021

steveire added a comment to D69218: [ASTMatchers] Add `cxxBaseSpecifier` matcher (non-top-level).

@nick Sorry that getting these changes merged takes so long.

@njames93 If you have an alternative way forward, please let us know what it is.

Otherwise, this LGTM too and we should merge it soon unless there are objections which haven't been addressed.

I had a PR in Boost that took 4 years to merge, so it is nothing new to me :D

Apr 3 2021, 7:24 AM · Restricted Project
steveire added a reviewer for D99715: [CMake] Fix Python 3 lookup when building LLVM with tests: steveire.

Yes, please remove the line instead.

Apr 3 2021, 5:26 AM · Restricted Project

Mar 28 2021

steveire added a comment to D69218: [ASTMatchers] Add `cxxBaseSpecifier` matcher (non-top-level).

@nick I think this also might need to be rebased again, sorry.

Mar 28 2021, 8:43 AM · Restricted Project
steveire accepted D69218: [ASTMatchers] Add `cxxBaseSpecifier` matcher (non-top-level).
Mar 28 2021, 8:39 AM · Restricted Project
steveire added a comment to D69218: [ASTMatchers] Add `cxxBaseSpecifier` matcher (non-top-level).

@nick Sorry that getting these changes merged takes so long.

Mar 28 2021, 8:39 AM · Restricted Project

Mar 27 2021

steveire updated the diff for D98296: [clang-tidy] Simplify readability checks to not need ignoring* matchers.

Rebase

Mar 27 2021, 3:23 PM · Restricted Project, Restricted Project
steveire accepted D97513: [CMake] Add <Project>ConfigVersion.cmake files.

I'll commit this on Wednesday if none of the other reviewers object by then.

Mar 27 2021, 3:09 PM · Restricted Project, Restricted Project, Restricted Project
steveire added inline comments to D97513: [CMake] Add <Project>ConfigVersion.cmake files.
Mar 27 2021, 3:00 PM · Restricted Project, Restricted Project, Restricted Project
steveire accepted D99451: Use write_basic_package_version_file for LLVM.

LGTM, thanks!

Mar 27 2021, 2:02 PM · Restricted Project
steveire added a comment to D99451: Use write_basic_package_version_file for LLVM.

There are LLVMConfig.cmake files in both ${CMAKE_CURRENT_BINARY_DIR}/CMakeFiles/LLVMConfig.cmake and ${llvm_cmake_builddir}/LLVMConfig.cmake, so there should be LLVMConfigVersion.cmake files next to both as well.

Mar 27 2021, 12:08 PM · Restricted Project
steveire added a comment to D99451: Use write_basic_package_version_file for LLVM.

Also adds the file to the build tree, which the comments suggested would happen, but wasn't.

Mar 27 2021, 4:31 AM · Restricted Project

Mar 26 2021

steveire added inline comments to D97513: [CMake] Add <Project>ConfigVersion.cmake files.
Mar 26 2021, 3:18 PM · Restricted Project, Restricted Project, Restricted Project
steveire added inline comments to D97513: [CMake] Add <Project>ConfigVersion.cmake files.
Mar 26 2021, 3:17 PM · Restricted Project, Restricted Project, Restricted Project

Mar 23 2021

steveire requested review of D99231: [AST] Add introspection support for more base nodes.
Mar 23 2021, 5:26 PM · Restricted Project
steveire added a comment to D69218: [ASTMatchers] Add `cxxBaseSpecifier` matcher (non-top-level).

Does anything prevent this going in now?

Mar 23 2021, 3:23 PM · Restricted Project

Mar 21 2021

steveire updated the diff for D98775: [AST] Add introspection support for Decls.

Update

Mar 21 2021, 8:05 AM · Restricted Project

Mar 20 2021

steveire updated the diff for D98775: [AST] Add introspection support for Decls.

Update

Mar 20 2021, 11:12 AM · Restricted Project
steveire added a comment to D98774: [AST] De-duplicate empty node introspection.

@thakis Do you have any more on this? Can we de-duplicate?

Mar 20 2021, 9:30 AM · Restricted Project, Restricted Project

Mar 18 2021

steveire added a comment to D98827: [AST] Ensure that an empty json file is generated if compile errors.

Are there any tests that ensure something is always outputted?

Mar 18 2021, 12:32 PM · Restricted Project

Mar 17 2021

steveire updated the diff for D98775: [AST] Add introspection support for Decls.

Update

Mar 17 2021, 4:31 PM · Restricted Project
steveire requested review of D98827: [AST] Ensure that an empty json file is generated if compile errors.
Mar 17 2021, 4:30 PM · Restricted Project
steveire added reviewers for D69218: [ASTMatchers] Add `cxxBaseSpecifier` matcher (non-top-level): steveire, njames93.

Could you rebase this? Parts of it were common with another patch which was merged.

Mar 17 2021, 10:46 AM · Restricted Project
steveire added a comment to D98774: [AST] De-duplicate empty node introspection.

Why were there two copies in the first place? What was the copy in the cmake file good for, why wasn't the one written by generate_cxx_src_locs.py sufficient?

Mar 17 2021, 10:15 AM · Restricted Project, Restricted Project
steveire updated the diff for D98774: [AST] De-duplicate empty node introspection.

Update

Mar 17 2021, 10:02 AM · Restricted Project, Restricted Project
steveire requested review of D98775: [AST] Add introspection support for Decls.
Mar 17 2021, 5:03 AM · Restricted Project
steveire requested review of D98774: [AST] De-duplicate empty node introspection.
Mar 17 2021, 4:40 AM · Restricted Project, Restricted Project

Mar 16 2021

steveire added a comment to D93164: [AST] Add generator for source location introspection.

I am seeing a spew of errors after 19740652c4c4329e2b9e77f96e5e31c360b4e8bb (what appears to be the latest version of this patch):

Mar 16 2021, 4:47 PM · Restricted Project, Restricted Project
steveire updated the diff for D98296: [clang-tidy] Simplify readability checks to not need ignoring* matchers.

Update

Mar 16 2021, 4:37 PM · Restricted Project, Restricted Project
steveire added inline comments to D97121: [clang-tidy] Add a Standalone diagnostics mode to clang-tidy.
Mar 16 2021, 3:45 PM · Restricted Project, Restricted Project, Restricted Project

Mar 14 2021

steveire added a comment to D93164: [AST] Add generator for source location introspection.
Mar 14 2021, 9:27 AM · Restricted Project, Restricted Project
steveire added a comment to D93164: [AST] Add generator for source location introspection.
Mar 14 2021, 9:23 AM · Restricted Project, Restricted Project
steveire added inline comments to rGd627a27d264b: [AST] Add generator for source location introspection.
Mar 14 2021, 8:49 AM
steveire added a comment to D93164: [AST] Add generator for source location introspection.

Hello. Does this work when the default target triple isn't native? This seems to be trying to compile clang sources with the just built clang - something that I don't think is always possible. I'm seeing errors like fatal error: 'cstddef' file not found, and failing to link the new IntrospectionTests, with undefined references to NodeIntrospection::GetLocations, as a full toolchain is not setup. I don't believe the buildbots are working right now, so it's difficult to see if any other systems have similar problems.

Mar 14 2021, 8:47 AM · Restricted Project, Restricted Project
steveire added a comment to D93164: [AST] Add generator for source location introspection.

A few more high-level questions:

  • What's the point of the intermediary json file? Why not generate the final c++ directly? (As far as I can tell, this wasn't discussed during the review yet)
Mar 14 2021, 8:43 AM · Restricted Project, Restricted Project
Herald added a project to D97121: [clang-tidy] Add a Standalone diagnostics mode to clang-tidy: Restricted Project.
Mar 14 2021, 7:25 AM · Restricted Project, Restricted Project, Restricted Project