[Second Review] SE-0478: File-level defaults

[aviva (Aviva Ruben)]

On composition

@OneSadCookie

The spec doesn't seem to say what happens if you apply default @MainActor to a file in a MainActor-by-default module (which dialect do you get, or is it an error?) nor default nonisolated to a nonisolated-by-default module.

In a lot of cases it would "do nothing", but they compose. The module feature performs inference, which this feature does not. For example, in MainActor-by-default mode, conforming to a nonisolated protocol would make your conformance nonisolated. If you write that conformance in a file with default @MainActor, then it is as though @MainActor was written on the conformance, overriding the inference rule and potentially leading to an error (which alerts you that you wrote something impossible). In Swift 5 + MainActor-by-default mode, default @MainActor would apply @MainActor without @preconcurrency, while the module default would have @preconcurrency. Note that this is the same behavior as writing the annotation explicitly in this configuration.

@OneSadCookie

Since this is as if the user typed it, it "wins" the competition with the flag, but doesn't necessarily cancel out all the behaviors of the flag?

If you are aware of behavior this wouldn't cancel, I'm extremely interested to hear it! My intention and belief is that it would cancel out all the behaviors of the flag besides the other feature flags (NonisolatedNonsendingByDefault and NoExplicitNonisolated) that MainActor-by-default enables (turning off other associated feature flags per file would cause issues for adoption and unadoption, as well as being extremely surprising in my opinion).

[Second Review] SE-0478: File-level defaults - #17 by xwu looks correct, that reflects my intention with the proposal, and divergence from this in the implementation would be a bug.

Should we allow default anywhere in a file?

@John_McCall

Is there any chance we would want defaults to be overridable in the future as you proceed through the file? That would be one reason to pin them to the top for now.

I'm not confident we can rule this out, so it seems to have some (minimal) value to leave this door open. If we discovered a need to do this in the future but allow default anywhere in the file, we would need to invent new syntax, or conditionalize the behavior of default declarations based on their contents. However I'm personally opposed to a #pragma style feature that can be toggled throughout the file, since I feel it makes local reasoning harder.

@allevato

My gut feeling is that if you need multiple sections of defaults in the same file, any of the alternatives would be better than doing that:

• Move each section of defaults to its own file
• Drop default and just annotate the declarations directly

I agree! Swift is very amenable to splitting things between files, so I see no issue with this.

@beccadax

I’m sorry, this seems like a problem for linters, not language rules. Writing import in the middle of a file is ambiguous in precisely the same way (does the import affect the entire file, or just the declarations below it?), but we don’t have a rule forbidding it. I think Swift developers have internalized by now that Swift mostly doesn’t care about the order of declarations but that writing these sorts of file-wide meta-declarations first is good style. They don’t need the compiler to force them to do it.

@j-f1

+1 to this. Consider this example of how the proposed limitation still allows for confusing code:

import Foundation // should this import be allowed?

default @diagnose(ImportingModulesStartingWithTheLetterF, as: error)

Since I assume the diagnostic would apply to the whole file, this is really no different than allowing the file-level diagnostic to be placed after a regular declaration.

What is the argument for allowing a file default to be written anywhere? We can support a fix-it for moving the default declaration to the top of the file. I would even prefer it was not allowed after import, but I was told that would be onerous. In my mind, requiring file defaults to be written at the top of the file means "is there a default somewhere else" is never a question you need to wonder about when debugging, even when you are using a plain text editor. When I open a file, I see what defaults it has right away. For IDE features like jumping to definition or PR review this is less applicable... Yes, the compiler can understand it anywhere, but why is it ever desirable to allow writing a file level default in the middle of a file? My position/bet is that no programmer would feel hindered by needing to write file level defaults at the top of the file. If we discover that is wrong, we can always lift this restriction later, but we can never introduce it without staging and migration. Is it good that import statements can be written anywhere, beyond empowering more confusing code?

@allevato

We allow this:

let x: Foo = 5

// 10,000 lines of additional code

typealias Foo = Int

Is this desirable? Maybe the consensus is that Swift is already like this, so there's no reason to try to prevent it for new features?

I don't think forbidding file level defaults below the top of the file is necessary, but I think it's nice.

On language dialects

@OneSadCookie

And splitting of all Swift code into multiple dialects (default MainActor, NonisolatedNonsendingByDefault)

NonisolatedNonsendingByDefault and NoExplicitNonisolated are not intended to remain as dialects, there just needs to be staging and care there to not break source compatibility. See SE-0461 and this reply from Holly. -default-isolation is a permanent dialect, which I personally think is slightly unfortunate.

However I'd argue file-level default isolation is not a dialect; it's something that can be freely adopted and unadopted per-file!

IDE experience

@Jon_Shier

Additionally, when this is added, for the love of god, please, please, please, please add this info to the appropriate tools (LSP, whatever Xcode uses) so this can be visualized in source editors. I should be able to hold (or toggle) a button and see the defaults that are currently active on the declarations I'm looking at.

This should be the case with my current PR! File level default isolation is treated as another kind of inferred isolation (with the rule of applying to every top level declaration lacking an annotation), applied after explicit attributes but before other inference rules, so any editor that is able to show the type of a decl with an isolation that wasn't explicitly written (via an inlay hint, hover to show type, color coding, whatever your editor does already) should apply to file defaults. I personally agree it would be extremely valuable for Xcode to show inferred isolation as an inlay hint if it doesn't already.

@allevato

The test suite should include a bunch of different orders, even if they aren't something that a real developer would type, to make sure that they all behave identically so that we have confidence that defaults can be moved around by tooling.

The only order sensitivity is priority. No rules exist today prohibiting repeated @available or @diagnose on a declaration, and the order determines which wins. Which default @available wins is determined by order in the same way:

default @available(*, deprecated, message: "first default")
default @available(*, deprecated, message: "second default")

// CHECK:      @available(*, deprecated, message: "first default")
// CHECK-NEXT: @available(*, deprecated, message: "second default")
// CHECK-NEXT: public func defaultedOrderedFunc()
public func defaultedOrderedFunc() {}

// ...

func useOrdering() {
  defaultedOrderedFunc() // expected-warning {{'defaultedOrderedFunc()' is deprecated: second default}}
}

We can add a warning for this case if we add a warning for repeated available attributes more generally.

On inference, and being different from -default-isolation

@OneSadCookie

If we accept the idea that default @MainActor is strictly better than -default-isolation MainActor

They are very different features! The module default is a language dialect that tries its best to infer the intention of code without annotations and get it to compile. It is intended to make it easier to write mostly single-threaded things, and even if you feel it doesn't achieve this, it certainly does better than file-level default isolation at this purpose. A file-level default isolation, as proposed, attempts to apply your specified default to every single declaration where it is coherent, and give you an error when it causes issues. This would be a very poor experience for writing single threaded code that wants to interact with external functions and protocols. These two cases not being the "same kind" of default personally biases me back to using.

Do we need this?

@goose

I do have general hesitations about the value proposition of a feature like this. Something that I heard a long time ago that stuck with me is "Code is written once but read thousands of times," with the idea being that we should optimize for code readability.

While it doesn't exonerate a new feature, Swift is full of features that can be used to write both clean and illegible code. Powerful but risky features like operator overloading are eschewed in other languages but present in Swift, and it comes down to taste and convention to use them well.

I think the strongest argument for a feature like this regarding readability is highlighting the novel case; in a file where all but one declaration is nonisolated, the repeated nonisolated keyword becomes noise. Using default nonisolated, the one @MainActor declaration is highlighted. If something is actually boilerplate, it can improve readability to eliminate it. In a case like @unsafe, littering it throughout the file might instead be a smell and not something to eliminate.

I feel this is another tool in the toolbox for writing clean code. Programmers can use it where it is appropriate, and be explicit when that is more readable--just like other features that can hide information, like type inference! I see it being used primarily by -default-isolation MainActor projects for a couple files that implement concurrent types and algorithms, to isolate a file of UI state to @MainActor, and to be explicit about assumed inferred isolation.

Existing typealias based defaults

@xwu

It occurred to me as a shower thought that one future direction which could be a great use case for the general feature would be floating-point behavior.

I think it would be great to create attribute forms of the existing typealias configurations, and support them as file-level defaults! I forgot to mention distributed actors, but they could potentially also benefit from treatment like that in the future (if we defined an attribute syntax for them).

My feelings on syntax

@allevato

I'm quite happy with the use of default here. It uses an existing keyword in a way that makes sense for the feature and doesn't require the trickiness of parsing a contextual keyword.

I agree that it's clean! I was more excited about it when the pitch had file-level defaults and module-level defaults behave in the same way, and I do wonder if using or similar is worth consideration given that they are not the same "kind" of default. While no syntax choice will be fully explanatory without being overly verbose (file-level-apply-to-all-decls-default-isolation @MainActor) choosing using could make it clear that default isolation does something different from the language default. It is worth noting that we can implement using without any source compatibility issues since you can't write top level code like this proposal uses today. If we want using { ... } or using( ... ) in the future I suspect we will run into a bit of trouble.