Skip to content

Commit 070c77e

Browse files
DmT021DmT021
committed
fixup! [Diagnostics] Add -[no-]warning-as-error flags for precise control over warning behavior
1 parent 28883b6 commit 070c77e

File tree

6 files changed

+105
-49
lines changed

6 files changed

+105
-49
lines changed

‎include/swift/Basic/WarningAsErrorRule.h

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,10 @@
1313
#ifndef SWIFT_BASIC_WARNINGASERRORRULE_H
1414
#define SWIFT_BASIC_WARNINGASERRORRULE_H
1515

16+
#include "llvm/Support/ErrorHandling.h"
1617
#include <string>
1718
#include <variant>
19+
#include <vector>
1820

1921
namespace swift {
2022

@@ -38,6 +40,31 @@ class WarningAsErrorRule {
3840

3941
Target getTarget() const { return target; }
4042

43+
static bool hasConflictsWithSuppressWarnings(
44+
const std::vector<WarningAsErrorRule> &rules) {
45+
bool warningsAsErrorsAllEnabled = false;
46+
for (const auto &rule : rules) {
47+
const auto target = rule.getTarget();
48+
if (std::holds_alternative<TargetAll>(target)) {
49+
// Only `-warnings-as-errors` conflicts with `-suppress-warnings`
50+
switch (rule.getAction()) {
51+
case WarningAsErrorRule::Action::Enable:
52+
warningsAsErrorsAllEnabled = true;
53+
break;
54+
case WarningAsErrorRule::Action::Disable:
55+
warningsAsErrorsAllEnabled = false;
56+
break;
57+
}
58+
} else if (std::holds_alternative<TargetGroup>(target)) {
59+
// Both `-Wwarning` and `-Werror` conflict with `-suppress-warnings`
60+
return true;
61+
} else {
62+
llvm_unreachable("unhandled WarningAsErrorRule::Target");
63+
}
64+
}
65+
return warningsAsErrorsAllEnabled;
66+
}
67+
4168
private:
4269
Action action;
4370
Target target;

‎include/swift/Option/Options.td

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -806,7 +806,7 @@ def warnings_as_errors : Flag<["-"], "warnings-as-errors">,
806806
Flags<[FrontendOption]>,
807807
HelpText<"Treat warnings as errors">;
808808

809-
def warning_as_error : Separate<["-"], "warning-as-error">,
809+
def Werror : Separate<["-"], "Werror">,
810810
Group<warning_treating_Group>,
811811
Flags<[FrontendOption, HelpHidden]>,
812812
MetaVarName<"<diagnostic_group>">,
@@ -815,14 +815,14 @@ def warning_as_error : Separate<["-"], "warning-as-error">,
815815
def no_warnings_as_errors : Flag<["-"], "no-warnings-as-errors">,
816816
Group<warning_treating_Group>,
817817
Flags<[FrontendOption]>,
818-
HelpText<"Don't treat warnings as errors">;
818+
HelpText<"Treat warnings as warnings">;
819819

820-
def no_warning_as_error : Separate<["-"], "no-warning-as-error">,
820+
def Wwarning : Separate<["-"], "Wwarning">,
821821
Group<warning_treating_Group>,
822822
Flags<[FrontendOption, HelpHidden]>,
823823
MetaVarName<"<diagnostic_group>">,
824-
HelpText<"Don't treat this warning group as error">;
825-
824+
HelpText<"Treat this warning group as warning">;
825+
826826
def suppress_remarks : Flag<["-"], "suppress-remarks">,
827827
Flags<[FrontendOption]>,
828828
HelpText<"Suppress all remarks">;

‎lib/Driver/Driver.cpp

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -158,11 +158,20 @@ static void validateBridgingHeaderArgs(DiagnosticEngine &diags,
158158

159159
static void validateWarningControlArgs(DiagnosticEngine &diags,
160160
const ArgList &args) {
161-
if (args.hasArg(options::OPT_suppress_warnings) &&
162-
args.hasFlag(options::OPT_warnings_as_errors,
163-
options::OPT_no_warnings_as_errors, false)) {
164-
diags.diagnose(SourceLoc(), diag::error_conflicting_options,
165-
"-warnings-as-errors", "-suppress-warnings");
161+
if (args.hasArg(options::OPT_suppress_warnings)) {
162+
if (args.hasFlag(options::OPT_warnings_as_errors,
163+
options::OPT_no_warnings_as_errors, false)) {
164+
diags.diagnose(SourceLoc(), diag::error_conflicting_options,
165+
"-warnings-as-errors", "-suppress-warnings");
166+
}
167+
if (args.hasArg(options::OPT_Wwarning)) {
168+
diags.diagnose(SourceLoc(), diag::error_conflicting_options, "-Wwarning",
169+
"-suppress-warnings");
170+
}
171+
if (args.hasArg(options::OPT_Werror)) {
172+
diags.diagnose(SourceLoc(), diag::error_conflicting_options, "-Werror",
173+
"-suppress-warnings");
174+
}
166175
}
167176
}
168177

‎lib/Frontend/CompilerInvocation.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2268,10 +2268,10 @@ static bool ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args,
22682268
return WarningAsErrorRule(WarningAsErrorRule::Action::Enable);
22692269
case OPT_no_warnings_as_errors:
22702270
return WarningAsErrorRule(WarningAsErrorRule::Action::Disable);
2271-
case OPT_warning_as_error:
2271+
case OPT_Werror:
22722272
return WarningAsErrorRule(WarningAsErrorRule::Action::Enable,
22732273
arg->getValue());
2274-
case OPT_no_warning_as_error:
2274+
case OPT_Wwarning:
22752275
return WarningAsErrorRule(WarningAsErrorRule::Action::Disable,
22762276
arg->getValue());
22772277
default:
@@ -2321,6 +2321,10 @@ static bool ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args,
23212321
Opts.LocalizationPath = A->getValue();
23222322
}
23232323
}
2324+
assert(!(Opts.SuppressWarnings &&
2325+
WarningAsErrorRule::hasConflictsWithSuppressWarnings(
2326+
Opts.WarningsAsErrorsRules)) &&
2327+
"conflicting arguments; should have been caught by driver");
23242328

23252329
return false;
23262330
}

‎test/Driver/warnings-control.swift

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,14 @@
22
// RUN: not %target-swiftc_driver -warnings-as-errors %s 2>&1 | %FileCheck -check-prefix=WERR %s
33
// RUN: not %target-swiftc_driver -suppress-warnings %s 2>&1 | %FileCheck -check-prefix=NOWARN %s
44

5-
// RUN: not %target-swiftc_driver -suppress-warnings -warnings-as-errors %s 2>&1 | %FileCheck -check-prefix=FLAGS_CONFLICT %s
6-
// FLAGS_CONFLICT: error: conflicting options '-warnings-as-errors' and '-suppress-warnings'
5+
// RUN: not %target-swiftc_driver -suppress-warnings -warnings-as-errors %s 2>&1 | %FileCheck -check-prefix=FLAGS_CONFLICT_WAE %s
6+
// FLAGS_CONFLICT_WAE: error: conflicting options '-warnings-as-errors' and '-suppress-warnings'
7+
8+
// RUN: not %target-swiftc_driver -suppress-warnings -Wwarning test %s 2>&1 | %FileCheck -check-prefix=FLAGS_CONFLICT_WW %s
9+
// FLAGS_CONFLICT_WW: error: conflicting options '-Wwarning' and '-suppress-warnings'
10+
11+
// RUN: not %target-swiftc_driver -suppress-warnings -Werror test %s 2>&1 | %FileCheck -check-prefix=FLAGS_CONFLICT_WE %s
12+
// FLAGS_CONFLICT_WE: error: conflicting options '-Werror' and '-suppress-warnings'
713

814
func foo() -> Int {
915
let x = 1

‎test/diagnostics/warnings_as_errors_rules.swift

Lines changed: 45 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,20 @@
1-
// RUN: not %target-swift-frontend -typecheck -diagnostic-style llvm -warnings-as-errors %s 2>&1 | %FileCheck %s --check-prefix=CHECK-WAE-ALL
2-
// RUN: not %target-swift-frontend -typecheck -diagnostic-style llvm -warning-as-error availability_deprecated %s 2>&1 | %FileCheck %s --check-prefix=CHECK-WAE-GROUP
3-
// RUN: not %target-swift-frontend -typecheck -diagnostic-style llvm -warning-as-error deprecated %s 2>&1 | %FileCheck %s --check-prefix=CHECK-WAE-SUPERGROUP
4-
// RUN: %target-swift-frontend -typecheck -diagnostic-style llvm -warnings-as-errors -no-warnings-as-errors %s 2>&1 | %FileCheck %s --check-prefix=CHECK-WAE-ALL-NWAE-ALL
5-
// RUN: %target-swift-frontend -typecheck -diagnostic-style llvm -warnings-as-errors -no-warning-as-error availability_deprecated %s 2>&1 | %FileCheck %s --check-prefix=CHECK-WAE-ALL-NWAE-GROUP
6-
// RUN: %target-swift-frontend -typecheck -diagnostic-style llvm -warnings-as-errors -no-warning-as-error deprecated %s 2>&1 | %FileCheck %s --check-prefix=CHECK-WAE-ALL-NWAE-SUPERGROUP
7-
// RUN: %target-swift-frontend -typecheck -diagnostic-style llvm -warning-as-error deprecated -no-warning-as-error availability_deprecated %s 2>&1 | %FileCheck %s --check-prefix=CHECK-WAE-SUPERGROUP-NWAE-GROUP
1+
// RUN: not %target-swift-frontend -typecheck -diagnostic-style llvm -warnings-as-errors %s 2>&1 | %FileCheck %s --check-prefix=CHECK-WAE
2+
// RUN: not %target-swift-frontend -typecheck -diagnostic-style llvm -Werror availability_deprecated %s 2>&1 | %FileCheck %s --check-prefix=CHECK-WE-GROUP
3+
// RUN: not %target-swift-frontend -typecheck -diagnostic-style llvm -Werror deprecated %s 2>&1 | %FileCheck %s --check-prefix=CHECK-WE-SUPERGROUP
4+
// RUN: %target-swift-frontend -typecheck -diagnostic-style llvm -warnings-as-errors -no-warnings-as-errors %s 2>&1 | %FileCheck %s --check-prefix=CHECK-WAE-NWAE
5+
// RUN: %target-swift-frontend -typecheck -diagnostic-style llvm -warnings-as-errors -Wwarning availability_deprecated %s 2>&1 | %FileCheck %s --check-prefix=CHECK-WAE-WW-GROUP
6+
// RUN: %target-swift-frontend -typecheck -diagnostic-style llvm -warnings-as-errors -Wwarning deprecated %s 2>&1 | %FileCheck %s --check-prefix=CHECK-WAE-WW-SUPERGROUP
7+
// RUN: %target-swift-frontend -typecheck -diagnostic-style llvm -Werror deprecated -Wwarning availability_deprecated %s 2>&1 | %FileCheck %s --check-prefix=CHECK-WE-SUPERGROUP-WW-GROUP
8+
9+
// This test verifies that the warning control flags apply with respect to
10+
// the order they are specified in the cmd line.
11+
// Naming:
12+
// WAE: -warnings-as-errors
13+
// NWAE: -no-warnings-as-errors
14+
// WE-xxxx: -Werror xxxx
15+
// WW-xxxx: -Wwarning xxxx
16+
// GROUP - refers to a narrower group
17+
// SUPERGROUP - refers to a broader group that includes GROUP
818

919

1020
@available(*, deprecated)
@@ -16,35 +26,35 @@ func bar() {
1626
}
1727

1828

19-
// CHECK-WAE-ALL: error: 'foo()' is deprecated
20-
// CHECK-WAE-ALL-NOT: warning: 'foo()' is deprecated
21-
// CHECK-WAE-GROUP: error: 'foo()' is deprecated
22-
// CHECK-WAE-GROUP-NOT: warning: 'foo()' is deprecated
23-
// CHECK-WAE-SUPERGROUP: error: 'foo()' is deprecated
24-
// CHECK-WAE-SUPERGROUP-NOT: warning: 'foo()' is deprecated
25-
// CHECK-WAE-ALL-NWAE-ALL: warning: 'foo()' is deprecated
26-
// CHECK-WAE-ALL-NWAE-ALL-NOT: error: 'foo()' is deprecated
27-
// CHECK-WAE-ALL-NWAE-GROUP: warning: 'foo()' is deprecated
28-
// CHECK-WAE-ALL-NWAE-GROUP-NOT: error: 'foo()' is deprecated
29-
// CHECK-WAE-ALL-NWAE-SUPERGROUP: warning: 'foo()' is deprecated
30-
// CHECK-WAE-ALL-NWAE-SUPERGROUP-NOT: error: 'foo()' is deprecated
31-
// CHECK-WAE-SUPERGROUP-NWAE-GROUP: warning: 'foo()' is deprecated
32-
// CHECK-WAE-SUPERGROUP-NWAE-GROUP-NOT: error: 'foo()' is deprecated
29+
// CHECK-WAE: error: 'foo()' is deprecated
30+
// CHECK-WAE-NOT: warning: 'foo()' is deprecated
31+
// CHECK-WE-GROUP: error: 'foo()' is deprecated
32+
// CHECK-WE-GROUP-NOT: warning: 'foo()' is deprecated
33+
// CHECK-WE-SUPERGROUP: error: 'foo()' is deprecated
34+
// CHECK-WE-SUPERGROUP-NOT: warning: 'foo()' is deprecated
35+
// CHECK-WAE-NWAE: warning: 'foo()' is deprecated
36+
// CHECK-WAE-NWAE-NOT: error: 'foo()' is deprecated
37+
// CHECK-WAE-WW-GROUP: warning: 'foo()' is deprecated
38+
// CHECK-WAE-WW-GROUP-NOT: error: 'foo()' is deprecated
39+
// CHECK-WAE-WW-SUPERGROUP: warning: 'foo()' is deprecated
40+
// CHECK-WAE-WW-SUPERGROUP-NOT: error: 'foo()' is deprecated
41+
// CHECK-WE-SUPERGROUP-WW-GROUP: warning: 'foo()' is deprecated
42+
// CHECK-WE-SUPERGROUP-WW-GROUP-NOT: error: 'foo()' is deprecated
3343
foo()
3444

3545

36-
// CHECK-WAE-ALL: error: 'bar()' is deprecated: renamed to 'bar2'
37-
// CHECK-WAE-ALL-NOT: warning: 'bar()' is deprecated: renamed to 'bar2'
38-
// CHECK-WAE-GROUP: error: 'bar()' is deprecated: renamed to 'bar2'
39-
// CHECK-WAE-GROUP-NOT: warning: 'bar()' is deprecated: renamed to 'bar2'
40-
// CHECK-WAE-SUPERGROUP: error: 'bar()' is deprecated: renamed to 'bar2'
41-
// CHECK-WAE-SUPERGROUP-NOT: warning: 'bar()' is deprecated: renamed to 'bar2'
42-
// CHECK-WAE-ALL-NWAE-ALL: warning: 'bar()' is deprecated: renamed to 'bar2'
43-
// CHECK-WAE-ALL-NWAE-ALL-NOT: error: 'bar()' is deprecated: renamed to 'bar2'
44-
// CHECK-WAE-ALL-NWAE-GROUP: warning: 'bar()' is deprecated: renamed to 'bar2'
45-
// CHECK-WAE-ALL-NWAE-GROUP-NOT: error: 'bar()' is deprecated: renamed to 'bar2'
46-
// CHECK-WAE-ALL-NWAE-SUPERGROUP: warning: 'bar()' is deprecated: renamed to 'bar2'
47-
// CHECK-WAE-ALL-NWAE-SUPERGROUP-NOT: error: 'bar()' is deprecated: renamed to 'bar2'
48-
// CHECK-WAE-SUPERGROUP-NWAE-GROUP: warning: 'bar()' is deprecated: renamed to 'bar2'
49-
// CHECK-WAE-SUPERGROUP-NWAE-GROUP-NOT: error: 'bar()' is deprecated: renamed to 'bar2'
46+
// CHECK-WAE: error: 'bar()' is deprecated: renamed to 'bar2'
47+
// CHECK-WAE-NOT: warning: 'bar()' is deprecated: renamed to 'bar2'
48+
// CHECK-WE-GROUP: error: 'bar()' is deprecated: renamed to 'bar2'
49+
// CHECK-WE-GROUP-NOT: warning: 'bar()' is deprecated: renamed to 'bar2'
50+
// CHECK-WE-SUPERGROUP: error: 'bar()' is deprecated: renamed to 'bar2'
51+
// CHECK-WE-SUPERGROUP-NOT: warning: 'bar()' is deprecated: renamed to 'bar2'
52+
// CHECK-WAE-NWAE: warning: 'bar()' is deprecated: renamed to 'bar2'
53+
// CHECK-WAE-NWAE-NOT: error: 'bar()' is deprecated: renamed to 'bar2'
54+
// CHECK-WAE-WW-GROUP: warning: 'bar()' is deprecated: renamed to 'bar2'
55+
// CHECK-WAE-WW-GROUP-NOT: error: 'bar()' is deprecated: renamed to 'bar2'
56+
// CHECK-WAE-WW-SUPERGROUP: warning: 'bar()' is deprecated: renamed to 'bar2'
57+
// CHECK-WAE-WW-SUPERGROUP-NOT: error: 'bar()' is deprecated: renamed to 'bar2'
58+
// CHECK-WE-SUPERGROUP-WW-GROUP: warning: 'bar()' is deprecated: renamed to 'bar2'
59+
// CHECK-WE-SUPERGROUP-WW-GROUP-NOT: error: 'bar()' is deprecated: renamed to 'bar2'
5060
bar()

0 commit comments

Comments
 (0)