-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
extern "C" functions are not internalized with -C lto #18134
Comments
Some further investigation shows that this is due to |
At some point, there was a decision to treat |
Why would there be no way to make them internally visible? |
I can't tell you the reasoning behind it because I strongly disagreed with it and made that clear when it happened. I can point you at the commit that implemented this. I didn't understand why the ABI is relevant to visibility then and I don't understand it now. IMO, it's an awful hack to work around broken code that could have just been fixed. |
Nominating since fixing this would be a backwards incompatible language change. It would begin causing errors due to missing symbols in previously valid code. |
@Zoxc can you provide a test case? I'm unable to reproduce this with a few different permutations |
Define a |
Buliding:
|
Assigning P-low, not 1.0. |
I'm actually going to close this as working as intended now that I've had some time to investigate it more thoroughly. The impetus for this functionality is #14500, which I do think is a real and valid issue to consider. Any reachable functions with a If you do not compile with |
Rust is exposing unreachable functions from the final executable. It would only be reachable if the executable crate exposed it as public at the top-level. The included library crate is private at the top-level so those symbols shouldn't get exported in an LTO build. If you want them exported, you should be able to mark the crate as public (or use There's a valid performance bug as long as there's not an equivalent to |
Currently we have no notion of a "private crate" or a "public crate", we just link crates together and the interface they expose is always public. I'm not sure why you say we don't have the equivalent of I also don't quite understand why you think that we're exposing unreachable functions. All of the exported functions are reachable with respect to privacy (as the example from @Zoxc shows). Do you have an example where an un-reachable extern function gets exposed? I do think that would be a bug (I haven't seen an example in this thread). Some exact reproducible code would also be more helpful than a description of a test case. |
One case where this would be useful is where is using lto so that cross-language optimizations can take place. For instance, one can (or should be able to) inline a small C function compiled with Clang into Rust code, or inline a Rust function into C code. In both cases, the function may be private to the combined library. |
@alexcrichton So we first made extern "C" functions stay around with LTO, but now we no longer do this, so I'm not sure -- should we close this issue? It's not clear whether we want or don't want this. |
I believe bugs have been fixed in the meantime, it looks like this issue is no longer applicable. |
internal: Extend SourceChangeBuilder to make make working with `SyntaxEditor`s easier Part of rust-lang#15710 Adds additional `SourceChangeBuilder` methods to make it easier to migrate assists to `SyntaxEditor`. As `SyntaxEditor`s are composable before they're completed, each created `SyntaxEditor` can represent logical groups of changes (e.g. independently performing renames of uses in a file from inserting the new item). Once a group of changes is considered "done", `SourceChangeBuilder::add_file_edits` is used to submit a set of changes to be part of the source change. `SyntaxAnnotation`s are used to indicate where snippets are attached to, and using `SyntaxAnnotation`s also means that we can attach snippets at any time, rather than being required to be after all edits.
When marking public functions with
#[no_mangle]
and then linking to the crate with the functions using-C lto
, the functions will be exported in the output.This can result in errors about duplicate definitions (when linking things linked with
-C lto
together) and also prevents the removal of unused functions.The text was updated successfully, but these errors were encountered: