Fix implicit UnresolvedMemberExpr crash

[beccadax]

The isImplicit() flag on an UnresolvedMemberExpr was not carried over to the CallExpr node generated after type checking, which could cause later stages of compilation to use invalid SourceLocs. This change correctly propagates the flag from the UnresolvedMemberExpr to the CallExpr.

(I'm not aware of any way the compiler might currently malfunction due to this bug, but it was happening in new code I was writing.)

[xedin]

Thanks! Could you please add a test to validation-test/compiler_crashers_2_fixed to make sure that it doesn't regress in the future?

[beccadax]

@xedin I took a look at this, and I don't think there's actually a way to exercise this bug currently. The four places where the compiler creates an UnresolvedMemberExpr all set Implicit to false. The bug caused a crash in modifications I'm making to the compiler, but if my new code gets merged at all, it'll need to wait for an evolution proposal.

So unless there's some hidden way to mark a node as Implicit, or to write a test case that generates an arbitrary AST, I don't know how to write a regression test for this issue.

[xedin]

It does make sense for me in theory - if the argument to unresolved member is implicit we want to make the call implicit as well, but I'm just having a hard time trying to imagine how this is going to be expressed syntactically, as I see it right now, there is no real reason to have implicit flag for the unresolved member at all...

It seems like this doesn't do any harm if we merge, I just want to understand what is really going on here...

[xedin]

@swift-ci please test

[xedin]

@swift-ci please test source compatibility

[beccadax]

In my case, I'm synthesizing an initializer call very early, before I know what type it will be used on, and the easiest way to have the compiler infer the type and insert it into the call for me is to create an implicit UnresolvedMemberExpr. I might change my implementation later so that it doesn't create the call until after the type is inferred, but even if I do, we'll have the same issue if we eventually add some synthesis that needs to do this.

If we don't want to support implicit UnresolvedMemberExprs turning into CallExprs, we should add an assertion that trips when you try to do it—preferably as soon as you try to create the node. But if we're not going to forbid it, I think it's pretty clear what the code ought to do, because what we're doing right now reliably builds ASTs that break our invariants.

[xedin]

Ok, sounds reasonable to me!