Skip to content

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
Member

@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
Copy link
Member

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 (swiftlang#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
(swiftlang#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
(swiftlang#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
(swiftlang/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.

4 participants