swift-module-digester: diagnose reordering and adding decls in fixed layout types. #19373
Conversation
…re comparing them. This allows us to add comments to the error messages without breaking the tests.
|
@swift-ci please smoke test |
nkcsgexi
force-pushed
the
diagnose-reorder
branch
from
293190e to
1a234e5
Compare
|
@swift-ci please smoke test |
| /* Fixed-layout Type changes */ | ||
| cake1: EnumElement FrozenKind.Fixed in a fixed layout type changes position from 1 to 2 | ||
| cake1: EnumElement FrozenKind.Rigid in a fixed layout type changes position from 2 to 1 | ||
| cake1: Func fixedLayoutStruct.foo() in a fixed layout type changes position from 0 to 1 |
There was a problem hiding this comment.
Only re-ordering stored instance properties is bad.
Re-ordering anything else (computed properties, static properties, methods, etc) is OK.
There was a problem hiding this comment.
is VAD->hasStorage() && !VAD->isStatic() sufficient to check this? frozen enums don't need this constraint, right?
There was a problem hiding this comment.
Yes, it is sufficient. For enums, the equivalent is to just check all EnumElementDecls.
| @@ -423,6 +429,11 @@ bool SDKNodeDeclType::isConformingTo(KnownProtocolKind Kind) const { | |||
| } | |||
| } | |||
|
|
|||
| bool SDKNodeDeclType::hasFixedLayout() const { | |||
There was a problem hiding this comment.
This will still return false for non-resilient modules, where all declarations are fixed-layout/frozen. Instead, use NominalTypeDecl::isResilient().
| @@ -674,6 +674,23 @@ static bool isOwnershipEquivalent(ReferenceOwnership Left, | |||
| return false; | |||
| } | |||
|
|
|||
| static void detectReordering(SDKNodeDecl *L, SDKNodeDecl *R, SDKContext &Ctx) { | |||
| if (L->isImplicit()) | |||
There was a problem hiding this comment.
It's not correct to ignore implicit declarations. For example, I think the underlying storage of a lazy property is implicit. Please test re-ordering lazy properties.
| assert(!Left); | ||
| Right->annotate(NodeAnnotation::Added); | ||
| if (Ctx.checkingABI()) { | ||
| // Adding members to a fixed layout type is ABI-breaking. |
There was a problem hiding this comment.
Removing members too (even if they're not public).
There was a problem hiding this comment.
I'll have a separate PR for adding/removing nodes.
nkcsgexi
force-pushed
the
diagnose-reorder
branch
2 times, most recently
from
1d496a3 to
2fc8c55
Compare
|
@swift-ci please smoke test |
…ut type under ABI mode.
nkcsgexi
force-pushed
the
diagnose-reorder
branch
from
2fc8c55 to
05e1592
Compare
|
@swift-ci please smoke test |
|
@swift-ci Please smoke test Linux platform |
No description provided.