-
Notifications
You must be signed in to change notification settings - Fork 13.7k
[Clang][Sema] Fix Wswitch-default bad warning in template #76007
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
Conversation
@llvm/pr-subscribers-clang Author: None (hstk30-hw) ChangesFix #75943 Full diff: https://github.com/llvm/llvm-project/pull/76007.diff 2 Files Affected:
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 63348d27a8c94a..adc2055ec4e659 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -1327,9 +1327,6 @@ Sema::ActOnFinishSwitchStmt(SourceLocation SwitchLoc, Stmt *Switch,
}
}
- if (!TheDefaultStmt)
- Diag(SwitchLoc, diag::warn_switch_default);
-
if (!HasDependentValue) {
// If we don't have a default statement, check whether the
// condition is constant.
@@ -1344,6 +1341,7 @@ Sema::ActOnFinishSwitchStmt(SourceLocation SwitchLoc, Stmt *Switch,
assert(!HasConstantCond ||
(ConstantCondValue.getBitWidth() == CondWidth &&
ConstantCondValue.isSigned() == CondIsSigned));
+ Diag(SwitchLoc, diag::warn_switch_default);
}
bool ShouldCheckConstantCond = HasConstantCond;
diff --git a/clang/test/Sema/switch-default-template.cpp b/clang/test/Sema/switch-default-template.cpp
new file mode 100644
index 00000000000000..c671164bd785b0
--- /dev/null
+++ b/clang/test/Sema/switch-default-template.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wswitch-default %s
+
+template<typename Index>
+int f1(Index i)
+{
+ switch (i) { // expected-warning {{'switch' missing 'default' label}}
+ case 0: return 0;
+ case 1: return 1;
+ }
+ return 0;
+}
+
+template<typename Index>
+int f2(Index i)
+{
+ switch (i) { // no-warning
+ case 0: return 0;
+ case 1: return 1;
+ default: return 2;
+ }
+ return 0;
+}
+
+int main() {
+ return f1(1); // expected-note {{in instantiation of function template specialization 'f1<int>' requested here}}
+}
+
|
Can you add more details to you summary e.g. "#73077 added -Wswitch-default diagnostic but it produced false positives in templates. This PR will address that issue" |
c3d5ac4
to
7b8c1c7
Compare
thanks for correcting this. |
7b8c1c7
to
2991e9b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now I guess this is Ok, although I think the better fix would be to diagnose missing or duplicate default
labels even in the dependent case. Because default
labels themselves are never dependent.
We should probably add a FIXME for this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks
2991e9b
to
d6c5cfe
Compare
✅ With the latest revision this PR passed the C/C++ code formatter. |
[llvm#73077] added Wswitch-default diagnostic but it produced false positives in templates. This PR address it.
d6c5cfe
to
8e00f93
Compare
This test case exists upstream currently. It was deleted by mistake when reapplying the patch: [clang][Sema] Add -Wswitch-default warning option (llvm#73077) The test case had been added upstream for a subsequent patch [Clang][Sema] Fix Wswitch-default bad warning in template (llvm#76007) Change-Id: I94fdbe2daa4703166e0d7fc00a3a4d08f98ae410
cherry-picked: 9e1f1cf sdkrystian@gmail.com Tue Jul 9 16:40:53 2024 -0400 [Clang][Sema] Handle class member access expressions with valid nested-name-specifiers that become invalid after lookup (llvm#98167) 584e431 sdkrystian@gmail.com Wed Jul 3 12:12:53 2024 -0400 [Clang][Sema] Treat explicit specializations of static data member templates declared without 'static' as static data members when diagnosing uses of 'auto' (llvm#97425) a1bce0b dblaikie@gmail.com Thu Jun 27 08:17:40 2024 -0700 Clang: Add warning flag for storage class specifiers on explicit specializations (llvm#96699) f46d146 erickvelez7@gmail.com Fri May 31 11:02:21 2024 -0700 [clang] require template arg list after template kw (llvm#80801) 033ec09 hanwei62@huawei.com Fri Dec 22 09:00:41 2023 +0800 [Clang][Sema] Fix Wswitch-default bad warning in template (llvm#76007) c281782 dongjianqiang2@huawei.com Thu Dec 7 09:03:15 2023 +0800 [clang][Sema] Add -Wswitch-default warning option (llvm#73077) Change-Id: Ib2582201b01cc62c3bf62011347925556e8531f7
#73077 added -Wswitch-default diagnostic but it produced false positives in templates. This PR will address that. #75943