FYI - there's nothing "anti-inclusive" about East/West.

Making a separate tool for this makes no sense. Especially as you are only proposing it to satisfy one (or are there more?) vocal objector.

In D69764#2867109, @MyDeveloperDay wrote:

In D69764#2863648, @owenpan wrote:

Has this been tested against a large code base? It also needs an unqualified LGTM before it can be merged.

D105701: [clang-format] test revision (NOT FOR COMMIT) to demonstrate east/west const fixer capability demonstrates transforming clang-format itself to east const.

Actually transformation of the whole of the clang subfolder is actually holding up pretty well. I'm not seeing an violations (not sure if I transformed all the files) but certainly so much so that creating a review that covered all of it was way too big.

Testing on a large code base can be hard especially one as large as LLVM where its not currently fully clang-formatted in the first place.

Of course the lit tests get mangled as the test code gets swapped but the //CHECK-FIXES doesn't

Like I mentioned before, by gut feeling is that this option is MOST useful in preventing violation to your current style from creeping in than going to the extreme of transforming a whole project from east to west or vice versa.

In D69764#2863266, @aaron.ballman wrote:

In D69764#2863213, @steveire wrote:

@MyDeveloperDay Does anything prevent this being merged, instead of just rebased?

Please see my comments from https://reviews.llvm.org/D69764#2533538.

@MyDeveloperDay Does anything prevent this being merged, instead of just rebased?

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.

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.

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

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).

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.

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

In D101572#2730012, @SilensAngelusNex wrote:

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

According to

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

Fix windows build

LGTM!

Update test

Update

Update

Update

Update

In D98774#2701853, @thakis wrote:

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

@aaron.ballman ping?

Update

Update

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

Great, thanks!

Update

Update

Update

Thanks!

Update

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

Update

Update

Update

Update

Add locations for CXXBaseSpecifier

@njames93 ping?

In D69218#2654638, @nick wrote:

In D69218#2654614, @steveire wrote:

@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

Yes, please remove the line instead.

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

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

Rebase

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

LGTM, thanks!

In D99451#2654218, @alexreinking wrote:

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.

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

Does anything prevent this going in now?

Update

Update

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

In D98827#2635303, @njames93 wrote:

Are there any tests that ensure something is always outputted?

Update

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

In D98774#2631898, @thakis wrote:

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?

Update

In D93164#2630203, @nathanchance wrote:

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

Update