[PITCH] compilerSettings: a top level statement for enabling compiler flags locally in a specific file

[ibex10]

@owenv

And something like the following seems at least somewhat aligned from a syntax perspective:

private typealias DefaultActorIsolationType = MainActor

But, that just looks like plain old code; it doesn't give any indication of the intended meaning, which is that it is providing information to the compiler.

We need special syntax.

Maybe, something like this would be clearer:

#pragma push (DefaultActorIsolationType = MainActor)
:
:
:
#pragma pop (DefaultActorIsolationType)

[FranzBusch]

@hborla

I completely agree with this. To my mind, the settings that are crucial to support per-file are:

• Default actor isolation
• Strict concurrency level
• Strict memory safety
• Warning control

I do agree with the other feedback that has been brought up regarding reducing the settings that can be applied on a per file basis. From that list though I'm still a bit worried about warning control.

Right now warnings-as-errors in the package ecosystem isn't enabled by any package since it would require an unsafeFlag which would make the package practically unusable. However, even if we could set it safely, I'm not sure modules or files should enable it. We commonly introduce new warnings in newer compiler versions. If a module/file in one of my transitive dependencies has warnings-as-errors turned on it might prohibit me from updating my compiler version or test a nightly snapshot.

Would it make sense that any warning/error control setting would only apply when the module is part of the root package/project being compiled and not when it is in a transitive dependency?

[j-f1 (Jed Fox)]

@FranzBusch

Would it make sense that any warning/error control setting would only apply when the module is part of the root package/project being compiled and not when it is in a transitive dependency?

This matches up with how clang suppresses warnings (and errors?) from “system” headers. As the user of a package/system framework you don’t have any ability to actually fix the diagnostic, so it doesn’t make sense to be bothered by it. I don’t know if it’s feasible to ignore actual compilation/type checking errors in a Swift package since you only have it in source form, but it definitely makes sense to me that warnings elevated to errors would be hidden.

[xwu (Xiaodi Wu)]

@owenv

I do wonder if default actor isolation is conceptually different enough from the other three items on this list that it might benefit from a different approach to enablement.

I agree with this perspective. From what I can see, the shortlist of features that can benefit from a per-file toggle are: (1) the default actor; and (2) opt-in diagnostics.

We have conceptualized both as "dialects" but they are different in kind in the way that @owenv lays it out. For (1), I agree a good-ol' typealias is perfectly serviceable and has precedent; and for (2), a much more lightweight opt-in incantation, something like #use strict(concurrency, safety) should be fine: I agree with the perspective that #... gives the right connotation here that it's a compile-time knob, and the dialecticization in question is a fairly new direction that can justify a sort of novel spelling.

I think @beccadax is quite on the nose that the pitched design invents a sort of faux Swift for its own sake that could be pared back or eliminated.

[ktoso (Konrad 'ktoso' Malawski 🐟🏴‍☠️)]

@xwu

I agree a good-ol' typealias is perfectly serviceable and has precedent

Especially a “Default…” typealias is what we did for Distributed and back then it was argued that this is in order to follow some prior precedent already. Though I don’t remember nor can quickly find on my phone which that other precedent was, but yes that would indeed follow existing patterns.

Currently those are per module if I remember well, but we could say that one should declare it file private perhaps… and this way control how wide it affects hmmm. I’ll do some history digging tho

[allevato (Tony Allevato)]

@ktoso

Especially a “Default…” typealias is what we did for Distributed and back then it was argued that this is in order to follow some prior precedent already.

I also raised the idea of using a top-level typealias in the pitch thread on controlling default actor isolation, based on the (albeit little used) feature where the default type of literals can be changed within a file.

IIRC, a fileprivate typealias in the literal case only affects that file, not the whole module. I didn't realize Distributed also used a top-level typealias for something like this, and that just points to even more precedent for expressing certain type-based defaults in this manner.

[scanon (Steve Canon)]

@Slava_Pestov

@scanon

In these examples, is compilerSettings actually the "first AST node"?

These days yes, because of @Douglas_Gregor's work to get rid of IfConfigDecl. We now expand such conditions while building the AST.

Glad to hear this. But, this raises an issue that "the AST" isn't a stably-defined thing as far as the actual Swift language goes, which means that we cannot appeal to it to define what's permitted in the language. It sounds like the weight of feedback on this pitch is pushing it in another direction anyway, but if we were going to keep a design like what's proposed, we would have to come up with a rule in terms of the actual surface language, rather than a compiler-internal structure.

[beccadax (Becca Royal-Gordon)]

@ktoso

• I don't think we should be focusing on the "compiler" angle of these settings (in the naming)
  • The fact that these are eventually turned into some set of flags/options passed to the compiler frontend feels somewhat insignificant, we're talking about aspects of the language and how to influence it IMHO

Quite right. I think these are language settings, basically fine-tuning the Swift language mode. (Especially if upcoming features are going to be part of this!)

@ktoso

Another alternative to think about, if we want to make sure this is distinct from "just normal imports", we already have such way to classify imports: import struct is a thing, so we could do

import feature defaultIsolation.mainActor

This may not be quite the backwards-compatibility hack you think it is—the documented grammar, the legacy parser, and the SwiftSyntax parser all recognize specific lists of existing keywords in this position, so they would all parse this code as importing the module feature and then having some junk at the end of the line which was probably supposed to be a separate statement. You might get slightly better misbehavior if you use an existing reserved keyword instead of feature, but even then, I don't think you'll get existing tools magically doing more or less the right thing with this syntax.

(Of course, there are a number of other flaws in the parsing here. For instance, actor and macro aren't supported (you have to use class for the former and an unscoped import for the latter). I also notice that the SwiftSyntax parser allows inout here for some reason, unlike the legacy parser and the documented grammar. Nor does scoped import work on nested declarations; nor does it support compound names when you use import func. Overall, scoped imports are kind of a mess…)

If we want to leverage existing import syntax, I might consider using submodule syntax on the Swift module:

import Swift.StrictMemorySafety
import Swift.StrictConcurrency.Complete

This is nice because these submodule names currently parse just fine—they're just rejected immediately by the Swift module loaders (the clang module loader supports them). As the owners of the standard library, we can decide that even if we eventually add submodules to the language, the standard library will simply avoid using any of the magic submodule names we've reserved for compiler settings.

@owenv

And something like the following seems at least somewhat aligned from a syntax perspective:

private typealias DefaultActorIsolationType = MainActor

I quite like the way this aligns with other magic typealiases. (We should probably warn about making this magic typealias public, though; unlike e.g. _ColorLiteralType, I'm not sure there are good use cases for changing the default actor isolation of your module's clients.)

@ktoso

Especially a “Default…” typealias is what we did for Distributed and back then it was argued that this is in order to follow some prior precedent already.

The pre-distributed prior art was the typealiases that can override the default types for various literals. For instance, if you have typealias StringLiteralType = StaticString as a top-level declaration, the compiler will use StaticString rather than String when the type of a string literal is ambiguous. (In fact, the reason it defaults to String is that the standard library has a public typealias StringLiteralType = String, so your typealias is just shadowing ours!)

If you didn't know about all this, I can't blame you—I don't believe TSPL covers this, and the doc comments on the standard library typealiases allude to their function very cryptically. The only clear documentation I can find for it is a single sentence towards the end of a file in the Swift repo's docs/ folder.

@scanon

if we were going to keep a design like what's proposed, we would have to come up with a rule in terms of the actual surface language, rather than a compiler-internal structure.

Agreed. It'd probably look something like:

compiler-settings-declaration, if present, must be the first statement of top-level-declaration after flattening any conditional-compilation-block directives down to their active branch's statements.

However, I'm a little dubious about the benefits of defining such a rule. I would like to see a rule which ensures that compiler settings cannot affect parsing (which would disqualify some language features from being supported in compiler settings, incidentally), and if such a rule is put in place, there is no technical reason to require the settings to be at the top of the file.

At that point it seems like the only objection is stylistic, but the ability to arbitrarily order other declarations doesn't seem to cause poor style in practice; for instance, even though import statements can technically be placed anywhere in the top-level declarations, you almost always see them grouped at the top of a file, even though this norm is enforced only by linters and disgusted comments in code reviews. I see no reason why a similar norm would not develop for compiler settings.

(But again, I actually favor not using a dedicated syntax for this. If we use import, the problem even solves itself: people will naturally group imports used to influence compiler settings at the top of the file with other imports.)

[Geordie_J (ephemer)]

I prefer the import Feature syntax or #feature(…) over the current suggestion, which feels heavyweight and foreign to me.

It was mentioned up-thread that the @_optimize attribute covers the particular case of optimization level. To be honest that’s the first time I’ve heard of this attribute, and I’ve been looking for it on and off for as long as Swift has existed (including more than one thread on these forums). Now that I know it exists I’d be happy to use it, but I do think that feature should either independently „graduate“ from being underscored (and be documented accordingly!), or be added to the list of compiler settings here.

[Nobody1707]

I'd like to second the #feature(...). syntax,

[pierremb (Pierre Monod-Broca)]

+1 on the feature, -1 on the syntax

Like others said it's particularly alluring for concurrency mode and default isolation.

I think putting it in the file is the right way to go. I have been hurt by Xcode's per-file flags with Objective-C (in the target configuration).

I would be pleased with a #strictConcurrency syntax or some kind of import. (I admit my first reaction at the import syntaxes was not very good, but I am already better inclined).

The typealias for the default isolation also surprised me, and I'm still undecided: it might be a bit too magic, but I'm not against it.

[Michael_Gottesman]

I just did a go over of the proposal taking in feedback from you all. I appreciate the time and thought that you have put into the feedback.

Here is the updated pitch: swift-evolution/proposals/0000-swift-settings.md at compiler-settings · gottesmm/swift-evolution · GitHub

(I will also update the initial post with the updated link)

[MPLewis (Mike Lewis)]

@MPLewis

Then as part of the implementation, it’d be really useful to also provide negative versions of the flags that are being supported by this directive, in case a file wants to opt out of a module-level setting like in my original comment.

Because I didn't get an explicit response to this and the updated pitch doesn't reference it, are you not planning on allowing a file to opt out of a module-level setting such as warnings-as-errors?

[Michael_Gottesman]

@MPLewis

@MPLewis

Then as part of the implementation, it’d be really useful to also provide negative versions of the flags that are being supported by this directive, in case a file wants to opt out of a module-level setting like in my original comment.

Because I didn't get an explicit response to this and the updated pitch doesn't reference it, are you not planning on allowing a file to opt out of a module-level setting such as warnings-as-errors?

To me that is more of a future directions sort of thing. I am trying to keep this proposal focused.

[allevato (Tony Allevato)]

The current construction allows (syntactically) for settings to be listed multiple times. What are the semantics in that case? It may depend on the specific setting. For example, the following seems like a reasonable thing to want to write:

#SwiftSettings(
  .warningsAsErrors(.concurrency),
  .warningsAsErrors(.deprecatedDeclarations),
)

But strict-concurrency and default-isolation don't make sense to specify multiple times, so it would either have to be treated as an error, or decide on a policy like last-one-wins.

Should we require instead that each setting can only be specified once, and if it makes sense for a particular setting to have multiple values, it should take them as a variadic list instead? That would make the signature for warningsAsErrors into (_ diagnosticGroups: DiagnosticGroup...), for example.

Are there any use cases where this would fall apart?

[Michael_Gottesman]

My initial thought was to just keep things simple and make it such that the behavior was customized to either last one wins or appending (in the case of warningsAsErrors).

But I think you bring up a good point. I would be fine with using warningsAsErrors as a variadic parameter and requiring things to only be passed in once. I think that that provides a simpler model that doesn't have sort of an implicit is it appending or set once. Instead it is specified simply via the type system.