Revisiting the source compatibility impact of SE-0274: Concise magic file names

Hi all,

In light of the discussion of principles for (trailing closure) evolution proposals and based on feedback we've received from the Swift 5.3 development snapshots, I'd like to revisit SE-0274: Concise magic file names. In particular, I feel like SE-0247 has violated the "Source Compatibility" principle more than is necessary, and that we should consider revising the proposal.

SE-0247 includes an additive change (#filePath literal to provide the full path to the file), but then introduces three changes that affect existing sources:

  1. #file literals have a different form that does not include the full path name. Therefore, the spelling #file has changed meaning.
  2. The compiler provides a warning when a wrapper around a #filePath -defaulting function passes it #file instead, or vice versa.
  3. Standard library functions like precondition currently use #file, and therefore have had their behavior changed to no longer produce the full path name by default.

My concern is with #1 and #2. With #1, we have changed language semantics without a language version bump and without a deprecation cycle. With #2, we are seeing a bunch of warnings in user code about the migration. In both cases, if you want to fix your code to work with Swift 5.2 and Swift 5.3, you have to use #if's on language version in a bunch of places, which is an unfortunate workaround because it litters code with unnecessary divergence.

I consider #3 to be a necessary semantic change. As the proposal highlights, the standard library is effectively leaking private information about the build environment into binaries, and paying a significant percentage of the code size to do so.

Here is my proposal for fixing these issues with SE-0247:

  • Introduce #fileId to have the shortened spelling
  • Update all of the Standard Library's usage of #file to #fileId
  • Keep #file meaning the same thing as #filePath in Swift 5.3
  • Change the meaning of #file to be an alias of #fileId in Swift 6

Thoughts?

Doug

(Note: I advocated for SE-0247 as it was accepted, including the source breaks above, but I've been thinking about that decision more in light of the other discussions going on.)

15 Likes

I agree that we should havenโ€™t to use โ€˜#ifโ€™s to support different language versions, which are not major. One thing I didnโ€™t understand is whether you propose having both #filePath and #file or just #file. As far is the deprecation goes, I agree that the current behavior is actively harmful and I support changing it as soon as posting -in the next major version.

Since it's common to put #file in default argument expressions, is it even possible to if uses without putting your whole function in the if body?

Isn't this the status quo as far as runtime behaviour? I thought only the compiler warnings were enabled by default in 5.3. For example [SE-0274] Enable concise #file by default by beccadax ยท Pull Request #30959 ยท apple/swift ยท GitHub is still open.

Anyway, I'm supportive of doing this migration as slowly as needed to make it painless.

Thank you SO MUCH! I agree with everything you say, I'm fully on board with this pitch as well as the #fileId idea. If you asked me to choose a name, I'd probably go for #fileBasename or #fileID but I'd take anything to make this issue go away :slight_smile:.

As you point out it breaks source compatibility more than necessary. Worst of all actually is that the biggest #file vs. #filePath problems happen in test assertions. The problem with those is that under normal circumstances, they don't actually print anything. So a potential #file to #filePath forwarding issue will potentially be hidden until the worst point in time: when your test assertion fails, printing the wrong #filePath (this literally happened to me today...).

Below a few bugs and pull requests we made until we discovered that neither the first, nor the second idea how to forward #file to #filePath warning free worked, which then meant we needed a third one which actually works :slight_smile:.

2 Likes

My understanding is that we are still in the midst of discussing these principles as a community; I'm somewhat surprised to see that they're being adopted already and particularly in the context of revisiting existing proposals.

I do think that this is a reasonable migration path. For consistency, it'd probably best be spelled #fileID or #fileString, but besides bikeshedding I would agree with this revision.

9 Likes

Thanks so much @johannesweiss for digging deep into this one... The [SR-12936] create new {{-swift-version}} BEFORE we do {{#file}} != {{#filePath}} ยท Issue #55382 ยท apple/swift ยท GitHub makes it indeed a bit insane to deal with. I also missed this during skimming the initial pitch :frowning_face: Proposed way forward I'll avoid bikeshedding the name of the alternave #file... but @xwu's suggestions sound good.

What about the warnings, we'd want to not issue them anyway even if a #file is forwarded to what has a default #filePath right? (So: allow #file to #filePath forwarding without warnings by weissi ยท Pull Request #32445 ยท apple/swift ยท GitHub)

Do we genuinely want to have #fileId (or #fileID?) in the language after #fileโ€™s behavior changes? If not, we could tie #fileโ€˜s behavior to the language version mode and either add an underscored #_fileID and use it only in the standard library, or simply wait until Swift 6 lands for the standard libraryโ€™s behavior to change (and have it change only for clients compiled in Swift 6 mode).

3 Likes

One advantage of full paths is itโ€™s easier for editor clients to map run-time errors back to files. Changing the behavior of fatalError and precondition is definitely a sensible change (and maybe even assert), but something like XCTest benefits from using the full path.

2 Likes

I feel like what #fileID does is valuable and should be made available more generally. Now, we could say it only exists in Swift < 6, and deprecate it in favor of the new #file behavior in Swift 6, so the result is back down to 2 ideas, with the shorter one being the preferred one for most tasks.

2 Likes

Nope, you have to duplicate the body! Thatโ€™s part of the reason the user impact here is so high.

These principles arenโ€™t new to Swift; that they are being written down is new. As is the feedback weโ€™re getting from folks trying out the 5.3 development snapshots.

1 Like

Are other names on the table instead of fileID, such as fileName or something more expressive?

1 Like

Would it help if #fileID uses inode in some way?

That would break the goal of reproducible builds. And possibly cause problems with network drives.

2 Likes

Sure; I just picked #fileId out of a hat. The string has both the module name and the file name in it, so the name should reflect that.

Doug

If the name is just an artifact of the transition period (and I hope it is), we shouldn't sweat it too much.

2 Likes

Just for information,

GRDB + Xcode 12 beta is littered with warning: parameter 'file' with default argument '#file' passed to parameter 'file', whose default argument is '#filePath'.

In order to avoid code duplication, the workaround I'm currently using is:

private func _assertStuff(..., file: StaticString, line: UInt) {
    XCTAssert(..., file: file, line: line)
}
#if compiler(>=5.3)
func assertStuff(..., file: StaticString = #filePath, line: UInt = #line) {
    _assertStuff(..., file: file, line: line)
}
#else
func assertStuff(..., file: StaticString = #file, line: UInt = #line) {
    _assertStuff(..., file: file, line: line)
}
#endif

The worse side effect of this workaround is that it erases default arguments. Consequence: it not only prevents the quoted warning, but also all future compiler warnings of the same kind.

+1 to this proposal, thanks @Douglas_Gregor.

Yeah that's one workaround. Unfortunately it requires the entire signature to be duplicated three times. This makes me wish we could do something like this in Swift:

func foo(
    #if compiler(>=5.3)
    file: String = #filePath
    #else
    file: String = #file
    #endif
)
2 Likes

I've written up a proposal for this change. To summarize:

  • #fileID will generate the concise file string in current Swift; the standard library assertions will adopt it, and other Swift 5 code can adopt it too.
  • #file's meaning will only change in "Swift 6 mode" (whatever that ends up being). At that time, we will deprecate #fileID.
  • The mismatched-default-argument warning will consider #file interchangeable with both #filePath and #fileID in Swift 5 mode; in "Swift 6 mode", it will complain about #filePath but not #fileID.

I'll start implementing this today.

13 Likes

Thank you for highlighting these issues from the original proposal discussion once more, @Douglas_Gregor. It utterly escapes me why the proposal was approved in the form it's in. Please make the changes you have proposed to introduce #fileId.