[SE-0039] [SR-3504] Require object literal arguments to be literals without interpolation

[xwu]

SE-0039 specified the following grammar for an image literal:

image-literal → #imageLiteral(resourceName: image-resource-name)
image-resource-name → static-string-literal referring to image resource name

However, as originally implemented, any variable of type String could be used for an image literal. This conflicts with the notion of a literal and is not in line with the approved design outlined in the proposal. This PR modifies the signature of the initializer in the underscored image and file literal protocols to require the resource name to be of type StaticString has the parser ensure that all arguments to object literals are themselves literal expressions, and that they are not interpolated string literals.

This change is potentially source-breaking. However, as the current implementation differs from the approved design, this may be judged permissible under the following rule for source compatibility: "If a bug fix breaks source in a fringe case as determined by a scan of existing source, the benefit might outweigh the cost and it could be allowed."

(Note that AppKit NSImageName* constants are bridged as Strings, which is one plausible scenario in which end users' existing code may rely on existing behavior. However, the value for each constant is guaranteed by documentation in NSImage.h to be the "same as the constant name without the 'ImageName' part" and this use case is therefore trivial to migrate.)

Resolves SR-3504.
Requires apple/swift-corelibs-foundation#757 to be simultaneously committed for Linux.

[xwu]

@tkremenek As the original implementor of SE-0039, is this the right approach to addressing the discrepancy? I did notice that file and image literals have accepted plain ol' Strings since before the implementation of the last proposal. Wondering if there are wider reasons for that.

[tkremenek]

@xwu This seems like an funcational approach, but the user experience here isn't great. We get a type system error, saying that a String can be accepted and the user must use a StaticString. That's clinically correct, but a better error message would really tell the user what they did wrong. A message that said something along the lines of "you must use a string literal" would help guide the user to the right solution. Since this is really a fix to have the compiler's behavior match the grammar, can we possibly fix this in the parser instead of make it a hack fixed by the type system? That would allow the issue to be caught much earlier and with a better diagnostic.

As for why we accepted plain old String before—I don't know anymore, but I don't think it matters. The intent here, per SE-0039, is to just accept String literals.

[xwu]

@tkremenek I think this is what you had in mind, yes? As a side benefit, #colorLiteral will now require literal numbers as well.

[tkremenek]

Can these kind of indentation changes go in a different commit? These have nothing to do with the change proposed in the pull request.

[tkremenek]

Can we make this error being conditional on Swift 4 mode, leaving this a warning in Swift 3 compatibility mode? That would give people a heads up that this behavior is going away without it being a hard break.

[tkremenek]

@xwu This is more what I had in mind. Can we make this an error in Swift 4 mode, and a warning in Swift 3 mode?

[tkremenek]

@xwu This is looking great. Let me discuss this with other members of the Core Team next week (after most of us return from a holiday break) to make sure we're all OK with this source-breaking change. Gating this under the Swift 4 version makes me personally comfortable with doing this.

[tkremenek]

@xwu Thanks for your patience. I discussed this with the Core Team, and after a round of discussion we decided to leave the current behavior in the compiler. While the restriction enforced by the change in the pull request (and in the original proposal) is all that the tools that use this feature (e.g., Playgrounds) generate today there is no real harm with leaving this generalization in there and leaves the option open for tools to make use of this generalization in the future. Thank you for working on this, but I think we'll end up not making this change. We should go back and amend the original proposal.