Skip to content

Print attributes on enum cases correctly (like 'indirect') #26311

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

Merged
merged 1 commit into from
Jul 24, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions lib/AST/ASTPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 << " ";
}
Expand Down Expand Up @@ -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(),
Expand Down
10 changes: 10 additions & 0 deletions lib/AST/ASTVerifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
40 changes: 40 additions & 0 deletions test/ParseableInterface/Inputs/enums-layout-helper.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think it's worth to add test cases for multi element case decl? e.g.

enum Something {
  indirect case a(Int), b(Int)
  case c
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mm, good point. This is a time-sensitive fix so I'll do it in a follow-up.

38 changes: 37 additions & 1 deletion test/ParseableInterface/enums-layout.swift
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -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
}