Skip to content

Conversation

@maurer
Copy link

@maurer maurer commented Dec 3, 2025

While the functionality of this flag is obvious in the implementation, tool users may not know what it does with the short description provided. Notably, it is not obvious from the short description that:

  • Functions provided will be converted to internal linkage (and thus discarded if unused) even if unreferenced.
  • Functions in the first file will not be internalized, even if referenced by a later one.

The Rust for Linux project has found use for this flag to support inlining static inline functions in C into code compiled by Rust when rustc and clang share a LLVM.

I wanted to send this PR both to:

  1. Help others find the flag in the future
  2. Clarify that our understanding of what we can rely upon this option to do is correct, and not just what the implementation happens to do at the moment.

cc @Darksonn (also with Rust-for-Linux)

cc @nikic Perhaps able to review or route us to an appropriate reviewer?

While the functionality of this flag is obvious in the implementation,
tool users may not know what it does with the short description
provided. Notably, it is not obvious from the short description that:

* Functions provided will be converted to internal linkage (and thus
  discarded if unused) even if unreferenced.
* Functions in the first file will not be internalized, even if
  referenced by a later one.
@github-actions
Copy link

github-actions bot commented Dec 3, 2025

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@nikic nikic requested a review from teresajohnson December 3, 2025 07:50
@nikic
Copy link
Contributor

nikic commented Dec 3, 2025

I'd suggest adding more detailed documentation to the man page (https://github.com/llvm/llvm-project/blob/main/llvm/docs/CommandGuide/llvm-link.rst) rather than the option description.

@nikic nikic requested review from Artem-B and JDevlieghere December 3, 2025 07:53
Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

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

I believe your description of the behavior is correct and I wouldn't expect it to change, but I've added a few more people to double check, as I'm not particularly familiar with llvm-link.

@teresajohnson
Copy link
Contributor

I'm not an llvm-link user, but it does look like I reviewed @JDevlieghere 's change many years ago to add this functionality (https://reviews.llvm.org/D30738). This change is largely accurate, but in fact looking at that old patch the point was to internalize all but some special symbols, and symbols referenced by llvm*.used. That's probably a bit wordy to add to the option description, but you might want to note that it isn't all symbols.

I agree with @nikic 's suggestion to add documentation to the man page as well.

@Artem-B Artem-B removed their request for review December 4, 2025 19:31
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.

3 participants