https://github.com/rnk approved this pull request.
Thanks!
https://github.com/llvm/llvm-project/pull/138562
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/zmodem updated
https://github.com/llvm/llvm-project/pull/138562
>From e221ba3b0f7b08bcfc56bf75f7505265c332637d Mon Sep 17 00:00:00 2001
From: Hans Wennborg
Date: Mon, 5 May 2025 20:24:15 +0200
Subject: [PATCH 1/9] [Sema] Warn about omitting deprecated enumerator in
switch
T
https://github.com/AaronBallman approved this pull request.
Please add a release note to `clang/docs/ReleaseNotes.rst` so users know about
the new functionality and flag.
Otherwise, LGTM!
https://github.com/llvm/llvm-project/pull/138562
___
cfe-commi
AaronBallman wrote:
> Thanks Aaron, that's a good example.
>
> This is a pickle; it doesn't seem like there's an obviously Right
> Solution(tm) here.
That's the same conclusion I'm coming to. These situations are kind of mutually
exclusive.
> I think we're agreeing on the first part, that un
https://github.com/zmodem updated
https://github.com/llvm/llvm-project/pull/138562
>From e221ba3b0f7b08bcfc56bf75f7505265c332637d Mon Sep 17 00:00:00 2001
From: Hans Wennborg
Date: Mon, 5 May 2025 20:24:15 +0200
Subject: [PATCH 1/8] [Sema] Warn about omitting deprecated enumerator in
switch
T
@@ -6009,6 +6009,8 @@ def note_not_found_by_two_phase_lookup : Note<"%0 should
be declared prior to th
def err_undeclared_use : Error<"use of undeclared %0">;
def warn_deprecated : Warning<"%0 is deprecated">,
InGroup;
+def warn_deprecated_switch_case : Warning<"%0 is depr
https://github.com/zmodem updated
https://github.com/llvm/llvm-project/pull/138562
>From e221ba3b0f7b08bcfc56bf75f7505265c332637d Mon Sep 17 00:00:00 2001
From: Hans Wennborg
Date: Mon, 5 May 2025 20:24:15 +0200
Subject: [PATCH 1/7] [Sema] Warn about omitting deprecated enumerator in
switch
T
@@ -6009,6 +6009,8 @@ def note_not_found_by_two_phase_lookup : Note<"%0 should
be declared prior to th
def err_undeclared_use : Error<"use of undeclared %0">;
def warn_deprecated : Warning<"%0 is deprecated">,
InGroup;
+def warn_deprecated_switch_case : Warning<"%0 is depr
zmodem wrote:
Thanks Aaron, that's a good example.
This is a pickle; it doesn't seem like there's an obviously Right Solution(tm)
here.
I think we're agreeing on the first part, that unhandled deprecated enums
should trigger the "not covered" warning.
Some users will prefer to handle that wi
rnk wrote:
> I think silencing the warning is better than suggesting a default case, which
> may not be considered good practice.
I'm not sure, I think our perspectives as compiler people might be a bit off
base. We're always forming closed algebraic sum types, like variants, AST
nodes, that
zmodem wrote:
> Now we have a special Wdeprecated-declarations carveouts for switches, but if
> you unpack the switch into if / else chain comparisons, you get deprecation
> warnings.
Agreed, that it is a bit inconsistent. There is prior art though:
ba87c626f9b7c48a79673d3fd682c2a1e80a7200 ma
@@ -6767,6 +6767,9 @@ class Sema final : public SemaBase {
};
std::optional DelayedDefaultInitializationContext;
+/// Whether evaluating an expression for a switch case label.
zmodem wrote:
Done.
https://github.com/llvm/llvm-project/pull/138562
_
https://github.com/zmodem updated
https://github.com/llvm/llvm-project/pull/138562
>From e221ba3b0f7b08bcfc56bf75f7505265c332637d Mon Sep 17 00:00:00 2001
From: Hans Wennborg
Date: Mon, 5 May 2025 20:24:15 +0200
Subject: [PATCH 1/6] [Sema] Warn about omitting deprecated enumerator in
switch
T
https://github.com/rnk edited https://github.com/llvm/llvm-project/pull/138562
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/rnk commented:
I'm OK with this, but I feel like this is creating scope creep. Now we have a
special Wdeprecated-declarations carveouts for switches, but if you unpack the
switch into if / else chain comparisons, you get deprecation warnings. Should
we disable deprecation wa
@@ -6767,6 +6767,9 @@ class Sema final : public SemaBase {
};
std::optional DelayedDefaultInitializationContext;
+/// Whether evaluating an expression for a switch case label.
rnk wrote:
supernit: pack it with the contextual bools above, for both
zmodem wrote:
I've added suppression of deprecated values in case expressions. Please take
another look!
https://github.com/llvm/llvm-project/pull/138562
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/li
https://github.com/zmodem updated
https://github.com/llvm/llvm-project/pull/138562
Rate limit ยท GitHub
body {
background-color: #f6f8fa;
color: #24292e;
font-family: -apple-system,BlinkMacSystemFont,Segoe
UI,Helvetica,Arial,sans-se
https://github.com/zmodem updated
https://github.com/llvm/llvm-project/pull/138562
>From e221ba3b0f7b08bcfc56bf75f7505265c332637d Mon Sep 17 00:00:00 2001
From: Hans Wennborg
Date: Mon, 5 May 2025 20:24:15 +0200
Subject: [PATCH 1/4] [Sema] Warn about omitting deprecated enumerator in
switch
T
cor3ntin wrote:
> Maybe we should suppress that warning for switch cases? Or suggest adding a
> default label instead?
I think silencing the warning is better than suggesting a default case, which
may not be considered good practice.
Maybe we can add a -Wdeprecated-switch-case warning that peo
zmodem wrote:
I wonder if we should also do something special about the
`-Wdeprecated-declarations` diagnostic in switches.
If the user fixes the -Wswitch / -Wreturn-type warnings from `testSwitchFour`
the natural way:
```
int testSwitchFour(enum SwitchFour e) {
switch (e) {
case Red:
zmodem wrote:
> Fly-by comment: I drafted the following alternative comment in another
> thread. Please feel free to disregard as I don't know the tone and style of
> comments in these files. (But I appreciate your consideration for the
> contents in the message I'd like to see!)
>
> ```
>
@@ -15,7 +15,7 @@ enum SwitchTwo {
};
void testSwitchTwo(enum SwitchTwo st) {
- switch (st) {} // expected-warning{{enumeration values 'Vim' and 'Emacs' not
handled in switch}}
+ switch (st) {} // expected-warning{{enumeration values 'Ed', 'Vim' and
'Emacs' not handled in
https://github.com/zmodem updated
https://github.com/llvm/llvm-project/pull/138562
>From e221ba3b0f7b08bcfc56bf75f7505265c332637d Mon Sep 17 00:00:00 2001
From: Hans Wennborg
Date: Mon, 5 May 2025 20:24:15 +0200
Subject: [PATCH 1/2] [Sema] Warn about omitting deprecated enumerator in
switch
T
ReticulateSplines wrote:
Fly-by comment: I drafted the following alternative comment in another thread.
Please feel free to disregard as I don't know the tone and style of comments in
these files. (But I appreciate your consideration for the contents in the
message I'd like to see!)
https://github.com/rnk commented:
Thanks! I think this is sufficiently niche that we don't need a flag flip to
manage the diagnostic change fallout.
https://github.com/llvm/llvm-project/pull/138562
___
cfe-commits mailing list
cfe-commits@lists.llvm.o
@@ -15,7 +15,7 @@ enum SwitchTwo {
};
void testSwitchTwo(enum SwitchTwo st) {
- switch (st) {} // expected-warning{{enumeration values 'Vim' and 'Emacs' not
handled in switch}}
+ switch (st) {} // expected-warning{{enumeration values 'Ed', 'Vim' and
'Emacs' not handled in
https://github.com/rnk edited https://github.com/llvm/llvm-project/pull/138562
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Hans Wennborg (zmodem)
Changes
This undoes part of 3e4e3b17c14c15c23c0ed18ca9165b42b1b13ae3 which added the
"Omitting a deprecated constant is ok; it should never materialize." logic.
That seems wrong: deprecated means the enumerator is l
https://github.com/zmodem created
https://github.com/llvm/llvm-project/pull/138562
This undoes part of 3e4e3b17c14c15c23c0ed18ca9165b42b1b13ae3 which added the
"Omitting a deprecated constant is ok; it should never materialize." logic.
That seems wrong: deprecated means the enumerator is likel
30 matches
Mail list logo
