Skip to content

Frontend: add a frontend flag to generate empty ABI descriptors to workaround deserialization issues #41538

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
Feb 24, 2022

Conversation

nkcsgexi
Copy link
Contributor

ABI descriptors should always be emitted as sidecars for library-evolution-enabled modules.
However, generating these files requires traversing the entire module (like indexing), which may
hit additional deserialization issues. To unblock builds, this patch introduces a flag to skip
the traversing logic so that we emit an empty ABI descriptor file. The empty file serves as
a placeholder so that the build system doesn't need to know the details.

@nkcsgexi
Copy link
Contributor Author

@swift-ci please smoke test

…rkaround deserialization issues

ABI descriptors should always be emitted as sidecars for library-evolution-enabled modules.
However, generating these files requires traversing the entire module (like indexing), which may
hit additional deserialization issues. To unblock builds, this patch introduces a flag to skip
the traversing logic so that we emit an empty ABI descriptor file. The empty file serves as
a placeholder so that build system doesn't need to know the details.
@nkcsgexi
Copy link
Contributor Author

@swift-ci please smoke test

Copy link
Contributor

@bnbarham bnbarham left a comment

Choose a reason for hiding this comment

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

Small somewhat nitpick of a change, but otherwise LGTM.

Comment on lines +2514 to +2522
SWIFT_DEFER {
payload.allContsValues = &extractor.getAllConstValues();
dumpSDKRoot(collector.getSDKRoot(), payload, OutputFile);
};
if (Empty) {
return;
}
collector.lookupVisibleDecls({MD});
extractor.extract(MD);
Copy link
Contributor

Choose a reason for hiding this comment

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

Works fine obviously, but I think:

if (!Empty) {
  collector.lookupVisibleDecls({MD});
  extractor.extract(MD);
  payload.allContsValues = &extractor.getAllConstValues();
}
dumpSDKRoot(collector.getSDKRoot(), payload, OutputFile);

Is a little more obvious. dumpSDKRoot handles an empty PayLoad. Alternatively

if (Empty) {
  dumpSDKRoot(collector.getSDKRoot(), OutputFile)
  return;
}

Right after the collector is nice too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for taking a look! @bnbarham The reason I still prefer we set the payload to an empty array of constant values is that the client side, likely written in Swift, can still parse the JSON to get an empty table.

@nkcsgexi nkcsgexi merged commit 5473cec into swiftlang:main Feb 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants