Skip to content

Commit 89cd4ba

Browse files
committed
Add capability to fully suppress a warning group and have warnings default-escalate to errors
- Adds '-Wsuppress <group>' flag which can be used to fully supress emission of warning diagnostics belonging to the specified group. - Adds 'DefaultEscalate' flag which can be used on grouped warning diagnostics in order to have them escalated to errors by-default. They can still be downgraded back to warnings with '-Wwarning'.
1 parent 9deaa13 commit 89cd4ba

File tree

11 files changed

+133
-59
lines changed

11 files changed

+133
-59
lines changed

include/swift/AST/DiagnosticEngine.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
#include "swift/Basic/PrintDiagnosticNamesMode.h"
2626
#include "swift/Basic/Statistic.h"
2727
#include "swift/Basic/Version.h"
28-
#include "swift/Basic/WarningAsErrorRule.h"
28+
#include "swift/Basic/WarningTreatmentRule.h"
2929
#include "swift/Localization/LocalizationFormat.h"
3030
#include "llvm/ADT/BitVector.h"
3131
#include "llvm/ADT/StringRef.h"
@@ -911,12 +911,12 @@ namespace swift {
911911
}
912912

913913
/// Apply rules specifing what warnings should or shouldn't be treated as
914-
/// errors. For group rules the string is a group name defined by
915-
/// DiagnosticGroups.def
914+
/// errors or be suppressed. For group rules the string is a group name
915+
/// defined by DiagnosticGroups.def.
916916
/// Rules are applied in order they appear in the vector.
917917
/// In case the vector contains rules affecting the same diagnostic ID
918918
/// the last rule wins.
919-
void setWarningsAsErrorsRules(const std::vector<WarningAsErrorRule> &rules);
919+
void setWarningTreatmentRules(const std::vector<WarningTreatmentRule> &rules);
920920

921921
/// Whether to print diagnostic names after their messages
922922
void setPrintDiagnosticNamesMode(PrintDiagnosticNamesMode val) {

include/swift/AST/DiagnosticsSema.def

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8939,10 +8939,10 @@ ERROR(attr_inline_always_no_usable_from_inline,none,
89398939
// MARK: Swift Performance hints
89408940
//===----------------------------------------------------------------------===//
89418941

8942-
GROUPED_WARNING(perf_hint_function_returns_array,PerfHintReturnTypeImplicitCopy,none,
8942+
GROUPED_WARNING(perf_hint_function_returns_array,PerfHintReturnTypeImplicitCopy,DefaultEscalate,
89438943
"Performance: %0 returns a%select{ dictionary|n array}1, leading to implicit copies. "
89448944
"Consider using an 'inout' parameter instead.", (const FuncDecl *, bool))
8945-
GROUPED_WARNING(perf_hint_closure_returns_array,PerfHintReturnTypeImplicitCopy,none,
8945+
GROUPED_WARNING(perf_hint_closure_returns_array,PerfHintReturnTypeImplicitCopy,DefaultEscalate,
89468946
"Performance: closure returns a%select{ dictionary|n array}0, leading to implicit copies. "
89478947
"Consider using an 'inout' parameter instead.", (bool))
89488948

include/swift/Basic/DiagnosticOptions.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
#define SWIFT_BASIC_DIAGNOSTICOPTIONS_H
1515

1616
#include "swift/Basic/PrintDiagnosticNamesMode.h"
17-
#include "swift/Basic/WarningAsErrorRule.h"
17+
#include "swift/Basic/WarningTreatmentRule.h"
1818
#include "llvm/ADT/Hashing.h"
1919
#include <vector>
2020

@@ -61,8 +61,8 @@ class DiagnosticOptions {
6161
/// Suppress all remarks
6262
bool SuppressRemarks = false;
6363

64-
/// Rules for escalating warnings to errors
65-
std::vector<WarningAsErrorRule> WarningsAsErrorsRules;
64+
/// Rules for suppressing or escalating warnings to errors
65+
std::vector<WarningTreatmentRule> WarningTreatmentRules;
6666

6767
/// When printing diagnostics, include either the diagnostic name
6868
/// (diag::whatever) at the end or the associated diagnostic group.
Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,17 @@
1-
//===--- WarningAsErrorRule.h -----------------------------------*- C++ -*-===//
1+
//===--- WarningTreatmentRule.h -----------------------------------*- C++ -*-===//
22
//
33
// This source file is part of the Swift.org open source project
44
//
5-
// Copyright (c) 2014 - 2024 Apple Inc. and the Swift project authors
5+
// Copyright (c) 2014 - 2025 Apple Inc. and the Swift project authors
66
// Licensed under Apache License v2.0 with Runtime Library Exception
77
//
88
// See https://swift.org/LICENSE.txt for license information
99
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
1010
//
1111
//===----------------------------------------------------------------------===//
1212

13-
#ifndef SWIFT_BASIC_WARNINGASERRORRULE_H
14-
#define SWIFT_BASIC_WARNINGASERRORRULE_H
13+
#ifndef SWIFT_BASIC_WARNINGTREATMENTRULE_H
14+
#define SWIFT_BASIC_WARNINGTREATMENTRULE_H
1515

1616
#include "llvm/Support/ErrorHandling.h"
1717
#include <string>
@@ -21,45 +21,49 @@
2121
namespace swift {
2222

2323
/// Describes a rule how to treat a warning or all warnings.
24-
class WarningAsErrorRule {
24+
class WarningTreatmentRule {
2525
public:
26-
enum class Action { Disable, Enable };
26+
enum class Action { AsWarning, AsError, Suppress };
2727
struct TargetAll {};
2828
struct TargetGroup {
2929
std::string name;
3030
};
3131
using Target = std::variant<TargetAll, TargetGroup>;
3232

3333
/// Init as a rule targeting all diagnostic groups
34-
WarningAsErrorRule(Action action) : action(action), target(TargetAll()) {}
34+
WarningTreatmentRule(Action action) : action(action), target(TargetAll()) {}
3535
/// Init as a rule targeting a specific diagnostic group
36-
WarningAsErrorRule(Action action, const std::string &group)
36+
WarningTreatmentRule(Action action, const std::string &group)
3737
: action(action), target(TargetGroup{group}) {}
3838

3939
Action getAction() const { return action; }
4040

4141
Target getTarget() const { return target; }
4242

4343
static bool hasConflictsWithSuppressWarnings(
44-
const std::vector<WarningAsErrorRule> &rules) {
44+
const std::vector<WarningTreatmentRule> &rules) {
4545
bool warningsAsErrorsAllEnabled = false;
4646
for (const auto &rule : rules) {
4747
const auto target = rule.getTarget();
4848
if (std::holds_alternative<TargetAll>(target)) {
4949
// Only `-warnings-as-errors` conflicts with `-suppress-warnings`
5050
switch (rule.getAction()) {
51-
case WarningAsErrorRule::Action::Enable:
51+
case Action::AsError:
5252
warningsAsErrorsAllEnabled = true;
5353
break;
54-
case WarningAsErrorRule::Action::Disable:
54+
case Action::AsWarning:
5555
warningsAsErrorsAllEnabled = false;
5656
break;
57+
case Action::Suppress:
58+
llvm_unreachable("cannot suppress all warnings");
59+
break;
5760
}
5861
} else if (std::holds_alternative<TargetGroup>(target)) {
5962
// Both `-Wwarning` and `-Werror` conflict with `-suppress-warnings`
60-
return true;
63+
if (rule.getAction() == Action::AsError || rule.getAction() == Action::AsWarning)
64+
return true;
6165
} else {
62-
llvm_unreachable("unhandled WarningAsErrorRule::Target");
66+
llvm_unreachable("unhandled WarningTreatmentRule::Target");
6367
}
6468
}
6569
return warningsAsErrorsAllEnabled;
@@ -72,4 +76,4 @@ class WarningAsErrorRule {
7276

7377
} // end namespace swift
7478

75-
#endif // SWIFT_BASIC_WARNINGASERRORRULE_H
79+
#endif // SWIFT_BASIC_WARNINGTREATMENTRULE_H

include/swift/Option/Options.td

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -929,6 +929,12 @@ def Wwarning : Separate<["-"], "Wwarning">,
929929
MetaVarName<"<diagnostic_group>">,
930930
HelpText<"Treat this warning group as warning">;
931931

932+
def Wsuppress : Separate<["-"], "Wsuppress">,
933+
Group<warning_treating_Group>,
934+
Flags<[FrontendOption, HelpHidden]>,
935+
MetaVarName<"<diagnostic_group>">,
936+
HelpText<"Suppress this warning group">;
937+
932938
def suppress_remarks : Flag<["-"], "suppress-remarks">,
933939
Flags<[FrontendOption]>,
934940
HelpText<"Suppress all remarks">;

lib/AST/DiagnosticEngine.cpp

Lines changed: 44 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,11 @@ enum class DiagnosticOptions {
8282
/// by various warning options (-Wwarning, -Werror). This only makes sense
8383
/// for warnings.
8484
DefaultIgnore,
85+
86+
/// This warning diagnostic should be escalated to an error by default,
87+
/// but can be downgraded accordingly with -Wwarning. This only makes sense
88+
/// for warnings
89+
DefaultEscalate
8590
};
8691
struct StoredDiagnosticInfo {
8792
DiagnosticKind kind : 2;
@@ -91,16 +96,19 @@ struct StoredDiagnosticInfo {
9196
bool isDeprecation : 1;
9297
bool isNoUsage : 1;
9398
bool defaultIgnore : 1;
99+
bool defaultEscalate : 1;
94100
DiagGroupID groupID;
95101

96102
constexpr StoredDiagnosticInfo(DiagnosticKind k, bool firstBadToken,
97103
bool fatal, bool isAPIDigesterBreakage,
98104
bool deprecation, bool noUsage,
99-
bool defaultIgnore, DiagGroupID groupID)
105+
bool defaultIgnore, bool defaultEscalate,
106+
DiagGroupID groupID)
100107
: kind(k), pointsToFirstBadToken(firstBadToken), isFatal(fatal),
101108
isAPIDigesterBreakage(isAPIDigesterBreakage),
102109
isDeprecation(deprecation), isNoUsage(noUsage),
103-
defaultIgnore(defaultIgnore), groupID(groupID) {}
110+
defaultIgnore(defaultIgnore), defaultEscalate(defaultEscalate),
111+
groupID(groupID) {}
104112
constexpr StoredDiagnosticInfo(DiagnosticKind k, DiagnosticOptions opts,
105113
DiagGroupID groupID)
106114
: StoredDiagnosticInfo(k,
@@ -110,6 +118,7 @@ struct StoredDiagnosticInfo {
110118
opts == DiagnosticOptions::Deprecation,
111119
opts == DiagnosticOptions::NoUsage,
112120
opts == DiagnosticOptions::DefaultIgnore,
121+
opts == DiagnosticOptions::DefaultEscalate,
113122
groupID) {}
114123
};
115124
} // end anonymous namespace
@@ -162,6 +171,21 @@ DiagnosticState::DiagnosticState() {
162171

163172
// Initialize warningsAsErrors to default
164173
warningsAsErrors.resize(DiagGroupsCount);
174+
175+
// Initialize warning diagnostics which default to
176+
// being emitted as errors (DefaultEscalate)
177+
for (const auto &info : storedDiagnosticInfos) {
178+
if (info.defaultEscalate) {
179+
180+
if (info.groupID != DiagGroupID::no_group) {
181+
getDiagGroupInfoByID(info.groupID).traverseDepthFirst([&](auto group) {
182+
warningsAsErrors[(unsigned)info.groupID] = true;
183+
for (DiagID diagID : group.diagnostics)
184+
ignoredDiagnostics[(unsigned)diagID] = false;
185+
});
186+
}
187+
}
188+
}
165189
}
166190

167191
Diagnostic::Diagnostic(DiagID ID)
@@ -567,38 +591,44 @@ bool DiagnosticEngine::finishProcessing() {
567591
return hadError;
568592
}
569593

570-
void DiagnosticEngine::setWarningsAsErrorsRules(
571-
const std::vector<WarningAsErrorRule> &rules) {
594+
void DiagnosticEngine::setWarningTreatmentRules(
595+
const std::vector<WarningTreatmentRule> &rules) {
572596
std::vector<std::string> unknownGroups;
573597
for (const auto &rule : rules) {
574-
bool isEnabled = [&] {
598+
bool treatAsError = [&] {
575599
switch (rule.getAction()) {
576-
case WarningAsErrorRule::Action::Enable:
600+
case WarningTreatmentRule::Action::AsError:
577601
return true;
578-
case WarningAsErrorRule::Action::Disable:
602+
case WarningTreatmentRule::Action::AsWarning:
603+
case WarningTreatmentRule::Action::Suppress:
579604
return false;
580605
}
581606
}();
582607
auto target = rule.getTarget();
583-
if (auto group = std::get_if<WarningAsErrorRule::TargetGroup>(&target)) {
608+
if (auto group = std::get_if<WarningTreatmentRule::TargetGroup>(&target)) {
584609
auto name = std::string_view(group->name);
585610
// Validate the group name and set the new behavior for each diagnostic
586611
// associated with the group and all its subgroups.
587612
if (auto groupID = getDiagGroupIDByName(name);
588613
groupID && *groupID != DiagGroupID::no_group) {
589614
getDiagGroupInfoByID(*groupID).traverseDepthFirst([&](auto group) {
590-
state.setWarningsAsErrorsForDiagGroupID(*groupID, isEnabled);
591-
for (DiagID diagID : group.diagnostics) {
592-
state.setIgnoredDiagnostic(diagID, false);
615+
if (rule.getAction() == WarningTreatmentRule::Action::Suppress) {
616+
for (DiagID diagID : group.diagnostics)
617+
state.setIgnoredDiagnostic(diagID, true);
618+
}
619+
else {
620+
state.setWarningsAsErrorsForDiagGroupID(*groupID, treatAsError);
621+
for (DiagID diagID : group.diagnostics)
622+
state.setIgnoredDiagnostic(diagID, false);
593623
}
594624
});
595625
} else {
596626
unknownGroups.push_back(std::string(name));
597627
}
598-
} else if (std::holds_alternative<WarningAsErrorRule::TargetAll>(target)) {
599-
state.setAllWarningsAsErrors(isEnabled);
628+
} else if (std::holds_alternative<WarningTreatmentRule::TargetAll>(target)) {
629+
state.setAllWarningsAsErrors(treatAsError);
600630
} else {
601-
llvm_unreachable("unhandled WarningAsErrorRule::Target");
631+
llvm_unreachable("unhandled WarningTreatmentRule::Target");
602632
}
603633
}
604634
for (const auto &unknownGroup : unknownGroups) {

lib/Frontend/CompilerInvocation.cpp

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2646,31 +2646,36 @@ static bool ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args,
26462646
// If the "embedded" flag was provided, enable the EmbeddedRestrictions
26472647
// warning group. This group is opt-in in non-Embedded builds.
26482648
if (isEmbedded(Args)) {
2649-
Opts.WarningsAsErrorsRules.push_back(
2650-
WarningAsErrorRule(WarningAsErrorRule::Action::Disable,
2651-
"EmbeddedRestrictions"));
2649+
Opts.WarningTreatmentRules.push_back(
2650+
WarningTreatmentRule(WarningTreatmentRule::Action::AsWarning,
2651+
"EmbeddedRestrictions"));
26522652
}
26532653

26542654
Opts.SuppressWarnings |= Args.hasArg(OPT_suppress_warnings);
26552655
Opts.SuppressRemarks |= Args.hasArg(OPT_suppress_remarks);
26562656
for (const Arg *arg : Args.filtered(OPT_warning_treating_Group)) {
2657-
Opts.WarningsAsErrorsRules.push_back([&] {
2657+
Opts.WarningTreatmentRules.push_back([&] {
26582658
switch (arg->getOption().getID()) {
26592659
case OPT_warnings_as_errors:
2660-
return WarningAsErrorRule(WarningAsErrorRule::Action::Enable);
2660+
return WarningTreatmentRule(WarningTreatmentRule::Action::AsError);
26612661
case OPT_no_warnings_as_errors:
2662-
return WarningAsErrorRule(WarningAsErrorRule::Action::Disable);
2662+
return WarningTreatmentRule(WarningTreatmentRule::Action::AsWarning);
26632663
case OPT_Werror:
2664-
return WarningAsErrorRule(WarningAsErrorRule::Action::Enable,
2665-
arg->getValue());
2664+
return WarningTreatmentRule(WarningTreatmentRule::Action::AsError,
2665+
arg->getValue());
26662666
case OPT_Wwarning:
2667-
return WarningAsErrorRule(WarningAsErrorRule::Action::Disable,
2668-
arg->getValue());
2667+
return WarningTreatmentRule(WarningTreatmentRule::Action::AsWarning,
2668+
arg->getValue());
2669+
// ACTODO
2670+
case OPT_Wsuppress:
2671+
return WarningTreatmentRule(WarningTreatmentRule::Action::Suppress,
2672+
arg->getValue());
26692673
default:
26702674
llvm_unreachable("unhandled warning as error option");
26712675
}
26722676
}());
26732677
}
2678+
26742679
if (Args.hasArg(OPT_debug_diagnostic_names)) {
26752680
Opts.PrintDiagnosticNames = PrintDiagnosticNamesMode::Identifier;
26762681
}
@@ -2715,8 +2720,8 @@ static bool ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args,
27152720
}
27162721
}
27172722
assert(!(Opts.SuppressWarnings &&
2718-
WarningAsErrorRule::hasConflictsWithSuppressWarnings(
2719-
Opts.WarningsAsErrorsRules)) &&
2723+
WarningTreatmentRule::hasConflictsWithSuppressWarnings(
2724+
Opts.WarningTreatmentRules)) &&
27202725
"conflicting arguments; should have been caught by driver");
27212726

27222727
return false;
@@ -2735,7 +2740,7 @@ static void configureDiagnosticEngine(
27352740
if (Options.SuppressRemarks) {
27362741
Diagnostics.setSuppressRemarks(true);
27372742
}
2738-
Diagnostics.setWarningsAsErrorsRules(Options.WarningsAsErrorsRules);
2743+
Diagnostics.setWarningTreatmentRules(Options.WarningTreatmentRules);
27392744
Diagnostics.setPrintDiagnosticNamesMode(Options.PrintDiagnosticNames);
27402745

27412746
std::string docsPath = Options.DiagnosticDocumentationPath;

test/PerformanceHints/array-ret-override.swift

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,11 @@
66
func foo() -> [Int] {
77
return [1,2,3,4,5,6]
88
}
9+
10+
@performanceOverride(PerfHintReturnTypeImplicitCopy, "Just Testing")
11+
func bar() -> Int {
12+
let myCoolArray = { () -> [Int] in
13+
return [1, 2]
14+
}
15+
return myCoolArray().count
16+
}
Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,30 @@
11
// RUN: %empty-directory(%t)
22

3-
// RUN: %target-swift-frontend -typecheck %s -diagnostic-style llvm -enable-experimental-feature SwiftPerformanceHints &> %t/output_warn.txt
3+
// Ensure diagnosed as error by-default
4+
// RUN: not %target-swift-frontend -typecheck %s -diagnostic-style llvm -enable-experimental-feature SwiftPerformanceHints &> %t/output_err.txt
5+
// RUN: cat %t/output_err.txt | %FileCheck %s
6+
7+
// Ensure downgraded to a warning with '-Wwarning'
8+
// RUN: %target-swift-frontend -typecheck %s -diagnostic-style llvm -enable-experimental-feature SwiftPerformanceHints -Wwarning PerfHintReturnTypeImplicitCopy &> %t/output_warn.txt
49
// RUN: cat %t/output_warn.txt | %FileCheck %s -check-prefix CHECK-WARN
510

11+
// Ensure fully suppressed with '-Wsuppress'
12+
// RUN: %target-swift-frontend -typecheck %s -diagnostic-style llvm -enable-experimental-feature SwiftPerformanceHints -Wsuppress PerfHintReturnTypeImplicitCopy -verify
13+
14+
// CHECK: error: Performance: 'foo()' returns an array, leading to implicit copies. Consider using an 'inout' parameter instead. [#PerfHintReturnTypeImplicitCopy]
15+
// CHECK: error: Performance: closure returns an array, leading to implicit copies. Consider using an 'inout' parameter instead. [#PerfHintReturnTypeImplicitCopy]
16+
617
// CHECK-WARN: warning: Performance: 'foo()' returns an array, leading to implicit copies. Consider using an 'inout' parameter instead. [#PerfHintReturnTypeImplicitCopy]
18+
// CHECK-WARN: warning: Performance: closure returns an array, leading to implicit copies. Consider using an 'inout' parameter instead. [#PerfHintReturnTypeImplicitCopy]
19+
720

821
func foo() -> [Int] {
922
return [1,2,3,4,5,6]
1023
}
24+
25+
func bar() -> Int {
26+
let myCoolArray = { () -> [Int] in
27+
return [1, 2]
28+
}
29+
return myCoolArray().count
30+
}

0 commit comments

Comments
 (0)