Skip to content

De-underscore @frozen, apply it to structs #24185

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
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 include/swift/AST/Attr.def
Original file line number Diff line number Diff line change
Expand Up @@ -364,10 +364,11 @@ SIMPLE_DECL_ATTR(_weakLinked, WeakLinked,
OnNominalType | OnAssociatedType | OnFunc | OnAccessor | OnVar |
OnSubscript | OnConstructor | OnEnumElement | OnExtension | UserInaccessible,
75)
SIMPLE_DECL_ATTR(_frozen, Frozen,
OnEnum |
SIMPLE_DECL_ATTR(frozen, Frozen,
OnEnum | OnStruct |
UserInaccessible,
76)
DECL_ATTR_ALIAS(_frozen, Frozen)
SIMPLE_DECL_ATTR(_forbidSerializingReference, ForbidSerializingReference,
OnAnyDecl |
LongAttribute | RejectByParser | UserInaccessible | NotSerialized,
Expand Down
2 changes: 1 addition & 1 deletion include/swift/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -5064,7 +5064,7 @@ class VarDecl : public AbstractStorageDecl {
/// exposed to clients.
/// There's a very narrow case when we would: if the decl is an instance
/// member with an initializer expression and the parent type is
/// @_fixed_layout and resides in a resilient module.
/// @frozen and resides in a resilient module.
bool isInitExposedToClients() const;

/// Is this a special debugger variable?
Expand Down
19 changes: 14 additions & 5 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -1388,8 +1388,8 @@ WARNING(pattern_type_not_usable_from_inline_warn,none,
"%select{%select{variable|constant}0|property}1 "
"should be '@usableFromInline' or public",
(bool, bool))
ERROR(pattern_type_not_usable_from_inline_fixed_layout,none,
"type referenced from a stored property in a '@_fixed_layout' struct must "
ERROR(pattern_type_not_usable_from_inline_frozen,none,
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we use this diagnostic for classes too? If so it needs a %choice

Copy link
Contributor

Choose a reason for hiding this comment

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

I bet we do, but it also says "struct" in the message, so…

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah it was always struct-specific even if it applies to both. I'll happily correct it but I'd like to do that in a follow-on PR than lump it in here.

"type referenced from a stored property in a '@frozen' struct must "
"be '@usableFromInline' or public",
(/*ignored*/bool, /*ignored*/bool))
ERROR(pattern_type_not_usable_from_inline_inferred,none,
Expand All @@ -1404,9 +1404,9 @@ WARNING(pattern_type_not_usable_from_inline_inferred_warn,none,
"with inferred type %2 "
"should be '@usableFromInline' or public",
(bool, bool, Type))
ERROR(pattern_type_not_usable_from_inline_inferred_fixed_layout,none,
ERROR(pattern_type_not_usable_from_inline_inferred_frozen,none,
"type referenced from a stored property with inferred type %2 in a "
"'@_fixed_layout' struct must be '@usableFromInline' or public",
"'@frozen' struct must be '@usableFromInline' or public",
(/*ignored*/bool, /*ignored*/bool, Type))

ERROR(pattern_binds_no_variables,none,
Expand Down Expand Up @@ -4136,6 +4136,15 @@ ERROR(fixed_layout_attr_on_internal_type,
"%select{private|fileprivate|internal|%error|%error}1",
(DeclName, AccessLevel))

WARNING(fixed_layout_struct,
none, "'@frozen' attribute is now used for fixed-layout structs", ())
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: please pick either '@frozen' or 'frozen' attribute (both here and below); having both is redundant.


ERROR(frozen_attr_on_internal_type,
none, "'@frozen' attribute can only be applied to '@usableFromInline' "
"or public declarations, but %0 is "
"%select{private|fileprivate|internal|%error|%error}1",
(DeclName, AccessLevel))

ERROR(usable_from_inline_attr_with_explicit_access,
none, "'@usableFromInline' attribute can only be applied to internal "
"declarations, but %0 is %select{private|fileprivate|%error|public|open}1",
Expand All @@ -4152,7 +4161,7 @@ ERROR(usable_from_inline_attr_in_protocol,none,
"an '@inlinable' function|" \
"an '@_alwaysEmitIntoClient' function|" \
"a default argument value|" \
"a property initializer in a '@_fixed_layout' type}"
"a property initializer in a '@frozen' type}"

#define DECL_OR_ACCESSOR "%select{%0|%0 for}"

Expand Down
5 changes: 3 additions & 2 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1516,7 +1516,8 @@ bool VarDecl::isInitExposedToClients() const {
if (!parent) return false;
if (!hasInitialValue()) return false;
if (isStatic()) return false;
return parent->getAttrs().hasAttribute<FixedLayoutAttr>();
return parent->getAttrs().hasAttribute<FrozenAttr>() ||
parent->getAttrs().hasAttribute<FixedLayoutAttr>();
}

/// Check whether the given type representation will be
Expand Down Expand Up @@ -3188,7 +3189,7 @@ bool NominalTypeDecl::isFormallyResilient() const {
/*treatUsableFromInlineAsPublic=*/true).isPublic())
return false;

// Check for an explicit @_fixed_layout or @_frozen attribute.
// Check for an explicit @_fixed_layout or @frozen attribute.
if (getAttrs().hasAttribute<FixedLayoutAttr>() ||
getAttrs().hasAttribute<FrozenAttr>()) {
return false;
Expand Down
2 changes: 1 addition & 1 deletion lib/ClangImporter/ImportDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2730,7 +2730,7 @@ namespace {
errorWrapper->setAddedImplicitInitializers();
errorWrapper->setAccess(AccessLevel::Public);
errorWrapper->getAttrs().add(
new (Impl.SwiftContext) FixedLayoutAttr(/*IsImplicit*/true));
new (Impl.SwiftContext) FrozenAttr(/*IsImplicit*/true));

StringRef nameForMangling;
ClangImporterSynthesizedTypeAttr::Kind relatedEntityKind;
Expand Down
2 changes: 1 addition & 1 deletion lib/IRGen/StructLayout.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ StructLayout::StructLayout(IRGenModule &IGM,

assert(typeToFill == nullptr || Ty == typeToFill);

// If the struct is not @_fixed_layout, it will have a dynamic
// If the struct is not @frozen, it will have a dynamic
// layout outside of its resilience domain.
if (decl) {
if (IGM.isResilient(decl, ResilienceExpansion::Minimal))
Expand Down
13 changes: 7 additions & 6 deletions lib/SIL/SILDeclRef.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -317,13 +317,14 @@ SILLinkage SILDeclRef::getLinkage(ForDefinition_t forDefinition) const {
if (isStoredPropertyInitializer()) {
// Three cases:
//
// 1) Type is formally @_fixed_layout. Root initializers can be declared
// @inlinable. The property initializer must only reference
// 1) Type is formally @_fixed_layout/@frozen. Root initializers can be
// declared @inlinable. The property initializer must only reference
// public symbols, and is serialized, so we give it PublicNonABI linkage.
//
// 2) Type is not formally @_fixed_layout and the module is not resilient.
// Root initializers can be declared @inlinable. This is the annoying
// case. We give the initializer public linkage if the type is public.
// 2) Type is not formally @_fixed_layout/@frozen and the module is not
// resilient. Root initializers can be declared @inlinable. This is the
// annoying case. We give the initializer public linkage if the type is
// public.
//
// 3) Type is resilient. The property initializer is never public because
// root initializers cannot be @inlinable.
Expand Down Expand Up @@ -491,7 +492,7 @@ IsSerialized_t SILDeclRef::isSerialized() const {
}

// Stored property initializers are inlinable if the type is explicitly
// marked as @_fixed_layout.
// marked as @frozen.
if (isStoredPropertyInitializer()) {
auto *nominal = cast<NominalTypeDecl>(d->getDeclContext());
auto scope =
Expand Down
7 changes: 4 additions & 3 deletions lib/Sema/TypeCheckAccess.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1059,7 +1059,8 @@ class UsableFromInlineChecker : public AccessControlCheckerBase,
auto *parentStruct = dyn_cast<StructDecl>(PBD->getDeclContext());
if (!parentStruct)
return nullptr;
if (!parentStruct->getAttrs().hasAttribute<FixedLayoutAttr>() ||
if (!(parentStruct->getAttrs().hasAttribute<FrozenAttr>() ||
parentStruct->getAttrs().hasAttribute<FixedLayoutAttr>()) ||
PBD->isStatic() || !PBD->hasStorage()) {
return nullptr;
}
Expand Down Expand Up @@ -1091,7 +1092,7 @@ class UsableFromInlineChecker : public AccessControlCheckerBase,
auto diagID = diag::pattern_type_not_usable_from_inline_inferred;
if (fixedLayoutStructContext) {
diagID =
diag::pattern_type_not_usable_from_inline_inferred_fixed_layout;
diag::pattern_type_not_usable_from_inline_inferred_frozen;
} else if (!TC.Context.isSwiftVersionAtLeast(5)) {
diagID = diag::pattern_type_not_usable_from_inline_inferred_warn;
}
Expand Down Expand Up @@ -1127,7 +1128,7 @@ class UsableFromInlineChecker : public AccessControlCheckerBase,
DowngradeToWarning downgradeToWarning) {
auto diagID = diag::pattern_type_not_usable_from_inline;
if (fixedLayoutStructContext)
diagID = diag::pattern_type_not_usable_from_inline_fixed_layout;
diagID = diag::pattern_type_not_usable_from_inline_frozen;
else if (!TC.Context.isSwiftVersionAtLeast(5))
diagID = diag::pattern_type_not_usable_from_inline_warn;
auto diag = TC.diagnose(TP->getLoc(), diagID, anyVar->isLet(),
Expand Down
28 changes: 21 additions & 7 deletions lib/Sema/TypeCheckAttr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1957,6 +1957,11 @@ void AttributeChecker::visitSpecializeAttr(SpecializeAttr *attr) {
}

void AttributeChecker::visitFixedLayoutAttr(FixedLayoutAttr *attr) {
if (isa<StructDecl>(D)) {
TC.diagnose(attr->getLocation(), diag::fixed_layout_struct)
.fixItReplace(attr->getRange(), "@frozen");
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this needs to be getRangeWithAt (or that you should omit the @ from your replacement).

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I'm missing something, this looks right as-is to me:

'@frozen' attribute is now used for fixed-layout structs
@_fixed_layout
^~~~~~~~~~~~~~
@frozen

Copy link
Contributor

@jrose-apple jrose-apple May 28, 2019

Choose a reason for hiding this comment

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

Are you sure you weren't seeing

'@frozen' attribute is now used for fixed-layout structs
@_fixed_layout
^~~~~~~~~~~~~~
 @frozen

? Which is a different thing. You can test this: expected-warning {{'@frozen' attribute is now used for fixed-layout structs}} {{1-13=@frozen}}

Copy link
Member Author

Choose a reason for hiding this comment

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

A @_fixed_layout public struct FixedDeprecationWarning {} // expected-warning {{'@frozen' attribute is now used for fixed-layout structs}} {{1-15=@frozen}} test passes.

}

auto *VD = cast<ValueDecl>(D);

if (VD->getFormalAccess() < AccessLevel::Public &&
Expand Down Expand Up @@ -2462,16 +2467,25 @@ void AttributeChecker::visitImplementsAttr(ImplementsAttr *attr) {
}

void AttributeChecker::visitFrozenAttr(FrozenAttr *attr) {
auto *ED = cast<EnumDecl>(D);
if (auto *ED = dyn_cast<EnumDecl>(D)) {
if (!ED->getModuleContext()->isResilient()) {
diagnoseAndRemoveAttr(attr, diag::enum_frozen_nonresilient, attr);
Copy link
Contributor

Choose a reason for hiding this comment

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

This check probably applies to structs as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

Still not resolved.

Copy link
Member Author

Choose a reason for hiding this comment

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

Similar thing, this is the existing behavior, so I'd suggest landing and then a follow-up PR can make things more consistent across the two.

return;
}

if (!ED->getModuleContext()->isResilient()) {
diagnoseAndRemoveAttr(attr, diag::enum_frozen_nonresilient, attr);
return;
if (ED->getFormalAccess() < AccessLevel::Public &&
!ED->getAttrs().hasAttribute<UsableFromInlineAttr>()) {
diagnoseAndRemoveAttr(attr, diag::enum_frozen_nonpublic, attr);
return;
}
}

if (ED->getFormalAccess() < AccessLevel::Public &&
!ED->getAttrs().hasAttribute<UsableFromInlineAttr>()) {
diagnoseAndRemoveAttr(attr, diag::enum_frozen_nonpublic, attr);
auto *VD = cast<ValueDecl>(D);

if (VD->getFormalAccess() < AccessLevel::Public &&
!VD->getAttrs().hasAttribute<UsableFromInlineAttr>()) {
diagnoseAndRemoveAttr(attr, diag::frozen_attr_on_internal_type,
VD->getFullName(), VD->getFormalAccess());
}
}

Expand Down
2 changes: 1 addition & 1 deletion stdlib/private/StdlibUnittest/StdlibCoreExtras.swift
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ public func _isStdlibDebugConfiguration() -> Bool {
#endif
}

@_fixed_layout
@frozen
public struct LinearCongruentialGenerator: RandomNumberGenerator {

@usableFromInline
Expand Down
2 changes: 1 addition & 1 deletion stdlib/public/Darwin/ARKit/ARKit.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ extension ARCamera {
/**
A value describing the camera's tracking state.
*/
@_frozen
@frozen
public enum TrackingState {
public enum Reason {
/** Tracking is limited due to initialization in progress. */
Expand Down
2 changes: 1 addition & 1 deletion stdlib/public/Darwin/CoreGraphics/CGFloat.swift.gyb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ word_bits = int(CMAKE_SIZEOF_VOID_P) * 8
@_exported import CoreGraphics
import Darwin

@_fixed_layout
@frozen
public struct CGFloat {
#if arch(i386) || arch(arm)
/// The native type used to store the CGFloat, which is Float on
Expand Down
2 changes: 1 addition & 1 deletion stdlib/public/Darwin/Dispatch/Dispatch.swift
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ public struct DispatchQoS : Equatable {
}

///
@_frozen
@frozen
public enum DispatchTimeoutResult {
case success
case timedOut
Expand Down
10 changes: 5 additions & 5 deletions stdlib/public/Darwin/Foundation/Data.swift
Original file line number Diff line number Diff line change
Expand Up @@ -602,7 +602,7 @@ internal class __NSSwiftData : NSData {
#endif
}

@_fixed_layout
@frozen
public struct Data : ReferenceConvertible, Equatable, Hashable, RandomAccessCollection, MutableCollection, RangeReplaceableCollection, MutableDataProtocol, ContiguousBytes {
public typealias ReferenceType = NSData

Expand All @@ -618,7 +618,7 @@ public struct Data : ReferenceConvertible, Equatable, Hashable, RandomAccessColl
// A small inline buffer of bytes suitable for stack-allocation of small data.
// Inlinability strategy: everything here should be inlined for direct operation on the stack wherever possible.
@usableFromInline
@_fixed_layout
@frozen
internal struct InlineData {
#if arch(x86_64) || arch(arm64) || arch(s390x) || arch(powerpc64) || arch(powerpc64le)
@usableFromInline typealias Buffer = (UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8,
Expand Down Expand Up @@ -839,7 +839,7 @@ public struct Data : ReferenceConvertible, Equatable, Hashable, RandomAccessColl
// A buffer of bytes too large to fit in an InlineData, but still small enough to fit a storage pointer + range in two words.
// Inlinability strategy: everything here should be easily inlinable as large _DataStorage methods should not inline into here.
@usableFromInline
@_fixed_layout
@frozen
internal struct InlineSlice {
// ***WARNING***
// These ivars are specifically laid out so that they cause the enum _Representation to be 16 bytes on 64 bit platforms. This means we _MUST_ have the class type thing last
Expand Down Expand Up @@ -1085,7 +1085,7 @@ public struct Data : ReferenceConvertible, Equatable, Hashable, RandomAccessColl
// A buffer of bytes whose range is too large to fit in a signle word. Used alongside a RangeReference to make it fit into _Representation's two-word size.
// Inlinability strategy: everything here should be easily inlinable as large _DataStorage methods should not inline into here.
@usableFromInline
@_fixed_layout
@frozen
internal struct LargeSlice {
// ***WARNING***
// These ivars are specifically laid out so that they cause the enum _Representation to be 16 bytes on 64 bit platforms. This means we _MUST_ have the class type thing last
Expand Down Expand Up @@ -1261,7 +1261,7 @@ public struct Data : ReferenceConvertible, Equatable, Hashable, RandomAccessColl
// The actual storage for Data's various representations.
// Inlinability strategy: almost everything should be inlinable as forwarding the underlying implementations. (Inlining can also help avoid retain-release traffic around pulling values out of enums.)
@usableFromInline
@_frozen
@frozen
internal enum _Representation {
case empty
case inline(InlineData)
Expand Down
6 changes: 3 additions & 3 deletions stdlib/public/Darwin/ObjectiveC/ObjectiveC.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import _SwiftObjectiveCOverlayShims
/// On 64-bit iOS, the Objective-C BOOL type is a typedef of C/C++
/// bool. Elsewhere, it is "signed char". The Clang importer imports it as
/// ObjCBool.
@_fixed_layout
@frozen
public struct ObjCBool : ExpressibleByBooleanLiteral {
#if os(macOS) || (os(iOS) && (arch(i386) || arch(arm)))
// On OS X and 32-bit iOS, Objective-C's BOOL type is a "signed char".
Expand Down Expand Up @@ -101,7 +101,7 @@ func _convertObjCBoolToBool(_ x: ObjCBool) -> Bool {
/// convert between C strings and selectors.
///
/// The compiler has special knowledge of this type.
@_fixed_layout
@frozen
public struct Selector : ExpressibleByStringLiteral {
var ptr: OpaquePointer

Expand Down Expand Up @@ -150,7 +150,7 @@ extension Selector : CustomReflectable {
// NSZone
//===----------------------------------------------------------------------===//

@_fixed_layout
@frozen
public struct NSZone {
var pointer: OpaquePointer
}
Expand Down
2 changes: 1 addition & 1 deletion stdlib/public/Platform/Platform.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public var noErr: OSStatus { return 0 }
/// Foundation.
///
/// The C type is a typedef for `unsigned char`.
@_fixed_layout
@frozen
public struct DarwinBoolean : ExpressibleByBooleanLiteral {
@usableFromInline var _value: UInt8

Expand Down
4 changes: 2 additions & 2 deletions stdlib/public/core/ASCII.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
//
//===----------------------------------------------------------------------===//
extension Unicode {
@_frozen
@frozen
public enum ASCII {}
}

Expand Down Expand Up @@ -68,7 +68,7 @@ extension Unicode.ASCII : Unicode.Encoding {
return encode(FromEncoding.decode(content))
}

@_fixed_layout
@frozen
public struct Parser {
@inlinable
public init() { }
Expand Down
4 changes: 2 additions & 2 deletions stdlib/public/core/Algorithm.swift
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ public func max<T : Comparable>(_ x: T, _ y: T, _ z: T, _ rest: T...) -> T {
/// }
/// // Prints "0: foo"
/// // Prints "1: bar"
@_fixed_layout
@frozen
public struct EnumeratedSequence<Base: Sequence> {
@usableFromInline
internal var _base: Base
Expand All @@ -118,7 +118,7 @@ extension EnumeratedSequence {
///
/// To create an instance, call
/// `enumerated().makeIterator()` on a sequence or collection.
@_fixed_layout
@frozen
public struct Iterator {
@usableFromInline
internal var _base: Base.Iterator
Expand Down
2 changes: 1 addition & 1 deletion stdlib/public/core/AnyHashable.swift
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ internal struct _ConcreteHashableBox<Base : Hashable> : _AnyHashableBox {
/// print(descriptions[AnyHashable(43)]) // prints "nil"
/// print(descriptions[AnyHashable(Int8(43))]!) // prints "an Int8"
/// print(descriptions[AnyHashable(Set(["a", "b"]))]!) // prints "a set of strings"
@_fixed_layout
@frozen
public struct AnyHashable {
internal var _box: _AnyHashableBox

Expand Down
2 changes: 1 addition & 1 deletion stdlib/public/core/Array.swift
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@
/// - Note: The `ContiguousArray` and `ArraySlice` types are not bridged;
/// instances of those types always have a contiguous block of memory as
/// their storage.
@_fixed_layout
@frozen
public struct Array<Element>: _DestructorSafeContainer {
#if _runtime(_ObjC)
@usableFromInline
Expand Down
2 changes: 1 addition & 1 deletion stdlib/public/core/ArrayBody.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

import SwiftShims

@_fixed_layout
@frozen
@usableFromInline
internal struct _ArrayBody {
@usableFromInline
Expand Down
2 changes: 1 addition & 1 deletion stdlib/public/core/ArrayBuffer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ internal typealias _ArrayBridgeStorage
= _BridgeStorage<__ContiguousArrayStorageBase>

@usableFromInline
@_fixed_layout
@frozen
internal struct _ArrayBuffer<Element> : _ArrayBufferProtocol {

/// Create an empty buffer.
Expand Down
Loading