[PATCH] D27299: [Sema] C++11 opaque enums should avoid the "case value not in enumerated type" switch warning

[Alex Lorenz via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL289055: [Sema] Avoid "case value not in enumerated type" 
warning for C++11 opaque enums (authored by arphaman).

Changed prior to commit:
  https://reviews.llvm.org/D27299?vs=79892=80757#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D27299

Files:
  cfe/trunk/lib/Sema/SemaStmt.cpp
  cfe/trunk/test/SemaCXX/switch.cpp


Index: cfe/trunk/lib/Sema/SemaStmt.cpp
===
--- cfe/trunk/lib/Sema/SemaStmt.cpp
+++ cfe/trunk/lib/Sema/SemaStmt.cpp
@@ -1070,7 +1070,8 @@
 const EnumType *ET = CondTypeBeforePromotion->getAs();
 
 // If switch has default case, then ignore it.
-if (!CaseListIsErroneous  && !HasConstantCond && ET) {
+if (!CaseListIsErroneous && !HasConstantCond && ET &&
+ET->getDecl()->isCompleteDefinition()) {
   const EnumDecl *ED = ET->getDecl();
   EnumValsTy EnumVals;
 
Index: cfe/trunk/test/SemaCXX/switch.cpp
===
--- cfe/trunk/test/SemaCXX/switch.cpp
+++ cfe/trunk/test/SemaCXX/switch.cpp
@@ -100,3 +100,33 @@
   }
   template void f(S); // expected-note {{instantiation of}}
 }
+
+// rdar://29230764
+namespace OpaqueEnumWarnings {
+
+enum Opaque : int;
+enum class OpaqueClass : int;
+
+enum class Defined : int;
+enum class Defined : int { a };
+
+void test(Opaque o, OpaqueClass oc, Defined d) {
+  // Don't warn that case value is not present in opaque enums.
+  switch (o) {
+  case (Opaque)1:
+break;
+  }
+  switch (oc) {
+  case (OpaqueClass)1:
+break;
+  }
+
+  switch (d) {
+  case Defined::a:
+break;
+  case (Defined)2: // expected-warning {{case value not in enumerated type 
'OpaqueEnumWarnings::Defined'}}
+break;
+  }
+}
+
+}


Index: cfe/trunk/lib/Sema/SemaStmt.cpp
===
--- cfe/trunk/lib/Sema/SemaStmt.cpp
+++ cfe/trunk/lib/Sema/SemaStmt.cpp
@@ -1070,7 +1070,8 @@
 const EnumType *ET = CondTypeBeforePromotion->getAs();
 
 // If switch has default case, then ignore it.
-if (!CaseListIsErroneous  && !HasConstantCond && ET) {
+if (!CaseListIsErroneous && !HasConstantCond && ET &&
+ET->getDecl()->isCompleteDefinition()) {
   const EnumDecl *ED = ET->getDecl();
   EnumValsTy EnumVals;
 
Index: cfe/trunk/test/SemaCXX/switch.cpp
===
--- cfe/trunk/test/SemaCXX/switch.cpp
+++ cfe/trunk/test/SemaCXX/switch.cpp
@@ -100,3 +100,33 @@
   }
   template void f(S); // expected-note {{instantiation of}}
 }
+
+// rdar://29230764
+namespace OpaqueEnumWarnings {
+
+enum Opaque : int;
+enum class OpaqueClass : int;
+
+enum class Defined : int;
+enum class Defined : int { a };
+
+void test(Opaque o, OpaqueClass oc, Defined d) {
+  // Don't warn that case value is not present in opaque enums.
+  switch (o) {
+  case (Opaque)1:
+break;
+  }
+  switch (oc) {
+  case (OpaqueClass)1:
+break;
+  }
+
+  switch (d) {
+  case Defined::a:
+break;
+  case (Defined)2: // expected-warning {{case value not in enumerated type 'OpaqueEnumWarnings::Defined'}}
+break;
+  }
+}
+
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27299: [Sema] C++11 opaque enums should avoid the "case value not in enumerated type" switch warning

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak accepted this revision.
ahatanak added a comment.
This revision is now accepted and ready to land.

That makes sense. Thanks for the explanation.


Repository:
  rL LLVM

https://reviews.llvm.org/D27299



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27299: [Sema] C++11 opaque enums should avoid the "case value not in enumerated type" switch warning

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: lib/Sema/SemaStmt.cpp:1165
 
   if (!hasCasesNotInSwitch)
 SS->setAllEnumCasesCovered();

ahatanak wrote:
> This function used to call setAllEnumCasesCovered() when parsing a switch 
> statement with an opaque enum condition, but this patch changes the behavior. 
> I couldn't tell whether isAllEnumCasesCovered should return true or false for 
> such switch statements, but was this change intentional?
You're right, good catch! Originally it wasn't intentional, but I've looked 
over this change and it seems like the right change to me. We can't know if we 
actually covered all enum cases when checking an opaque enum because we don't 
know which cases the enum has (unless we have a switch that covers all the of 
possible values of the enum's integer type)! It seems that the previous 
behaviour was incorrect. 


Repository:
  rL LLVM

https://reviews.llvm.org/D27299



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27299: [Sema] C++11 opaque enums should avoid the "case value not in enumerated type" switch warning

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added inline comments.



Comment at: lib/Sema/SemaStmt.cpp:1165
 
   if (!hasCasesNotInSwitch)
 SS->setAllEnumCasesCovered();

This function used to call setAllEnumCasesCovered() when parsing a switch 
statement with an opaque enum condition, but this patch changes the behavior. I 
couldn't tell whether isAllEnumCasesCovered should return true or false for 
such switch statements, but was this change intentional?


Repository:
  rL LLVM

https://reviews.llvm.org/D27299



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27299: [Sema] C++11 opaque enums should avoid the "case value not in enumerated type" switch warning

[Alex Lorenz via Phabricator via cfe-commits]

arphaman created this revision.
arphaman added reviewers: rsmith, ahatanak.
arphaman added a subscriber: cfe-commits.
arphaman set the repository for this revision to rL LLVM.

This patch ensures that the switch warning "case value not in enumerated type" 
isn't shown for opaque enums. We don't know the actual list of values in opaque 
enums, so that warning is incorrect.


Repository:
  rL LLVM

https://reviews.llvm.org/D27299

Files:
  lib/Sema/SemaStmt.cpp
  test/SemaCXX/switch.cpp


Index: test/SemaCXX/switch.cpp
===
--- test/SemaCXX/switch.cpp
+++ test/SemaCXX/switch.cpp
@@ -100,3 +100,33 @@
   }
   template void f(S); // expected-note {{instantiation of}}
 }
+
+// rdar://29230764
+namespace OpaqueEnumWarnings {
+
+enum Opaque : int;
+enum class OpaqueClass : int;
+
+enum class Defined : int;
+enum class Defined : int { a };
+
+void test(Opaque o, OpaqueClass oc, Defined d) {
+  // Don't warn that case value is not present in opaque enums.
+  switch (o) {
+  case (Opaque)1:
+break;
+  }
+  switch (oc) {
+  case (OpaqueClass)1:
+break;
+  }
+
+  switch (d) {
+  case Defined::a:
+break;
+  case (Defined)2: // expected-warning {{case value not in enumerated type 
'OpaqueEnumWarnings::Defined'}}
+break;
+  }
+}
+
+}
Index: lib/Sema/SemaStmt.cpp
===
--- lib/Sema/SemaStmt.cpp
+++ lib/Sema/SemaStmt.cpp
@@ -1070,7 +1070,8 @@
 const EnumType *ET = CondTypeBeforePromotion->getAs();
 
 // If switch has default case, then ignore it.
-if (!CaseListIsErroneous  && !HasConstantCond && ET) {
+if (!CaseListIsErroneous && !HasConstantCond && ET &&
+ET->getDecl()->isCompleteDefinition()) {
   const EnumDecl *ED = ET->getDecl();
   EnumValsTy EnumVals;
 


Index: test/SemaCXX/switch.cpp
===
--- test/SemaCXX/switch.cpp
+++ test/SemaCXX/switch.cpp
@@ -100,3 +100,33 @@
   }
   template void f(S); // expected-note {{instantiation of}}
 }
+
+// rdar://29230764
+namespace OpaqueEnumWarnings {
+
+enum Opaque : int;
+enum class OpaqueClass : int;
+
+enum class Defined : int;
+enum class Defined : int { a };
+
+void test(Opaque o, OpaqueClass oc, Defined d) {
+  // Don't warn that case value is not present in opaque enums.
+  switch (o) {
+  case (Opaque)1:
+break;
+  }
+  switch (oc) {
+  case (OpaqueClass)1:
+break;
+  }
+
+  switch (d) {
+  case Defined::a:
+break;
+  case (Defined)2: // expected-warning {{case value not in enumerated type 'OpaqueEnumWarnings::Defined'}}
+break;
+  }
+}
+
+}
Index: lib/Sema/SemaStmt.cpp
===
--- lib/Sema/SemaStmt.cpp
+++ lib/Sema/SemaStmt.cpp
@@ -1070,7 +1070,8 @@
 const EnumType *ET = CondTypeBeforePromotion->getAs();
 
 // If switch has default case, then ignore it.
-if (!CaseListIsErroneous  && !HasConstantCond && ET) {
+if (!CaseListIsErroneous && !HasConstantCond && ET &&
+ET->getDecl()->isCompleteDefinition()) {
   const EnumDecl *ED = ET->getDecl();
   EnumValsTy EnumVals;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits