Skip to content

[llvm] remove incorrect LLVM_ABI annotation usage #144606

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jun 23, 2025
Merged

[llvm] remove incorrect LLVM_ABI annotation usage #144606

merged 2 commits into from
Jun 23, 2025

Conversation

andrurogerz
Copy link
Contributor

@andrurogerz andrurogerz commented Jun 17, 2025 •
edited
Loading

Overview

This patch removes incorrect usage of LLVM_ABI macros that break the (currently incomplete) LLVM Windows DLL build.

  • ec71d80 added LLVM_ABI to template methods fully declared in a header file
  • 254a92d added LLVM_ABI to members of a class already annotated with LLVM_ABI

Background

Documentation for LLVM_ABI and related annotations is found in the LLVM repo here.

@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 17, 2025 •
edited
Loading

✅ With the latest revision this PR passed the C/C++ code formatter.

@andrurogerz andrurogerz marked this pull request as ready for review June 17, 2025 23:17
@andrurogerz
Copy link
Contributor Author

@compnerd, @vgvassilev here's the first PR I've had to make to fix mistaken LLVM_ABI annotations by others. I think we will need to update to ids so that it catches this sort of mistake. Right now, ids focuses on on where annotations are missing, but to fully prevent Windows DLL build breaks, it will also need to flag annotations that are unnecessary or in the wrong place.

@vgvassilev
Copy link
Contributor

@compnerd, @vgvassilev here's the first PR I've had to make to fix mistaken LLVM_ABI annotations by others. I think we will need to update to ids so that it catches this sort of mistake. Right now, ids focuses on on where annotations are missing, but to fully prevent Windows DLL build breaks, it will also need to flag annotations that are unnecessary or in the wrong place.

That’s a good point and probably a good time for the ci work to start. Can you find us on discord — we have a few questions for you outside of this pr ;)

vgvassilev
vgvassilev approved these changes Jun 20, 2025
View reviewed changes
Copy link
Contributor

@vgvassilev vgvassilev left a comment

Choose a reason for hiding this comment

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

LGTM!

@compnerd compnerd merged commit c0ce9ad into llvm:main Jun 23, 2025
9 checks passed
miguelcsx pushed a commit to miguelcsx/llvm-project that referenced this pull request Jun 23, 2025
@andrurogerz @miguelcsx
[llvm] remove incorrect LLVM_ABI annotation usage (llvm#144606)
a50b880 
## Overview
This patch removes incorrect usage of `LLVM_ABI` macros that break the
(currently incomplete) LLVM Windows DLL build.
- ec71d80 added `LLVM_ABI` to template methods fully declared in a
header file
- 254a92d added `LLVM_ABI` to members of a class already annotated with
`LLVM_ABI`

# Background
Documentation for `LLVM_ABI` and related annotations is found in the
LLVM repo
[here](https://github.com/llvm/llvm-project/blob/main/llvm/docs/InterfaceExportAnnotations.rst).
Jaddyen pushed a commit to Jaddyen/llvm-project that referenced this pull request Jun 23, 2025
@andrurogerz @Jaddyen
[llvm] remove incorrect LLVM_ABI annotation usage (llvm#144606)
9003152 
## Overview
This patch removes incorrect usage of `LLVM_ABI` macros that break the
(currently incomplete) LLVM Windows DLL build.
- ec71d80 added `LLVM_ABI` to template methods fully declared in a
header file
- 254a92d added `LLVM_ABI` to members of a class already annotated with
`LLVM_ABI`

# Background
Documentation for `LLVM_ABI` and related annotations is found in the
LLVM repo
[here](https://github.com/llvm/llvm-project/blob/main/llvm/docs/InterfaceExportAnnotations.rst).
anthonyhatran pushed a commit to anthonyhatran/llvm-project that referenced this pull request Jun 26, 2025
@andrurogerz
[llvm] remove incorrect LLVM_ABI annotation usage (llvm#144606)
6679f11 
## Overview
This patch removes incorrect usage of `LLVM_ABI` macros that break the
(currently incomplete) LLVM Windows DLL build.
- ec71d80 added `LLVM_ABI` to template methods fully declared in a
header file
- 254a92d added `LLVM_ABI` to members of a class already annotated with
`LLVM_ABI`

# Background
Documentation for `LLVM_ABI` and related annotations is found in the
LLVM repo
[here](https://github.com/llvm/llvm-project/blob/main/llvm/docs/InterfaceExportAnnotations.rst).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vgvassilev vgvassilev vgvassilev approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@andrurogerz @vgvassilev @compnerd