[SE-0480] Build settings for warning control flags by DmT021 · Pull Request #8315 · swiftlang/swift-package-manager

DmT021 · 2025-02-27T15:36:16Z

This change adds warning control settings as described in the proposal. The flags for Swift targets are described by SE-0443, for Clang targets - here.

DougGregor · 2025-03-12T15:37:55Z

@swift-ci test

DougGregor

Aside from the two places where we can go quadratic in the number of command-line arguments, this is looking good to me. But I'm not super familiar with the SwiftPM code base.

DougGregor · 2025-03-12T16:02:42Z

+            // `-w` (suppress warnings) and the other warning control flags are mutually exclusive
+            for index in args.indices.reversed() {
+                let arg = args[index]
+                if arg.starts(with: "-W"), arg.count > 2 {
+                    // we consider the following flags:
+                    // -Wxxxx
+                    // -Wno-xxxx
+                    // -Werror
+                    // -Werror=xxxx
+                    // -Wno-error
+                    // -Wno-error=xxxx
+                    args.remove(at: index)


This loop could be a filter, which would make it linear-time rather than quadratic.

DougGregor · 2025-03-12T16:05:29Z

+            for index in args.indices.reversed() {
+                let arg = args[index]
+                switch arg {
+                case "-warnings-as-errors", "-no-warnings-as-errors":
+                    args.remove(at: index)
+                case "-Wwarning", "-Werror":
+                    guard args.indices.contains(index + 1) else {
+                        throw InternalError("Unexpected '\(arg)' at the end of args")
+                    }
+                    args.remove(at: index + 1)
+                    args.remove(at: index)
+                default:
+                    break
+                }


Also technically quadratic. Even though it's a little ugly, I think I'd still rather do it with a filter

var removeNextArg = false args = args.filter { arg in if removeNextArg { removeNextArg = true return false } switch arg { case "-warnings-as-errors", "-no-warnings-as-errors": return false case "-Wwarning", "-Werror": removeNextArg = true return false default: return true } }

Yes, the quadratic complexity is an oversight on my part. Good thing you noticed.
TBH, using an external state in a filter makes me a bit uneasy. The documentation for filter doesn't guarantee that the predicate will be invoked in any particular order, so I'd try to avoid it.
I don't insist though. If you think a filter here is fine, I don't mind. But what do you think about something more "manual", e.g:

var readIndex = args.startIndex var writeIndex = args.startIndex let endIndex = args.endIndex while readIndex < endIndex { let arg = args[readIndex] switch arg { case "-warnings-as-errors", "-no-warnings-as-errors": break case "-Wwarning", "-Werror": readIndex += 1 // -Wwarning and -Werror have an argument if readIndex >= endIndex { throw InternalError("Unexpected '\(arg)' at the end of args") } default: if readIndex != writeIndex { args[writeIndex] = args[readIndex] } writeIndex += 1 } readIndex += 1 } if writeIndex < endIndex { args.removeSubrange(writeIndex..<endIndex) }

dschaefer2 · 2025-03-12T18:42:58Z

A couple of general questions. Placing these flags in the package manifest, means all consumers of this package will build with the same settings. I was just wondering if it's a concern that consumers may want to set them differently. For example, if they are specifying additional warning setting flags on the swift build command lines, which ones take precedence.

Also, as we're working on adopting Swift Build to replace our native build system, we would need to make sure this is handled there as well.

dschaefer2 · 2025-03-12T18:44:40Z

Mind you the pitch and evolution proposal predates my participation so maybe this has already been addressed. I'll need to catch up.

DmT021 · 2025-03-12T19:09:22Z

I was just wondering if it's a concern that consumers may want to set them differently. For example, if they are specifying additional warning setting flags on the swift build command lines, which ones take precedence.

That's a good question. AFAIK, the additional flags (-Xcc, -Xcxx, -Xswiftc) are placed after the internally generated flags, but I didn't really check that. I'll dive into it later. If it's not the case, I think we should make it so. However, I need a confirmation from the maintainers that that's really the desired behavior.
If that's the guaranteed order of flags, then users will be able to either override or extend the warning treating rules. The rules are evaluated left to right in the compilers. So appending -[no-]warning-as-errors overrides everything that is earlier in the command line.

Also, as we're working on adopting Swift Build to replace our native build system, we would need to make sure this is handled there as well.

I've raised a discussion about that, but it seems like the Swift Build maintainers prefer a simpler approach to these flags. swiftlang/swift-build#248

Mind you the pitch and evolution proposal predates my participation so maybe this has already been addressed. I'll need to catch up.

The original proposal (SE-0443) doesn't address this API, but we're working on fixing that. We will probably write a new pitch.

DmT021 · 2025-03-15T09:59:11Z

@swift-ci Please test

dschaefer2 · 2025-03-17T13:57:09Z

@swift-ci please test windows

DmT021 · 2025-03-18T16:03:14Z

For example, if they are specifying additional warning setting flags on the swift build command lines, which ones take precedence.

Following up on this question. The additional arguments the used provided are placed after the ones SwiftPM generates.
The code responsible for this behavior has comments about that, which leads me to believe it was intentional.
For C/C++
For Swift

So if we consider the following Package.swift:

let package = Package(
    name: "MyExecutable",
    platforms: [
        .macOS(.v13),
    ],
    targets: [
        .target(
            name: "cfoo",
            cSettings: [
                .enableWarning(name: "all"),
                .disableWarning(name: "unused-parameter"),
                .treatAllWarnings(as: .error),
                .treatWarning(name: "unused-variable", as: .warning),
            ]
        ),
        .target(
            name: "cxxfoo",
            cxxSettings: [
                .enableWarning(name: "all"),
                .disableWarning(name: "unused-parameter"),
                .treatAllWarnings(as: .error),
                .treatWarning(name: "unused-variable", as: .warning),
            ]
        ),
        .executableTarget(
            name: "swiftfoo",
            swiftSettings: [
                .treatAllWarnings(as: .error),
                .treatWarning(name: "DeprecatedDeclaration", as: .warning),
            ]
        ),
    ]
)

And build it with additional arguments like this:

swift-build --very-verbose -Xcc -Wno-error -Xswiftc -no-warnings-as-errors

We will get the following invocations of the compilers:

For cfoo:
clang ... -Wall -Wno-unused-parameter -Werror -Wno-error=unused-variable ... -Wno-error ...
For cxxfoo:
clang ... -Wall -Wno-unused-parameter -Werror -Wno-error=unused-variable ... -Wno-error ...
For swiftfoo:
swiftc ... -warnings-as-errors -Wwarning DeprecatedDeclaration ... -no-warnings-as-errors -Xcc -Wno-error ...

Given that these flags are evaluated left-to-right it gives the user some control here at the build stage.
However, it doesn't compose well with -suppress-warnings, as it's mutually exclusive with the other flags:

swift-build --very-verbose -Xswiftc -suppress-warnings

swiftc ... -warnings-as-errors -Wwarning DeprecatedDeclaration ... -suppress-warnings ...

error: conflicting options '-warnings-as-errors' and '-suppress-warnings'
error: conflicting options '-Wwarning' and '-suppress-warnings'

DmT021 · 2025-03-26T12:55:55Z

@swift-ci Please test

DmT021 · 2025-03-26T15:27:50Z

@swift-ci Please test macOS

DmT021 · 2025-03-27T05:10:17Z

@swift-ci Please test

DmT021 · 2025-03-27T05:44:13Z

@swift-ci Please test macOS

DmT021 · 2025-03-27T13:16:29Z

@swift-ci Please test windows

DmT021 · 2025-06-10T15:04:29Z

@swift-ci Please test

DmT021 · 2025-06-10T22:20:11Z

@swift-ci Please test

DmT021 · 2025-06-11T12:32:54Z

@swift-ci Please test

DmT021 · 2025-06-11T16:38:26Z

@swift-ci Please smoke test

DmT021 · 2025-06-12T00:40:24Z

@swift-ci Please clean smoke test

DmT021 · 2025-06-12T00:42:00Z

@swift-ci Please smoke test

DmT021 · 2025-06-12T00:44:30Z

@swift-ci Please test

DmT021 · 2025-06-12T14:07:19Z

@swift-ci Please test macOS platform

bkhouri · 2025-06-12T14:26:18Z

+    }
+
+    func testWarningControlFlags() async throws {
+        try XCTSkipOnWindows(because: "https://github.com/swiftlang/swift-package-manager/issues/8543: there are compilation errors")


question: The references GitHub issue does not appear to be related to this test. Are we certain this test fails on Windows?

Yes, both testManifestGenerationWithWarningControlFlags and this test fail on Windows. And I think it is related to the issue. The CI job with the errors is already removed from the dashboard, so I can't provide you with the link, but it was showing the same type of errors as in the linked issue:

type 'CSetting' has no member 'enableWarning' type 'SwiftSetting' has no member 'treatAllWarnings' ...

If I understand the issue right, for some reason, tests on Windows use the wrong version of PackageDescription, not the one we build, so we don't see the newly added functions.

out of curiosity, do you recall if the failure occurred in the windows self hosted, windows smoke test, or both?

I believe it only fails in the windows self hosted builds. But I'm not certain.
I can remove the skips and rerun the tests if you want.

bkhouri · 2025-06-12T14:26:41Z

+import _InternalTestSupport
+import XCTest
+
+final class PackageDescription6_2LoadingTests: PackageDescriptionLoadingTests {


suggestion (non-blocking): Since this is a new test, consider migrating to Swift Testing

Done. I've marked it as an expected failure on Windows, so it will record the issue. A copy of the log is below

bkhouri · 2025-06-12T14:29:16Z

    }

+    func testManifestGenerationWithWarningControlFlags() async throws {
+        try XCTSkipOnWindows(because: "https://github.com/swiftlang/swift-package-manager/issues/8543: there are compilation errors")


question: The references GitHub issue does not appear to be related to this test. Are we certain this test fails on Windows?

DmT021 · 2025-06-14T04:19:47Z

@swift-ci Please test

DmT021 · 2025-06-14T16:44:46Z

The error on Swift Test Windows Platform (self hosted) (Swift 6.1)

Log

[2025-06-14T04:58:51.211Z] × Test warningControlFlags() recorded a known issue at PD_6_2_LoadingTests.swift:71:33: Caught error: Invalid manifest (compiled with: ["C:\\Users\\ContainerAdministrator\\AppData\\Local\\Programs\\Swift\\Toolchains\\6.1.0+Asserts\\usr\\bin\\swiftc.exe", "-vfsoverlay", "C:\\Users\\ContainerAdministrator\\AppData\\Local\\Temp\\TemporaryDirectory.k11eCm\\vfs.yaml", "-L", "C:\\Users\\ContainerAdministrator\\AppData\\Local\\Programs\\Swift\\Toolchains\\6.1.0+Asserts\\usr\\lib\\swift\\pm\\ManifestAPI", "-lPackageDescription", "-sdk", "C:\\Users\\ContainerAdministrator\\AppData\\Local\\Programs\\Swift\\Platforms\\6.1.0\\Windows.platform\\Developer\\SDKs\\Windows.sdk\\", "-libc", "MD", "-I", "C:\\Users\\ContainerAdministrator\\AppData\\Local\\Programs\\Swift\\Platforms\\6.1.0\\Windows.platform\\Developer\\Library\\XCTest-development\\usr\\lib\\swift\\windows", "-I", "C:\\Users\\ContainerAdministrator\\AppData\\Local\\Programs\\Swift\\Platforms\\6.1.0\\Windows.platform\\Developer\\Library\\XCTest-development\\usr\\lib\\swift\\windows\\x86_64", "-L", "C:\\Users\\ContainerAdministrator\\AppData\\Local\\Programs\\Swift\\Platforms\\6.1.0\\Windows.platform\\Developer\\Library\\XCTest-development\\usr\\lib\\swift\\windows\\x86_64", "-I", "C:\\Users\\ContainerAdministrator\\AppData\\Local\\Programs\\Swift\\Platforms\\6.1.0\\Windows.platform\\Developer\\Library\\Testing-development\\usr\\lib\\swift\\windows", "-L", "C:\\Users\\ContainerAdministrator\\AppData\\Local\\Programs\\Swift\\Platforms\\6.1.0\\Windows.platform\\Developer\\Library\\Testing-development\\usr\\lib\\swift\\windows\\x86_64", "-use-ld=lld", "-swift-version", "6", "-I", "C:\\Users\\ContainerAdministrator\\AppData\\Local\\Programs\\Swift\\Toolchains\\6.1.0+Asserts\\usr\\lib\\swift\\pm\\ManifestAPI", "-package-description-version", "6.2.0", "C:\\Package.swift", "-o", "C:\\Users\\ContainerAdministrator\\AppData\\Local\\Temp\\TemporaryDirectory.tyetO8\\-manifest.exe"])
[2025-06-14T04:58:51.211Z] C:\Package.swift:9:18: error: type 'CSetting' has no member 'enableWarning'
[2025-06-14T04:58:51.211Z]  7 |             name: "Foo",
[2025-06-14T04:58:51.211Z]  8 |             cSettings: [
[2025-06-14T04:58:51.211Z]  9 |                 .enableWarning("implicit-fallthrough"),
[2025-06-14T04:58:51.211Z]    |                  `- error: type 'CSetting' has no member 'enableWarning'
[2025-06-14T04:58:51.211Z] 10 |                 .disableWarning("unused-parameter"),
[2025-06-14T04:58:51.211Z] 11 |                 .treatAllWarnings(as: .error),
[2025-06-14T04:58:51.211Z] 
[2025-06-14T04:58:51.211Z] C:\Package.swift:10:18: error: type 'CSetting' has no member 'disableWarning'
[2025-06-14T04:58:51.211Z]  8 |             cSettings: [
[2025-06-14T04:58:51.211Z]  9 |                 .enableWarning("implicit-fallthrough"),
[2025-06-14T04:58:51.211Z] 10 |                 .disableWarning("unused-parameter"),
[2025-06-14T04:58:51.211Z]    |                  `- error: type 'CSetting' has no member 'disableWarning'
[2025-06-14T04:58:51.211Z] 11 |                 .treatAllWarnings(as: .error),
[2025-06-14T04:58:51.211Z] 12 |                 .treatWarning("deprecated-declarations", as: .warning),
[2025-06-14T04:58:51.211Z] 
[2025-06-14T04:58:51.211Z] C:\Package.swift:11:18: error: type 'CSetting' has no member 'treatAllWarnings'
[2025-06-14T04:58:51.211Z]  9 |                 .enableWarning("implicit-fallthrough"),
[2025-06-14T04:58:51.211Z] 10 |                 .disableWarning("unused-parameter"),
[2025-06-14T04:58:51.211Z] 11 |                 .treatAllWarnings(as: .error),
[2025-06-14T04:58:51.211Z]    |                  `- error: type 'CSetting' has no member 'treatAllWarnings'
[2025-06-14T04:58:51.211Z] 12 |                 .treatWarning("deprecated-declarations", as: .warning),
[2025-06-14T04:58:51.211Z] 13 |             ],
...

DmT021 · 2025-06-14T16:45:17Z

@swift-ci Please test Windows platform

DmT021 · 2025-06-15T01:06:26Z

@swift-ci Please test Windows platform

DmT021 · 2025-06-15T13:56:15Z

@swift-ci Please test Windows platform

DmT021 · 2025-06-16T16:25:30Z

@swift-ci Please test Windows platform

DmT021 · 2025-06-19T23:18:37Z

@bkhouri can you review this again, please? Or tag someone

bkhouri

I'm more familiar with the SwiftPM tests.

@dschaefer2 would be in a better position to approve this PR than me.

bkhouri · 2025-06-21T01:30:58Z

+                results.checkIsEmpty()
+            }
+        } when: {
+            isWindows


note (non-blocking): In SwiftPM, we can also use ProcessInfo.hostOperatingSystem == .windows

bkhouri · 2025-06-21T01:31:38Z

+import _InternalTestSupport
+import Testing
+
+struct PackageDescription6_2LoadingTests {


praise: It's great to a a new test suite being written in Swift Testing. thanks for this.

dschaefer2

I'm still concerned we're changing the Package Manifest without having a cohesive vision for it that we can measure this feature against. Unfortunately that's vision is going to take some time to put together.

Approving since it was an accepted evolution proposal. But I reserve the right to adjust it in the future :). Big things I'm wondering about are things like build configuration and platform specific build settings, build setting visibility like CMake has, etc. Not sure the current build settings syntax scales to that level of optionality, but that needs to be studied.

This change adds warning control settings as described in the [proposal](https://github.com/swiftlang/swift-evolution/blob/main/proposals/0480-swiftpm-warning-control.md). The flags for Swift targets are described by SE-0443, for Clang targets - [here](https://clang.llvm.org/docs/UsersManual.html#options-to-control-error-and-warning-messages).

DmT021 mentioned this pull request Mar 5, 2025

Rethinking support for warning control flags (SE-0443) swiftlang/swift-build#248

Open

DmT021 marked this pull request as ready for review March 9, 2025 18:23

DmT021 requested review from MaxDesiatov, bkhouri, bnbarham, bripeticca, dschaefer2, jakepetroules, owenv, plemarquand, shawnhyam and xedin as code owners March 9, 2025 18:23

DougGregor approved these changes Mar 12, 2025

View reviewed changes

DmT021 requested review from cmcgee1024 and daveyc123 as code owners March 26, 2025 10:08

DmT021 force-pushed the wp/warning-rules branch from 92fdcd3 to a51a5de Compare March 26, 2025 12:55

DmT021 force-pushed the wp/warning-rules branch from a51a5de to 5bbc023 Compare March 27, 2025 05:09

DmT021 force-pushed the wp/warning-rules branch from 5bbc023 to b78ab2c Compare June 10, 2025 15:04

DmT021 force-pushed the wp/warning-rules branch from b78ab2c to d6c1f61 Compare June 10, 2025 22:19

Build settings for warning control flags

13e33e9

DmT021 force-pushed the wp/warning-rules branch from d6c1f61 to 13e33e9 Compare June 11, 2025 12:32

DmT021 changed the title ~~Build settings for warning treating rules (SE-0443, draft)~~ [SE-0480] Build settings for warning control flags Jun 11, 2025

bkhouri reviewed Jun 12, 2025

View reviewed changes

Update PackageDescription6_2LoadingTests to use swift-testing

b05555d

bkhouri reviewed Jun 21, 2025

View reviewed changes

dschaefer2 approved these changes Jun 24, 2025

View reviewed changes

dschaefer2 merged commit 065df19 into swiftlang:main Jun 24, 2025
6 checks passed

This was referenced Sep 26, 2025

"InternalError: Unexpected \'-Wwarning\' or \'-Werror\' at the end of args" when building with -Xcc -Werror #9192

Open

fix: remove preceding -Xcc instead of the argument following -Werror or -Wwarning #9194

Open

coderabbitai Bot mentioned this pull request Apr 26, 2026

chore(v3.1.0): quality improvements roadmap (P0-P3 backlog) InnoSquadCorp/InnoRouter#23

Merged

40 tasks

Conversation

DmT021 commented Feb 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DougGregor commented Mar 12, 2025

Uh oh!

DougGregor left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DmT021 Mar 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dschaefer2 commented Mar 12, 2025

Uh oh!

dschaefer2 commented Mar 12, 2025

Uh oh!

DmT021 commented Mar 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DmT021 commented Mar 15, 2025

Uh oh!

dschaefer2 commented Mar 17, 2025

Uh oh!

DmT021 commented Mar 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DmT021 commented Mar 26, 2025

Uh oh!

DmT021 commented Mar 26, 2025

Uh oh!

DmT021 commented Mar 27, 2025

Uh oh!

DmT021 commented Mar 27, 2025

Uh oh!

DmT021 commented Mar 27, 2025

Uh oh!

DmT021 commented Jun 10, 2025

Uh oh!

DmT021 commented Jun 10, 2025

Uh oh!

DmT021 commented Jun 11, 2025

Uh oh!

DmT021 commented Jun 11, 2025

Uh oh!

DmT021 commented Jun 12, 2025

Uh oh!

DmT021 commented Jun 12, 2025

Uh oh!

DmT021 commented Jun 12, 2025

Uh oh!

DmT021 commented Jun 12, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DmT021 Jun 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bkhouri Jun 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DmT021 commented Jun 14, 2025

Uh oh!

DmT021 commented Jun 14, 2025

Uh oh!

DmT021 commented Jun 14, 2025

DmT021 commented Feb 27, 2025 •

edited

Loading

DmT021 Mar 12, 2025 •

edited

Loading

DmT021 commented Mar 12, 2025 •

edited

Loading

DmT021 commented Mar 18, 2025 •

edited

Loading

DmT021 Jun 12, 2025 •

edited

Loading

bkhouri Jun 12, 2025 •

edited

Loading