[Clang][Sema] Fix Wswitch-default bad warning in template#76007
[Clang][Sema] Fix Wswitch-default bad warning in template#76007
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
aaronpuchert
left a comment
There was a problem hiding this comment.
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. |
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