Change diagnostic error thrown for when string interpolations aren't closed by a parenthesis

[NSAntoine]

This pull request changes the diagnostic error thrown when a string interpolation isn't closed by a parenthesis properly (ie: let x = "g \(")

Previously, it would blurt out unterminated string literal, with these changes it'll instead throw expected ')' at end of string interpolation

[NSAntoine]

@hamishknight could you ask swift-ci to test this? once the test does succeed, ill add notes and fix-its, but i want to make sure this works first

[hamishknight]

@swift-ci please test

[NSAntoine]

CI down? nevermind

[NSAntoine]

@hamishknight

[hamishknight]

@swift-ci please test

[NSAntoine]

@hamishknight okay this one should 100% work

[hamishknight]

@swift-ci please test

[NSAntoine]

Great! Now time to add the notes and fix its once I get home

[NSAntoine]

@hamishknight

[hamishknight]

@swift-ci please test

[NSAntoine]

@hamishknight ninja: build stopped: subcommand failed. -- Warning: {} Host toolchain could not locate a compiler to build swift-driver.
Uhh.. can’t really be caused by us? Can it? It didn’t even get an opportunity to build the driver, so I assume it wasn’t even able to build swift itself

[hamishknight]

Looks like a legit error:

/Users/ec2-user/jenkins/workspace/swift-PR-macos/branch-main/swift/lib/Parse/Lexer.cpp:1896:13: error: expected expression
            .fixItInsert(Lexer::getSourceLoc(CurPtr), ")");
            ^

Usually it's good to search the CI logs for occurrences of error: , they can sometimes appear quite a bit further up the log. Jenkins is supposed to be able to detect things like this, but I guess not in this case.

[NSAntoine]

Ohh, I see, I’ll fix just a sec

[NSAntoine]

@hamishknight fixed, and I’ll take note of that, thanks!

[hamishknight]

@swift-ci please test

[NSAntoine]

@hamishknight this time could you test and ask it for a toolchain? I'm really sorry for all the trouble, but I can't compile locally unfortunately

[hamishknight]

@swift-ci please test

[hamishknight]

@swift-ci please build toolchain

[NSAntoine]

@hamishknight Okay this should finally be it.. also, when it builds the toolchain, does it put the toolchain online for download?

[hamishknight]

@swift-ci please test

[AnthonyLatsis]

If we've settled on the error also pointing to the opening paren, then let us merge the note into the error. I'm thinking cannot find ')' to match this opening '(' in string interpolation or similar.

[NSAntoine]

Sure!

[NSAntoine]

@AnthonyLatsis ^

[AnthonyLatsis]

@swift-ci please build toolchain macOS

[NSAntoine]

@AnthonyLatsis fixed the issues

[AnthonyLatsis]

@swift-ci please build toolchain macOS

[NSAntoine]

@AnthonyLatsis

Command Output (stderr):
--
SIL verification failed: struct operand type does not match field type
  $Builtin.Int64
  $Builtin.Int32
Verifying instruction:
     %33 = integer_literal $Builtin.Int64, 714    // user: %34
->   %34 = struct $UInt (%33 : $Builtin.Int64)    // user: %102
     %102 = apply %101(%89, %25, %32, %34, %100) : $@convention(thin) (StaticString, StaticString, StaticString, UInt, UInt32) -> Never

This literally can't be related to us, can it?

[AnthonyLatsis]

@swift-ci please build toolchain macOS

[NSAntoine]

@AnthonyLatsis this is so confusing.. I don’t think the SIL Error has anything to do with us though, could you just ask it to test only?

[AnthonyLatsis]

@swift-ci please smoke test macOS

[NSAntoine]

@AnthonyLatsis could you ask it to build the toolchain again now?

[AnthonyLatsis]

I will run validation tests first just in case. CI infographics indicate that the failure is non-deterministic, but the failure itself looks like it should be deterministic, which is a bit weird.

@swift-ci please test macOS

[AnthonyLatsis]

@eeckstein Could you take a look at this unrelated test failure?

https://ci.swift.org/job/swift-PR-toolchain-macos/164/console

[eeckstein]

Failed Tests (1):
  Swift-validation(watchsimulator-i386) :: SILOptimizer/hoist_destroy_addr.sil

@nate-chandler is this a fallout from the SSA destroy hoisting change (#58791)?

[nate-chandler]

Already fixed via #59048 .

[nate-chandler]

@swift-ci please build toolchain macOS

[AnthonyLatsis]

Already fixed via #59048 .

Ah, now it makes sense — I was looking at the hoist_destroy_addr.sil in test, not validation-test. Thank you!

[NSAntoine]

@AnthonyLatsis works locally! Ready for merge now?

[AnthonyLatsis]

We need a representative to review these changes first.

[AnthonyLatsis]

My only concern is that there is no discerning between an unterminated string inside the interpolation and a missing ), we just diagnose both issues unconditionally. We'd have to get more involved to make a difference though. Let's see what Pavel has to say.

[xedin]

Thank you all for the collaborative work on this one! Looks good to me! I have left a minor comment about the diagnostic itself inline. I'd also say that since "unterminated string literal" is not really a regression it could be improved separately.

[AnthonyLatsis]

@swift-ci please smoke test

[NSAntoine]

@AnthonyLatsis tests passed; time for merge?