Skip to content

[Diagnostics] Add -no-warning-as-error to except a specific warning from being treated as an error #74466

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 9, 2024

Conversation

DmT021
Copy link
Contributor

@DmT021 DmT021 commented Jun 17, 2024

This commit adds a new compiler option -no-warning-as-error which allows users to specify exceptions from the -warnings-as-errors option.
The format of the option is -no-warning-as-error <diag_id>, where <diag_id> is one of the diagnostic IDs listed in include/swift/AST/Diagnostics*.def.
The option can be added multiple times.

@DmT021 DmT021 marked this pull request as ready for review June 17, 2024 01:44
@DmT021
Copy link
Contributor Author

DmT021 commented Jun 25, 2024

@artemcm Can you take a look please?

@DmT021
Copy link
Contributor Author

DmT021 commented Jul 1, 2024

@hborla Can you take a look please?

Copy link
Contributor

@artemcm artemcm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay @DmT021.
This change looks good, and I think it's good to keep this in HelpHidden.

Please also make a matching change in:
https://github.com/apple/swift-driver?tab=readme-ov-file#rebuilding-optionsswift

@@ -61,6 +62,9 @@ class DiagnosticOptions {
/// Treat all warnings as errors
bool WarningsAsErrors = false;

/// Don't treat these warnings as errors when WarningsAsErrors=true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please document that these are diagnosticID strings here.

@artemcm
Copy link
Contributor

artemcm commented Jul 1, 2024

It's worth noting that there are no guarantees that diagnostic IDs are stable API between different compiler versions, they have not historically been treated as such.

@DmT021
Copy link
Contributor Author

DmT021 commented Jul 1, 2024

It's worth noting that there are no guarantees that diagnostic IDs are stable API between different compiler versions, they have not historically been treated as such.

Yeah, but when used with -debug-diagnostic-names it's pretty easy to see what the diagnosticID is for a particular warning. Should I include this note in the help text as well?

@artemcm
Copy link
Contributor

artemcm commented Jul 1, 2024

It's worth noting that there are no guarantees that diagnostic IDs are stable API between different compiler versions, they have not historically been treated as such.

Yeah, but when used with -debug-diagnostic-names it's pretty easy to see what the diagnosticID is for a particular warning. Should I include this note in the help text as well?

Yes, please.

Copy link
Contributor

@artemcm artemcm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After consulting with some more folks, I think the use of diagnostic IDs is a bigger concern.

Though the idea behind this change is worthwhile.
One possible approach to get the desired result is to implement something akin to Clang's diagnostic groups. Then warning diagnostics can be grouped into stable-named groups which can then be treated as exceptions to warning-as-error, for example.

@DougGregor
Copy link
Member

It's worth noting that there are no guarantees that diagnostic IDs are stable API between different compiler versions, they have not historically been treated as such.

This is a really important point. The addition of this feature would hamper further evolution of the compiler, because code that relies on -no-warning-as-error <diag_id> to compile will stop working if the diagnostic ID changes. We can't just promise not to make changes, because it's very common for us to improve diagnostics by adding more special-case versions of existing warnings, which will necessitate new diagnostic IDs.

I support the general idea of more specific control over warnings, but not based on the diagnostic IDs. Clang's approach of having documented "warning groups" with stable IDs, where each warning is sorted into an appropriate warning group, would be a reasonable model to follow. It's definitely more upfront work to do this... along with building the infrastructure for warning groups, we'd also need to sort the existing ~350 warnings into groups. But if we're going to do this, it's worth doing it well so we can provide flexibility to developers while still being able to evolve the compiler.

@DmT021
Copy link
Contributor Author

DmT021 commented Jul 1, 2024

Using groups will result in a loss of precision when defining exclusion rules, which isn't desirable either. Of course, I agree that control via diagnostic IDs isn't very rigid, and group-based rules are superior. But this is only important if the user wants to distribute the library. If they are building an end binary, it is quite acceptable.
This change addresses existing situations where a newly diagnosed warning by a new compiler led to a failed build because of the -warnings-as-errors rule.
What if we provide a generalized rule syntax that allows the user to specify a condition (either an ID or a group) to check a warning against? Something like

-no-warning-as-error ignored_import                     (group)
-no-warning-as-error id:sema_import_current_module      (id)

Using id-based rules is still discouraged for source-distributed libraries, but is still an option for everyone else.
And then we can categorize warnings into groups gradually.

@DougGregor
Copy link
Member

Using groups will result in a loss of precision when defining exclusion rules, which isn't desirable either

It's necessary to allow us to improve diagnostics in the future, but providing more-specific diagnostics in certain cases.

But this is only important if the user wants to distribute the library. If they are building an end binary, it is quite acceptable.

That's not true. As I described in my original reply, we must preserve the ability to create more specific versions of a given warning that provide a better diagnosis for the user. Adding this feature based on diagnostic IDs makes that kind of change a source-breaking change.

@DougGregor
Copy link
Member

@DmT021 there are a bunch of design and usability questions here, and it's grown beyond what's reasonable to figure out in the pull request. I think we'd benefit from taking the discussion over to forums.swift.org where we can get more feedback from others

@DmT021
Copy link
Contributor Author

DmT021 commented Jul 3, 2024

@DmT021 DmT021 force-pushed the wp/no-warning-as-error branch from 1e812ad to 7314f01 Compare July 17, 2024 03:38
@DmT021 DmT021 requested a review from DougGregor as a code owner July 17, 2024 03:38
@DmT021
Copy link
Contributor Author

DmT021 commented Jul 17, 2024

@artemcm @DougGregor This is still work-in-progress but I want to check if I'm moving in the right direction. Here's what I have so far:

  • DiagnosticGroups.def file defines diagnostic groups. I took clang's approach and made groups nested: a group can have subgroups.
  • To have something to begin with I declared several groups. In particular there are the deprecated group and availability_deprecated which is nested to deprecated. The idea here is to have all deprecation warnings nested in the deprecated group so they can be excluded together.
  • It compiles as constexpr.
  • WARNING and ERROR diagnostics now have GROUPED_ variants.
  • New compiler options -[no-]-warning-as-error <group>. <group> can be either a real group or a string _id:<diag_id>. And a new option group warning_treating_Group that aggregates them.
  • -[no-]-warning-as-error and -[no-]-warnings-as-errors apply in order they appear in the command line. If there're more than one rule affecting the behavior of the same DiagID - the last rule wins.
  • The Diagnostic Groups graph consistency expressed as static_asserts and not dedicated tests.
  • The diagnostic groups graph is checked for cycles.

What's left to do:

  • It seems reasonable to include the "warning suppression" rules in this mechanism. But I decided to avoid doing this until we agree on it.
  • There's a difficulty with testing that a particular warning is actually in the expected group(s).
    For example there's a bunch of test for the @available(*, deprecated) annotation. We need to check that all of them are in the availability_deprecated and deprecated groups. We can invoke the compiler with all combinations of -[no-]-warning-as-error and -[no-]-warnings-as-errors flags for each test, but this will result in combinatorial explosion.
    So I came up with an alternative: we can introduce a new flag -pring-diagnostic-groups which will add a suffix with [group:<group_name>] to all the warnings produced. We only need to print the group that directly includes this particular DiagID and not the whole hierarchy of groups. Then we test that warnings are suffixed with the expected groups.
    (We should also test that groups are nested correctly, but that's already done with static_asserts.)
    Is it ok?

@DmT021 DmT021 force-pushed the wp/no-warning-as-error branch 2 times, most recently from 88702f0 to 32d599d Compare July 18, 2024 02:19
@DmT021
Copy link
Contributor Author

DmT021 commented Jul 23, 2024

@artemcm @DougGregor ping?

Copy link
Member

@DougGregor DougGregor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll (separately) comment on the design aspects, but the implementation here looks good and I only have a few small comments.

#include "swift/AST/DefineDiagnosticGroupsMacros.h"

#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wgnu-zero-variadic-macro-arguments"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll have to see if this causes us any trouble with other compilers.

//
//===----------------------------------------------------------------------===//

#define DEFINE_DIAGNOSTIC_GROUPS_MACROS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not critical now, but at some point we'd want a big comment here documenting what it means to define a new group.

llvm::ArrayRef<DiagID> diagnostics;

void
traverseDepthFirst(std::function<void(const DiagGroupInfo &)> func) const;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The std::function could be llvm::function_ref, but it's not a big deal.

@@ -47,7 +47,7 @@ ERROR(error_no_group_info,none,
NOTE(brace_stmt_suggest_do,none,
"did you mean to use a 'do' statement?", ())

WARNING(error_in_future_swift_version,none,
GROUPED_WARNING(error_in_future_swift_version,error_in_future_swift_version,none,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This specific warning is a bit tricky, because everything that goes through it started its life as an error.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, it confuses me a little. It's not technically wrong - if a warning was upgraded to error because we enabled -swift 6 we want it to be an actual error and fail the build. But it would be nice if there was some cleaner solution for these warnings.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The unfortunate thing about this warning group is that it's going to mix unrelated diagnostics together into a single warning group, because we use it via the warnUntilSwiftVersion(N) on ERRORs of various flavors. The only other idea I have here is to (eventually) require ERRORs treated this way to be part of a warning group. For example, a "concurrency" warning group could encapsulate all of the concurrency diagnostics (which are ERRORs) when they get downgraded. But we'd have to disallow them from being downgraded in Swift 6 somehow, so there's a bit of work to get to that point.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was my initial idea too, which is why both GROUPED_WARNING and GROUPED_ERROR exist. The issue is that, theoretically, a Diagnostic can be nested inside another Diagnostic at any place within the Args vector. And this tree can be of any depth. If we want to check nested diagnostics we have to traverse all the tree.
And then if at least one diagnostic with an excluded groupID is found - apply the behavior(error/warning/suppress?) to the root Diagnostic.
I wasn't sure if it's correct.

return false;
}

static_assert(!hasCycle(), "Diagnostic groups graph has a cycle!");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is quite some clever metaprogramming.

@@ -0,0 +1,65 @@
// RUN: not %target-swift-frontend -typecheck -diagnostic-style llvm -warnings-as-errors %s 2>&1 | %FileCheck %s --check-prefix=CHECK-WAE-ALL
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this forcing the LLVM diagnostic style?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the swift style the compiler produces a message like this:

$ ./swift-frontend -typecheck -diagnostic-style swift -warnings-as-errors  ../../../../swift/test/diagnostics/warnings_as_errors_rules.swift
../../../../swift/test/diagnostics/warnings_as_errors_rules.swift:42:1: error: 'foo()' is deprecated
40 | // CHECK-WAE-GROUP-NWAE-DIAGID: warning: 'foo()' is deprecated [group:availability_deprecated]
41 | // CHECK-WAE-GROUP-NWAE-DIAGID-NOT: error: 'foo()' is deprecated [group:availability_deprecated]
42 | foo()
   | `- error: 'foo()' is deprecated
43 |
44 |

As you can see there are two lines more above and below the line 42. FileCheck sees it in its input and when it performs a -NOT check these comments actually produce a match (which fails the testcase).
Ideally I'd prefer if FileCheck ignored lines containing .*CHECK-.*, but I don't see a flag or option for this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I understand. In our call through to the swift-syntax diagnostic mechanism, we're hard-coding the context size to 2 lines:

  swift_ASTGen_renderQueuedDiagnostics(queuedDiagnostics, /*contextSize=*/2,
                                       forceColors ? 1 : 0,
                                       &bridgedRenderedString);

... we should have an option here to allow setting the context size (e.g., to 0), which would make FileCheck tests like this more feasible.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be clear, I don't mean "you have to add this other command-line argument yourself." I'm fine with your PR using -diagnostic-style llvm until we get the other command-line argument. I suspect that's the reason for some of the other existing uses of -diagnostic-style llvm

@DougGregor
Copy link
Member

@swift-ci please smoke test

@DmT021
Copy link
Contributor Author

DmT021 commented Jul 23, 2024

MSVC failed on __VA_OPT__. I might need some time to find a windows machine and come up with an alternative.

@DmT021
Copy link
Contributor Author

DmT021 commented Jul 27, 2024

The changes:

  • Removed __VA_OPT__
    I couldn't find an acceptable way to iterate over __VA_ARGS__ without __VA_OPT__. Most solutions fail to compile on MSVC. So I changed the syntax for declaring groups by splitting it into two macros: GROUP and GROUP_LINK. Once we switch to C++20, we can bring __VA_OPT__ back.
  • Removed support for _id:
  • Used llvm::function_ref
  • Added docs and examples for each diagnostic group.
  • Removed support for error_in_future_swift_version. I'll work on it separately.
  • Removed #pragma clang and used [[maybe_unused]] attribute instead.

@DougGregor Can you take a look and rerun the tests, please?

@tshortli
Copy link
Contributor

@swift-ci please test

@DmT021
Copy link
Contributor Author

DmT021 commented Jul 29, 2024

The fix for broken Linux builds has been merged. Can someone rerun CI?

@tshortli
Copy link
Contributor

@swift-ci please test Linux

Copy link
Member

@DougGregor DougGregor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks really good; I have no further comments about the implementation that aren't "ooh, and then we can do !"

@@ -311,8 +311,7 @@ void ToolChain::addCommonFrontendArgs(const OutputInfo &OI,
inputArgs.AddLastArg(arguments, options::OPT_profile_generate);
inputArgs.AddLastArg(arguments, options::OPT_profile_use);
inputArgs.AddLastArg(arguments, options::OPT_profile_coverage_mapping);
inputArgs.AddAllArgs(arguments, options::OPT_warnings_as_errors,
options::OPT_no_warnings_as_errors);
inputArgs.AddAllArgs(arguments, options::OPT_warning_treating_Group);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll (separately) need to do this in the new swift-driver

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, here's a PR swiftlang/swift-driver#1678

@DmT021
Copy link
Contributor Author

DmT021 commented Aug 3, 2024

This looks really good; I have no further comments about the implementation that aren't "ooh, and then we can do !"

@DougGregor Should I ping other folks to review this as well? Or I can just squash rebase and you'll merge it?

DmT021 added a commit to DmT021/swift-driver that referenced this pull request Aug 3, 2024
This commit adds support for the warning treating option group, including the following options: -warnings-as-errors, -no-warnings-as-errors, -warning-as-error, and -no-warning-as-error.
Options in this group are now preserved as-is. It is forbidden to reorder or drop any of them.
These changes reflect the modifications made to the frontend in swiftlang/swift#74466.
@DougGregor
Copy link
Member

This looks really good; I have no further comments about the implementation that aren't "ooh, and then we can do !"

@DougGregor Should I ping other folks to review this as well? Or I can just squash rebase and you'll merge it?

To cover all of the bases, I've brought up this feature with the Language Steering Group to determine whether we should review it through the evolution process. I suspect we don't, because it's primarily a compiler feature that's not exposed through the language, but it's enough of a change in direction that I want to be sure to involve them.

@mtzaquia
Copy link

This looks really good; I have no further comments about the implementation that aren't "ooh, and then we can do !"

@DougGregor Should I ping other folks to review this as well? Or I can just squash rebase and you'll merge it?

To cover all of the bases, I've brought up this feature with the Language Steering Group to determine whether we should review it through the evolution process. I suspect we don't, because it's primarily a compiler feature that's not exposed through the language, but it's enough of a change in direction that I want to be sure to involve them.

Hey @DougGregor, do you have any updates here? It would be great to have this merged as soon as you come to an agreement. Also, would you be able to provide a rough estimate on when these changes would land on a RELEASE build of Swift?

@DmT021
Copy link
Contributor Author

DmT021 commented Aug 19, 2024

@mtzaquia This feature has recently expanded beyond what is implemented in this PR and will go through a review.

@DougGregor
Copy link
Member

The proposal has been accepted!

@DmT021
Copy link
Contributor Author

DmT021 commented Sep 5, 2024

I'm aligning the implementation here with what the proposal says.

…er warning behavior

This commit adds new compiler options -no-warning-as-error/-warning-as-error which allows users to specify behavior for exact warnings and warning groups.
@DmT021 DmT021 force-pushed the wp/no-warning-as-error branch from 5dfd521 to 070c77e Compare September 7, 2024 05:59
@DmT021
Copy link
Contributor Author

DmT021 commented Sep 7, 2024

I fixed the flag names, added a check to the old driver to prevent the new flags from being used with -suppress-warnings, and updated the tests. I'll implement -print-diagnostic-groups in the next PR, but other than that, everything here is now in line with the proposal.

Someone, please rerun the tests 🙏

@DougGregor
Copy link
Member

@swift-ci please test

DmT021 added a commit to DmT021/swift-driver that referenced this pull request Sep 7, 2024
This commit adds support for the warning treating option group, including the following options: -warnings-as-errors, -no-warnings-as-errors, -Werror, and -Wwarning.
Options in this group are now preserved as-is. It is forbidden to reorder or drop any of them.
These changes reflect the modifications made to the frontend in swiftlang/swift#74466.
@DmT021
Copy link
Contributor Author

DmT021 commented Sep 7, 2024

The errors on macOS are rather strange. It doesn't look like it's because of this PR: 'Mirror' is unavailable, broken standard library: cannot find intrinsic operations on UnsafeMutablePointer<T>, failed to build module 'Darwin'; this SDK is not supported by the compiler. And I see other recently failed builds here with the same errors.

``` FAILED: stdlib/private/SwiftPrivate/FREESTANDING/arm64/SwiftPrivate.o /Users/ec2-user/jenkins/workspace/swift-PR-macos/branch-main/build/buildbot_incremental/minimalstdlib-macosx-x86_64/stdlib/private/SwiftPrivate/FREESTANDING/arm64/SwiftPrivate.o ... /Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk/usr/lib/swift/Darwin.swiftmodule/arm64e-apple-macos.swiftinterface:29:34: error: 'Mirror' is unavailable 27 | } 28 | extension Darwin.DarwinBoolean : Swift.CustomReflectable { 29 | public var customMirror: Swift.Mirror { | `- error: 'Mirror' is unavailable 30 | get 31 | }

Swift.Mirror:2:15: note: 'Mirror' has been explicitly marked unavailable here
1 | @available(*, unavailable)
2 | public struct Mirror {
| `- note: 'Mirror' has been explicitly marked unavailable here
3 | public enum AncestorRepresentation {
4 | case generated

/Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk/usr/lib/swift/Darwin.swiftmodule/arm64e-apple-macos.swiftinterface:28:40: error: 'CustomReflectable' is unavailable
26 | public typealias BooleanLiteralType = Swift.Bool
27 | }
28 | extension Darwin.DarwinBoolean : Swift.CustomReflectable {
| `- error: 'CustomReflectable' is unavailable
29 | public var customMirror: Swift.Mirror {
30 | get

Swift.CustomReflectable:2:17: note: 'CustomReflectable' has been explicitly marked unavailable here
1 | @available(*, unavailable)
2 | public protocol CustomReflectable {
| `- note: 'CustomReflectable' has been explicitly marked unavailable here
3 | var customMirror: Mirror { get }
4 | }

/Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk/usr/lib/swift/Darwin.swiftmodule/arm64e-apple-macos.swiftinterface:767:36: error: broken standard library: cannot find intrinsic operations on UnsafeMutablePointer
765 | @transparent public func lgamma( x: Swift.Float) -> (Swift.Float, Swift.Int) {
766 | var sign = Int32(0)
767 | let value = lgammaf_r(CFloat(x), &sign)
| `- error: broken standard library: cannot find intrinsic operations on UnsafeMutablePointer
768 | return (Float(value), Int(sign))
769 | }

/Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk/usr/lib/swift/Darwin.swiftmodule/arm64e-apple-macos.swiftinterface:772:36: error: broken standard library: cannot find intrinsic operations on UnsafeMutablePointer
770 | @transparent public func lgamma( x: Swift.Double) -> (Swift.Double, Swift.Int) {
771 | var sign = Int32(0)
772 | let value = lgamma_r(CDouble(x), &sign)
| `- error: broken standard library: cannot find intrinsic operations on UnsafeMutablePointer
773 | return (Double(value), Int(sign))
774 | }

/Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk/usr/lib/swift/Darwin.swiftmodule/arm64e-apple-macos.swiftinterface:777:43: error: broken standard library: cannot find intrinsic operations on UnsafeMutablePointer
775 | @transparent public func remquo( x: Swift.Float, _ y: Swift.Float) -> (Swift.Float, Swift.Int) {
776 | var quo = Int32(0)
777 | let rem = remquof(CFloat(x), CFloat(y), &quo)
| `- error: broken standard library: cannot find intrinsic operations on UnsafeMutablePointer
778 | return (Float(rem), Int(quo))
779 | }

/Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk/usr/lib/swift/Darwin.swiftmodule/arm64e-apple-macos.swiftinterface:782:44: error: broken standard library: cannot find intrinsic operations on UnsafeMutablePointer
780 | @transparent public func remquo( x: Swift.Double, _ y: Swift.Double) -> (Swift.Double, Swift.Int) {
781 | var quo = Int32(0)
782 | let rem = remquo(CDouble(x), CDouble(y), &quo)
| `- error: broken standard library: cannot find intrinsic operations on UnsafeMutablePointer
783 | return (Double(rem), Int(quo))
784 | }

/Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk/usr/lib/swift/Darwin.swiftmodule/arm64e-apple-macos.swiftinterface:787:21: error: broken standard library: cannot find intrinsic operations on UnsafeMutablePointer
785 | @available(swift, deprecated: 4.2, message: "use Float(nan: Float.RawSignificand).")
786 | @transparent public func nan( tag: Swift.String) -> Swift.Float {
787 | return Float(nanf(tag))
| `- error: broken standard library: cannot find intrinsic operations on UnsafeMutablePointer
788 | }
789 | @transparent public func jn( n: Swift.Int, _ x: Swift.Double) -> Swift.Double {

/Users/ec2-user/jenkins/workspace/swift-PR-macos/branch-main/swift/stdlib/private/SwiftPrivate/IO.swift:21:8: error: failed to build module 'Darwin'; this SDK is not supported by the compiler (the SDK is built with 'Apple Swift version 5.9.2 (swiftlang-5.9.2.2.11 clang-1500.1.0.2.2)', while this compiler is 'Apple Swift version 6.1-dev effective-5.10 (LLVM 80d178d8cbb1cee, Swift 67ac5b9)'). Please select a toolchain which matches the SDK.
19 | #else
20 | #if canImport(Darwin)
21 | import Darwin
| `- error: failed to build module 'Darwin'; this SDK is not supported by the compiler (the SDK is built with 'Apple Swift version 5.9.2 (swiftlang-5.9.2.2.11 clang-1500.1.0.2.2)', while this compiler is 'Apple Swift version 6.1-dev effective-5.10 (LLVM 80d178d8cbb1cee, Swift 67ac5b9)'). Please select a toolchain which matches the SDK.
22 | #elseif canImport(Glibc)
23 | import Glibc

/Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk/usr/lib/swift/Darwin.swiftmodule/arm64e-apple-macos.swiftinterface:29:34: error: 'Mirror' is unavailable
27 | }
28 | extension Darwin.DarwinBoolean : Swift.CustomReflectable {
29 | public var customMirror: Swift.Mirror {
| `- error: 'Mirror' is unavailable
30 | get
31 | }

</details>

@DougGregor
Copy link
Member

@swift-ci please smoke test macOS

@DougGregor
Copy link
Member

The macOS errors you are seeing do seem unrelated to your change.

@DougGregor
Copy link
Member

@swift-ci please smoke test macOS

Copy link
Member

@DougGregor DougGregor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All my remaining comments are cosmetic, and the proposal has been accepted. Let's merge this!

@@ -851,8 +852,8 @@ namespace swift {
/// Don't emit any remarks
bool suppressRemarks = false;

/// Emit all warnings as errors
bool warningsAsErrors = false;
/// Treat these warnings as errors. Indicies here corespond to DiagID enum
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typos "indicies" (for indices) and "corespond"

return state.getWarningsAsErrors();
}
/// Apply rules specifing what warnings should or shouldn't be treated as
/// errors. For group rules the string is either a group name defined by
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no "or" to go with the "either here".

Comment on lines +28 to +29
swiftc -warning-as-error availability_deprecated file.swift
swiftc -warnings-as-errors -no-warning-as-error availability_deprecated file.swift
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These flag names have changed in SE-0443.

Comment on lines +10 to +11
swiftc -warning-as-error deprecated file.swift
swiftc -warnings-as-errors -no-warning-as-error deprecated file.swift
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flag names have changed.

Such warnings are emitted after the behavior for all specified warning groups has been processed, which means their behavior can also be specified. For example:

```sh
swiftc -warning-as-error unknown_warning_group -warning-as-error non_existing_group file.swift
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flag names again

@DougGregor DougGregor merged commit 08e339b into swiftlang:main Sep 9, 2024
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants