Reviews are an important part of the Swift evolution process. All review feedback should be either on this forum thread or, if you would like to keep your feedback private, directly to me as the review manager, either via email or the forum messaging feature. When contacting me directly, please put "SE-0443" in the subject line.
What goes into a review?
The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:
What is your evaluation of the proposal?
Is the problem being addressed significant enough to warrant a change to Swift?
Does this proposal fit well with the feel and direction of Swift?
If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
More information about the Swift evolution process is available at
Super excited to see this! One common warning control that I've always wished we had for Swift is a way to suppress warnings in instances where we must call deprecated functions (from non deprecated functions) because we have no alternative. Silencing all deprecation warnings isn't great because we may be unaware of new deprecations, but "audited" deprecations tend to get in the way with no way to silence them. We've had a few of these situations in swift-foundation and thus have warnings that we must live with currently. Do you envision that this proposal might help give us the tools to resolve that, or does that not fit into this design yet?
I see your future direction section mentions #pragma diagnostic which is how we'd traditionally handle this in C code, but I wonder if maybe any of the tools here (such as the mentioned same-module-deprecation group) might get us close enough to a workable solution until we have an expressive way to annotate the function call itself?
I’ll cheerfully say I think this is a bad idea; something like #pragma diagnostic or Rust’s allow(diagnostic_name) is much less likely to accidentally silence something important, much easier to leave an explanatory comment next to, and possible to evolve in the future to an expectation, so that you know you’re silencing the right thing. Command-line arguments, meanwhile, can have ordering problems, are annoying to specify in IDEs (though not so bad in SwiftPM, with CMake somewhere in the middle), and can’t as easily be code-completed or Quick-Help’d.
I appreciate that this was straightforward to put together, with precedent in C compilers and many others. But I don’t actually think it’s a good idea, especially when the primary motivating use case is deprecation.
EDIT: I’m sorry for not bringing this up during the pitch phase, since I was on that thread. I had originally thought “well, Swift’s plan to not have warning flags in order to encourage specific ways of addressing warnings clearly hasn’t panned out, because some of these warnings are a decade old with no workaround”. It wasn’t until today that it clicked that local annotations were still a better path than top-level flags, a compromise position between “warnings should be addressed at the cause” and “devs should not be left with warnings they can’t control”.
There are a few points about group membership that I think I need clarification on:
The proposal is clear that diagnostics cannot belong directly to more than one group (they can belong to other groups indirectly, by the other group containing that one). Do they belong to exactly one group, or can they belong to no groups?
In at least one case, the proposal seems to suggest that a group might contain a single warning that is then aggregated into several groups.
Do you expect this pattern to be common?
Is it recommended?
Is it baked in somehow?
If it is, how is that materially different from making the diagnostic IDs public and then layering on a grouping system, other than the extra boilerplate of specifying both a diagnostic ID and a group name?
Is the expectation that all warnings will (ideally) be organized into appropriate groups, or do you imagine that this will be more opportunistic, with groups added to cover use cases as they are discovered and many warnings belonging to no group (or just an "all" group)?
Put another way, should there be project rules requiring that when you add a warning, you must find or create a place in the diagnostic group hierarchy for it?
You mostly talk about warnings. What about other diagnostics?
Is there a mechanism for creating diagnostics that are errors by default but can be downgraded to warnings? (I don't think we can allow all errors to be downgraded—some errors represent malformations so serious that the compiler cannot generate code for them, and some errors cut off further checking of a construct that might have found a non-downgraded error if it had been allowed to continue.)
Should remarks fit into this picture somehow?
I will likely have more to say once I have answers to these questions.
Tangentially:
Since -debug-diagnostic-names has been available in the compiler for a long time, we proceed from the fact that there are people who rely on this option and its format with square brackets.
As the person who suggested we add -debug-diagnostic-names, I think you should break its format rather than accepting limitations on its use. As the debug prefix suggests, the flag is an aid to compiler debugging. Nothing ought to depend on its format, and I can find no evidence in Google or GitHub that anything does. Compiler debugging features should not impede the development of user-facing features.
If you're really stuck on this point, though, perhaps you could display groups in angle brackets to distinguish them from diagnostic IDs.
I agree with @jrose’s analysis specifically with regards to silencing warnings; I don’t think a coarse-grained command line option for silencing specific categories of warnings is a good idea. I understood the pitch to be more narrowly focused on the warnings-as-errors situation, allowing for particular classes of warnings to be more freely moved in and out of the error bucket. I’m more amenable to that formulation of the proposal. But I think its a good policy to keep actual precise silencing mechanisms visible in source, even if that means introducing a more general #pragma diagnostic-like system (which the warning groups proposed here will make more useful!).
We use -warnings-as-errors by default at Airbnb. This is really valuable to us since it helps maintain hygiene. Most warnings can and should be addressed immediately.
Deprecation warnings in particular are a big challenge for us. Every year we get a large number of new deprecation warnings. Sometimes the replacement API is different enough that changing all of the callsites in our codebase (2M+ LOC) is infeasible. Instead, we just have to disable -warnings-as-errors for that particular module. Now there's nothing preventing other warnings from being introduced in that module, so those slowly start accumulating.
We would probably adopt -warnings-as-errors -Wwarning deprecated right away, because it would address this pain point we've had for a long time.
Huge +1, if only for the common use case described of treating all warnings as errors except for deprecations. This ability would radically improve my development process when updating code base to the latest SDKs. I think command line options would be preferable to inline pragma type directives for my use case. I’m mostly interested in elevating warnings (except for deprecations) to errors on CI, where local development would keep them as warnings.
Is the problem being addressed significant enough to warrant a change to Swift?
Yes. The status quo requires addressing all deprecations in one go when updating to the latest SDK, which can be a huge burden. Gaining fine grained control over warnings would allow us to address these deprecations incrementally.
Does this proposal fit well with the feel and direction of Swift?
I don’t have a strong opinion here but providing a path to incrementally update code seems to align with recent trends.
If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
I’ve used similar features in clang with objc and have found them quite helpful, but that’s the extent of my experience.
How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
Yes, it's a significant enough problem as it will allow larger projects to incrementally upgrade their codebases as the SDKs and the compiler evolve, and will make -warnings-as-errors viable.
I used similar features in Objective-C and SwiftLint. It would be great if there was also a way to disable warnings on a per-case basis – this should probably be the primary way of controlling warnings and errors, as others also mentioned in the comments.
Will this apply to the new and existing concurrency warnings and errors? If there was a way to disable some of these without modifying the code and risking introducing a regression, that would be perfect.
I appreciate the focus on providing the controls and letting the developers figure out how to best use these to their advantage.
I thought about not including suppressing in the new handling model. And in fact, right now the implementation PR is not aligned with the proposal in this regard: the options are called -warning-as-error and -no-warning-as-error and there is no support for -suppress-warnings. And in general, I agree that silencing warnings, especially in large groups, especially in the whole module is not a good thing.
However, the fact that -suppress-warnings already exists forces us to take it into account in some form.
Currently, the diagnostic handling mechanism is designed in such a way: if a warning becomes an error, it remains an error, and if not and suppressing is enabled, it is silenced.
So if we leave the -suppress-warnings flag as it is now, and only support -[no-]warning-as-error, then we can have a combination of flags:
which will be interpreted as "broader_group will turn into errors, except for warnings from narrower_group, which will be suppressed".
So we can't express the rule "Leave this warning as a warning".
This is not very good in itself, but it does not break the logic of -suppress-warnings as it is now.
However, if in the future we include -suppress-warning(s) in the unified warning handling scheme and make this flag positional, then this will be a breaking change.
For example, these three compiler calls will have different meanings:
-suppress-warnings -warning-as-error deprecated - "deprecated" warnings are treated as errors.
-warning-as-error deprecated -suppress-warnings - all warnings are suppressed, including the "deprecated" group.
-warnings-as-errors -suppress-warnings -no-warning-as-error deprecated - "deprecated" warnings are just warnings.
If we support -suppress-warnings immediately, we might avoid having to address this issue in the future.
So that's the reason why -Wsuppress and -suppress-warnings are included in the proposal. I see risks associated with the misuse of them. However, there are also risks in postponing this functionality.
I certainly agree with this, and in general, the narrower the scope to which a rule is applied, the easier it is to understand why. And we mention this in the Future Direction section. However, the design of this feature in the language itself may be even more controversial. In my opinion at some point, we should support it in the language, but having some experience with group-based rules might be helpful. Also, we have to have stable diagnostic identifiers anyway.
There's also a tricky warning "This is an error in the Swift N language mode", which is just a wrapper for an actual error. When you have -warnings-as-errors it becomes an error.
And deprecation warnings are not just those by the @availability attribute. A recent example is the deprecation of the @_implementationOnly attribute. So it's deprecation in a broader sense.
And I'm pretty sure there will be more.
It depends on what type of warning we a dealing with. In general, we don't know much about the context of the warning in the diagnostic engine (where we apply the rules). So to achieve some kind of differentiation of the warning we need to split it into more concrete variants and use the appropriate variant at the warning production site. The same-module-deprecation example is good in this aspect, but not every warning might be split in a similar way. If that's the case then the proposed flags won't be helpful. And for silencing an arbitrary warning I really think you better wait for the language support so you can silence it in the narrowest possible scope.
The ultimate goal is to sort out pretty much all the diagnostics into groups. However for now they can belong to no groups because we just can't manage all of them in a single run. Also, I'm aware of one particular warning ("...; This is an error in the Swift N language mode") that shouldn't belong to any group, because 1) it's a compiler implementation detail 2) it doesn't communicate much information about the actual warning, so it's pretty useless from user perspective.
I hope it makes sense.
Do you refer to the "unsafe_global_actor_deprecated" example? It's a matter of convenience in organizing the supergroups. And the group system allows great flexibility in how and when we can include a group in broader groups.
The basic rule is that a warning should be included in one narrow group. Narrow is 1-2 warnings. So for the unsafe_global_actor_deprecated warning, we should declare the unsafe_global_actor_deprecated group. But since we have "deprecated" and "concurrency" groups it's very natural to include the unsafe_global_actor_deprecated group in them as well.
I don't expect a warning can be contained in something like 10 unrelated supergroups, but there's no actual limitation implied here aside from the common sense. And I think with some practice we can come up with a more formal guideline.
Simply can't make diagnostic IDs public, too fragile to use in user-visible API.
Yes, pretty much all of them should be grouped. And yes, I think there should be such a rule and @Douglas_Gregor suggested enforcing it with an assert.
Errors can be grouped as well, but -W options don't apply to them. This is needed because some errors are warnings in some language modes (that warning about swift 6 again). So if an error is downgraded by some other logic to a warning then it will be subject to the warning control flags, but users can't downgrade errors on their will.
I didn't find a use for it yet, so left them without group support for now, but in general, I don't see why they couldn't fit the model.
I think @allevatomight disagree
Anyway, I think the proposed solution where we prevent interference with -debug-diagnostic-names is good enough. At least for now, I don't see why someone would need to use both of them.
After previously pushing for an attribute to silence deprecation warnings in specific scopes (that unfortunately didn’t go anywhere), I’m extremely happy to see this
Personally I'm looking for a way to selectively turn off warnings in specific parts of the code. That is, I think we need an equivalent of #pragma diagnostic. This would be extremely useful to turn off e.g. deprecation and concurrency warnings in particular parts of the code while retaining warnings everywhere else in order to avoid regressions. It's a bit unclear to me if this proposal would allow me to do this but it seems like it doesn't.
I would like to see more granular control at the source code level as @jrose suggested. And I'm more in favour of @Jumhyn option to have this change be more narrow to address -warnings-as-errors case only if we talk about current state, and not having this control as compiler flags in overall.
At the same time, since we already have -suppress-warnings, these changes seems to be beneficial and reasonable to add in general, and holding them back because they can be mis/ab-used seems to be not a significant enough reason. After reading a bit more I am reevaluating this part: suppressing all warnings different to selective. I don’t think we should have selective warning suppression as compilation flags at all. As downgrade from errors it makes sense, but not silencing.
It is with a heavy heart that I must -1 this proposal. The problem it is trying to solve is not new and absolutely something that needs to be addressed, but the approach outlined in the proposal is quite surprising to me considering that it seems to not match up with Core Team's position on this, which is something I know that they already have, given that people keep bringing this up as a problem and every time potential solutions get shot down with arguments like "warnings are there for a reason and we don't want people to just turn them off entirely" or "we don't want people making dialects of Swift". Then the proposal that actually gets to the review stage ends up with things like "oh you can actually turn off all deprecation warnings this is ok actually" which nobody actually wants and was asking for, except for I guess the people at Airbnb. (Ok, I'm not being entirely serious, the example @cal gave makes sense. But I think that is specific to the case of warnings-as-errors whereas the proposal also caters to people who just don't want warnings period.)
Anyway, I am going to join @jrose and say that any proposal that doesn't include source code annotations to precisely delineate code where warnings can be turned off or reduced should not be considered. I am ok with having compiler flags in addition to this but I think the common case here is that most warnings are good, including deprecation warnings, and there are just a handful of places in their code where turning the off makes sense. Most Swift users understand this and would actively prefer to not use constructs that just disable them entirely. To that end I think the "future directions" should actually become the meat of this proposal.
Naive clarification question: if I have several diagnostic groups I would like to manipulate, how do I discover whether I need to handle them independently or whether there exists a single group that contains them all?
For example, the proposal mentions (hypothetical) groups deprecated containing availability_deprecated containing availability_deprecated_same_module. If I'm understanding correctly, with the option -print-diagnostic-groups I would see some of my warnings annotated [availability_deprecated] and others annotated [availability_deprecated_same_module]. But where could I discover that the super-group deprecated exists and contains both of the others?
