Print attributes on enum cases correctly (like 'indirect') (swiftlang…

…#26311) Previously they were just skipped if enum elements weren't exploded into their own individual lines, since the ASTPrinter assumed they'd be present on the EnumCaseDecl. This led to miscompiles when using a module interface for an enum with indirect cases, since they'd be printed as non-indirect cases. rdar://problem/53329452
bartchr808 · Jul 24, 2019 · 3596df7 · 3596df7
1 parent f26048d
commit 3596df7
Show file tree

Hide file tree

Showing 4 changed files with 90 additions and 3 deletions.
diff --git a/lib/AST/ASTPrinter.cpp b/lib/AST/ASTPrinter.cpp
@@ -951,7 +951,8 @@ void PrintAST::printAttributes(const Decl *D) {
 
     // If the declaration is implicitly @objc, print the attribute now.
     if (auto VD = dyn_cast<ValueDecl>(D)) {
-      if (VD->isObjC() && !VD->getAttrs().hasAttribute<ObjCAttr>()) {
+      if (VD->isObjC() && !isa<EnumElementDecl>(VD) &&
+          !VD->getAttrs().hasAttribute<ObjCAttr>()) {
         Printer.printAttrName("@objc");
         Printer << " ";
       }
@@ -2927,8 +2928,8 @@ void PrintAST::visitEnumCaseDecl(EnumCaseDecl *decl) {
   if (!elems.empty()) {
     // Documentation comments over the case are attached to the enum elements.
     printDocumentationComment(elems[0]);
+    printAttributes(elems[0]);
   }
-  printAttributes(decl);
   Printer << tok::kw_case << " ";
 
   interleave(elems.begin(), elems.end(),

diff --git a/lib/AST/ASTVerifier.cpp b/lib/AST/ASTVerifier.cpp
@@ -2865,6 +2865,16 @@ class Verifier : public ASTWalker {
       verifyParsedBase(UED);
     }
 
+    void verifyParsed(EnumCaseDecl *D) {
+      PrettyStackTraceDecl debugStack("verifying EnumCaseDecl", D);
+      if (!D->getAttrs().isEmpty()) {
+        Out << "EnumCaseDecl should not have attributes";
+        abort();
+      }
+
+      verifyParsedBase(D);
+    }
+
     void verifyParsed(AbstractFunctionDecl *AFD) {
       PrettyStackTraceDecl debugStack("verifying AbstractFunctionDecl", AFD);
 

diff --git a/test/ParseableInterface/Inputs/enums-layout-helper.swift b/test/ParseableInterface/Inputs/enums-layout-helper.swift
@@ -37,3 +37,43 @@ public enum FutureproofEnum: Int {
   // CHECK-NEXT: case c = 100{{$}}
   case c = 100
 }
+
+// CHECK-LABEL: indirect public enum FutureproofIndirectEnum
+public indirect enum FutureproofIndirectEnum {
+  // CHECK-NEXT: case a{{$}}
+  case a
+  // CHECK-NEXT: case b(Swift.Int){{$}}
+  case b(Int)
+  // CHECK-NEXT: case c{{$}}
+  case c
+}
+
+// CHECK-LABEL: indirect public enum FrozenIndirectEnum
+@_frozen public indirect enum FrozenIndirectEnum {
+  // CHECK-NEXT: case a{{$}}
+  case a
+  // CHECK-NEXT: case b(Swift.Int){{$}}
+  case b(Int)
+  // CHECK-NEXT: case c{{$}}
+  case c
+}
+
+// CHECK-LABEL: public enum FutureproofIndirectCaseEnum
+public enum FutureproofIndirectCaseEnum {
+  // CHECK-NEXT: {{^}} case a{{$}}
+  case a
+  // CHECK-NEXT: indirect case b(Swift.Int){{$}}
+  indirect case b(Int)
+  // CHECK-NEXT: {{^}} case c{{$}}
+  case c
+}
+
+// CHECK-LABEL: public enum FrozenIndirectCaseEnum
+@_frozen public enum FrozenIndirectCaseEnum {
+  // CHECK-NEXT: {{^}} case a{{$}}
+  case a
+  // CHECK-NEXT: indirect case b(Swift.Int){{$}}
+  indirect case b(Int)
+  // CHECK-NEXT: {{^}} case c{{$}}
+  case c
+}
diff --git a/test/ParseableInterface/enums-layout.swift b/test/ParseableInterface/enums-layout.swift
@@ -1,7 +1,7 @@
 // RUN: %empty-directory(%t)
 // RUN: %target-swift-frontend -emit-module-interface-path %t/Lib.swiftinterface -typecheck -enable-library-evolution -enable-objc-interop -disable-objc-attr-requires-foundation-module -swift-version 5 %S/Inputs/enums-layout-helper.swift -module-name Lib
 // RUN: %FileCheck %S/Inputs/enums-layout-helper.swift < %t/Lib.swiftinterface
-// RUN: %target-swift-frontend -enable-objc-interop -O -emit-ir -primary-file %s -I %t | %FileCheck %s
+// RUN: %target-swift-frontend -enable-objc-interop -O -emit-ir -primary-file %s -I %t -Xllvm -swiftmergefunc-threshold=0 | %FileCheck %s
 
 import Lib
 
@@ -34,3 +34,39 @@ func testFrozenObjCEnum() -> FrozenObjCEnum {
   // CHECK: ret i{{32|64}} 10
   return .b
 } // CHECK-NEXT: {{^}$}}
+
+// CHECK-LABEL: define{{.+}}testFutureproofIndirectEnum
+func testFutureproofIndirectEnum() -> FutureproofIndirectEnum {
+  // CHECK: [[CASE:%.+]] = load i32, i32* @"$s3Lib23FutureproofIndirectEnumO1cyA2CmFWC"
+  // CHECK: [[METADATA_RESPONSE:%.+]] = tail call swiftcc %swift.metadata_response @"$s3Lib23FutureproofIndirectEnumOMa"
+  // CHECK: [[METADATA:%.+]] = extractvalue %swift.metadata_response [[METADATA_RESPONSE]], 0
+  // CHECK: call void {{%.+}}(%swift.opaque* noalias %0, i32 [[CASE]], %swift.type* [[METADATA]])
+  // CHECK-NEXT: ret void
+  return .c
+}
+
+// CHECK-LABEL: define{{.+}}testFrozenIndirectEnum
+func testFrozenIndirectEnum() -> FrozenIndirectEnum {
+  // Whether this is "1" or "2" depends on whether the reserved ObjC tagged
+  // pointer bit is the top or bottom bit on this platform.
+  // CHECK: ret i{{32|64}} {{1|2}}
+  return .c
+}
+
+// CHECK-LABEL: define{{.+}}testFutureproofIndirectCaseEnum
+func testFutureproofIndirectCaseEnum() -> FutureproofIndirectCaseEnum {
+  // CHECK: [[CASE:%.+]] = load i32, i32* @"$s3Lib27FutureproofIndirectCaseEnumO1cyA2CmFWC"
+  // CHECK: [[METADATA_RESPONSE:%.+]] = tail call swiftcc %swift.metadata_response @"$s3Lib27FutureproofIndirectCaseEnumOMa"
+  // CHECK: [[METADATA:%.+]] = extractvalue %swift.metadata_response [[METADATA_RESPONSE]], 0
+  // CHECK: call void {{%.+}}(%swift.opaque* noalias %0, i32 [[CASE]], %swift.type* [[METADATA]])
+  // CHECK-NEXT: ret void
+  return .c
+}
+
+// CHECK-LABEL: define{{.+}}testFrozenIndirectCaseEnum
+func testFrozenIndirectCaseEnum() -> FrozenIndirectCaseEnum {
+  // Whether this is "1" or "2" depends on whether the reserved ObjC tagged
+  // pointer bit is the top or bottom bit on this platform.
+  // CHECK: ret i{{32|64}} {{1|2}}
+  return .c
+}