Landing SE-0276 implementation

I've been trying to land the multi-pattern catch implementation from SE-0276 for awhile (https://github.com/apple/swift/pull/27776), but I've had trouble getting it reviewed since it involves fairly large AST changes that introduce frequent merge conflicts. Considering the 5.3 branch cut is coming up soon, I was hoping to get some advice on the best way to go about integrating these changes.

The best alternative to the monolithic patch I've been able to come up with so far is splitting out the swift-syntax and parser changes, and integrate those with the old AST first, diagnosing multiple clauses as parseable but not yet supported. Those changes have already been reviewed and should be good to go, but it would be less than ideal if 5.3 swift-syntax APIs gave the mistaken impression that this feature was supported, and those only account for ~10% of changes.

I wanted to see if anyone had advice on handling cross-cutting AST changes like this that require lots of updates across Parse/Sema/SILGen/LLDB.

3 Likes

I highly recommend splitting it up, and perhaps including the commit hashes of the previous commits in the commit message in the later commits (so one can follow the chain if need be).

Having to juggle change across 2 repos is bad enough, 3 repos is much worse. IMO you should avoid it unless absolutely necessary. Let's say something goes wrong after you merge the changes, which requires the commits to be reverted. The coordinated revert will be a lot more work. Instead, if you break up the changes and stagger them by a few days, it is much easier to pinpoint which changes need to be reverted in case something needs to be reverted.

Wrt the branch cut and the swift-syntax APIs, I'm not entirely sure -- hoping someone else can chime in.

1 Like

Sorry for the delay here. Rintaro reviewed all of the parsing-related changes, and I've reviewed the rest. There is a little cleanup we can do, but at this point I think the best way forward is to go ahead and merge. I've merged the main PR and the SwiftSyntax changes; the LLDB changes are going through PR testing right now. Thank you!

Doug

2 Likes

Thank you!

Terms of Service

Privacy Policy

Cookie Policy