Skip to content
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

[APINotes] Support SwiftImportAs for C++ structs #7386

Merged
merged 1 commit into from
Sep 5, 2023

Conversation

egorzhdan
Copy link

This allows annotating C++ types as e.g. immortal references using API Notes.

This is based on the original patch by Zoe (411d3ab).

rdar://114933812

@egorzhdan
Copy link
Author

I will also submit a part of this patch for review upstream once it is approved.

Copy link
Collaborator

@compnerd compnerd left a comment

Choose a reason for hiding this comment

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

Would be good to start flowing this upstream.

Comment on lines 541 to 545
if (ImportAsLength > 0) {
info.SwiftImportAs = (std::string(reinterpret_cast<const char *>(data),
ImportAsLength - 1));
data += ImportAsLength - 1;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

What do you think of this pattern?

Suggested change
if (ImportAsLength > 0) {
info.SwiftImportAs = (std::string(reinterpret_cast<const char *>(data),
ImportAsLength - 1));
data += ImportAsLength - 1;
}
if (auto BiasedLength = ImportAsLength - 1) {
info.SwiftImportAs = (std::string(reinterpret_cast<const char *>(data),
BiasedLength));
data += BiasedLength;
}

Copy link
Author

Choose a reason for hiding this comment

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

This underflows unsigned if ImportAsLength == 0, we could cast it to signed but I think that would slightly hurt readability.

This allows annotating C++ types as e.g. immortal references using API Notes.

This is based on the original patch by Zoe (411d3ab).

rdar://114933812
Copy link

@zoecarver zoecarver left a comment

Choose a reason for hiding this comment

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

This is super awesome! Thank you Egor! This will make a lot of people very happy :)

@egorzhdan egorzhdan merged commit 1279ac6 into next Sep 5, 2023
@egorzhdan egorzhdan deleted the egorzhdan/apinotes-importas branch September 5, 2023 12:41
egorzhdan added a commit to egorzhdan/llvm-project that referenced this pull request Sep 5, 2023
This upstreams a few Clang API Notes attributes that were recently added downstream in the Apple fork (apple#7386).
egorzhdan added a commit to llvm/llvm-project that referenced this pull request Sep 7, 2023
This upstreams a few Clang API Notes attributes that were recently added
downstream in the Apple fork
(apple#7386).
avillega pushed a commit to avillega/llvm-project that referenced this pull request Sep 11, 2023
This upstreams a few Clang API Notes attributes that were recently added
downstream in the Apple fork
(apple#7386).
qihangkong pushed a commit to rvgpu/llvm that referenced this pull request Apr 18, 2024
This upstreams a few Clang API Notes attributes that were recently added
downstream in the Apple fork
(apple/llvm-project#7386).
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.

None yet

3 participants