[Parser] Fix String interpolation accepts invalid argument label syntax without expression

[kntkymt]

Description

resolves: #80927

The interpolated string "(1, f:)" compiles without error, despite f: being an argument label with no following expression.

print("\(10,f:)") // 10

It seems to be regression of SE-0439 Allow trailing comma in comma-separated lists. #74522.

Approach

In this PR, I fixed the bug by aligning the approach for trailing comma in StringInterpolation with the approach in other contexts.

When we think about valid case: "\(10,)", In #74522, parseListItem returns ParseListItemResult::Continue when parser found right paren with trailing comma ,) in StringInterpolation then parseExprPrimary (triggered by the next iteration of parseListItem) will parse ) next to "\(10,. Therefore, we wrote the specific logic for trailing comma in String Interpolation at parseExprPrimary in order to enable trailing comma in String Interpolation.

// Enable trailing comma in string literal interpolation
// Note that 'Tok.is(tok::r_paren)' is not used because the text is ")\"\n" but the kind is actualy 'eof'
if (Tok.is(tok::eof) && Tok.getText() == ")") {
  return nullptr;
}

However, this logic has problem that incorrectly allow the pattern invalid argument label syntax without expression: \(10, f:) (since tokens f: are consumed as optional before we call parseExprPrimary.)

Regarding the approach for trailing comma in other than String Interpolation such as function call,parseListItem returns ParseListItemResult::Finished when parser found right paren with trailing comma ,).. in this case we don't have to write additional logic in parseExprPrimary and correctly emit error on the pattern: \(1, f:) as well. (parseExprPrimary originally correctly emit error on that pattern without the specific logic for trailing comma in String Interpolation)

if (Tok.isNot(RightK))
      return ParseListItemResult::Continue;

..omitted

return ParseListItemResult::Finished;

I this PR, to align the approach for trailing comma, parseListItem returns ParseListItemResult::FinishedInStringInterpolation when parser found right paren with trailing comma ,) in String Interpolation.

Alternative Considered

Add additional validation after calling parseExpr (and subsequent parseExprPrimary).

swift/lib/Parse/ParseExpr.cpp

Lines 3249 to 3253 in f31c89a

	} else {
	auto ParsedSubExpr = parseExpr(diag::expected_expr_in_expr_list);
	SubExpr = ParsedSubExpr.getPtrOrNull();
	Status = ParsedSubExpr;
	}

else {
    auto ParsedSubExpr = parseExpr(diag::expected_expr_in_expr_list);
    SubExpr = ParsedSubExpr.getPtrOrNull();
    Status = ParsedSubExpr;
    
    // added. if we have a label `f:`and its expression is empty, emit diagnosis.
    if ((Tok.is(tok::eof) && Tok.getText() == ")") && !SubExpr && FieldName.nonempty()) {
      diagnose(Tok, diag::expected_expr_in_expr_list);
    }
  }

It's a good thing that the change is purely additive, but in the first place, writing StringInterpolation-specific logic directly in parseExpr may harm scalability, considering the possibility of future grammar changes and modifications to parseExpr. Ensuring scalability in the long term requires aligning the approach for StringInterpolation with that of other contexts such as function calls..

[omochi]

@swift-ci please smoke test

[CodaFi]

Unrelated reflection test failed

@swift-ci smoke test macOS platform

[rintaro]

Thank you!

[rintaro]

I believe it's worth to cherry-pick this to 6.2 release. @kntkymt Could you open a PR for release/6.2?

[kntkymt]

@rintaro Thanks for the approve!

Could you open a PR for release/6.2?

sure!

[kntkymt]

@rintaro here is the cherry-pick PR!

#80950