Hmm, the tests pass locally for me. I'll have to look into that more.

I figured it out. I was using the released CLI, but #17941 changes the extractor and hence the test results, so I needed to build and use the go extractor locally.

This is ready to review now.

Note that getFieldCand uses hasEmbeddedField, which is defined using hasFieldCand. So it seems independent, but actually it isn't.

That's true, but for one layer we'll miss named pointer types AFAICT:

    exists(Field f | this.hasFieldCand(_, f, depth, true) |
      tp = f.getType() or
      tp = f.getType().(PointerType).getBaseType()
    )

That means getFieldCand -> hasEmbeddedField will miss the case where there's an embed of type Named which is defined as *MyEmbeddedStruct. For deeper embeddings it then uses hasFieldCand -> getFieldOfEmbedded, so this will only affect depth-1 embeddings.

I wrote a test for what you're describing.

type BazPointer *Baz;

type EmbedsBazPointer struct {
	BazPointer
}

It gave me a syntax error: "embedded field type cannot be a pointercompiler" and this link: InvalidPtrEmbed. The relevant part of the spec says:

An embedded field must be specified as a type name T or as a pointer to a non-interface type name *T, and T itself may not be a pointer type. The unqualified type name acts as the field name.

So I think it's okay that the new code-path doesn't exactly do what the old one did, because the missing case is for invalid code.

OK great. That actually suggests then to simplify getFieldOfEmbedded since that does a getUnderlyingType jump prior to checking for a pointer type that in fact can't happen.

I ran DCA before the simplification and it was fine. I've started another one. Clearly that code had been inefficient before, because it had a pragma. Hopefully with the simplification which means it can use lookThroughPointerType it will be optimized well without any guidance this time.

The second DCA seems to show a 2% speed up on the two biggest dbs. I should probably run QA in case there are any pathological cases.

I messed up the QA run by using a badly chosen nightly base, so the results are dominated by the results for #17494. From what I can see, there aren't any alert changes in queries which aren't affected by that PR, and there aren't any noticeable slowdowns. I think I will run another QA though, just to be sure.

I ran another QA and the results are completely uneventful. If anything, it sped things up (but it's probably just noise). And no alert changes. @smowton This is ready to re-review.