Support weak definitions #4414
Open
Support weak definitions #4414
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When a symbol only has a weak definition, this definition will be picked. When a symbol has both a weak and a regular definition, the regular definition will be picked instead.
|
This has been extracted out of rust-lang/rust#134522. |
bjorn3
commented
Jun 26, 2025
bjorn3
force-pushed
the
weak_defs
branch
2 times, most recently
from
9284d4c to
5a433c4
Compare
RalfJung
reviewed
Jun 28, 2025
| @@ -138,7 +139,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { | |||
| Entry::Occupied(e) => e.into_mut(), | |||
| Entry::Vacant(e) => { | |||
| // Find it if it was not cached. | |||
| let mut instance_and_crate: Option<(ty::Instance<'_>, CrateNum)> = None; | |||
| let mut instance_and_crate: Option<(ty::Instance<'_>, CrateNum, bool)> = None; | |||
There was a problem hiding this comment.
Please explain what the bool means, or use a self-explaining type. It might be worth using a struct instead of a tuple.
| if let Some((original_instance, original_cnum, original_is_weak)) = | ||
| instance_and_crate | ||
| { | ||
| match (is_weak, original_is_weak) { |
There was a problem hiding this comment.
Suggested change
| match (is_weak, original_is_weak) { | |
| // There is more than one definition with this name. What we do now | |
| // depends on whether one or both definitions are weak. | |
| match (is_weak, original_is_weak) { |
| (true, true) | (false, false) => { | ||
| // Either both definitions are non-weak or both are weak. In | ||
| // either case return an error. For weak definitions we error | ||
| // because it is undefined which definition would have been |
There was a problem hiding this comment.
Suggested change
| // because it is undefined which definition would have been | |
| // because it is unspecified which definition would have been |
Comment on lines
+215
to
+222
| if let Some((instance, _, _)) = instance_and_crate { | ||
| if !matches!(tcx.def_kind(instance.def_id()), DefKind::Fn | DefKind::AssocFn) { | ||
| throw_ub_format!( | ||
| "attempt to call an exported symbol that is not defined as a function" | ||
| ); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
I guess this logic can live here as well, but is there a specific reason you moved it?
There was a problem hiding this comment.
Please rename this to exported_symbol_weak for better test grouping.
|
@rustbot author |
rustbot
added
the
S-waiting-on-author
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When a symbol only has a weak definition, this definition will be picked. When a symbol has both a weak and a regular definition, the regular definition will be picked instead.
In the future the allocator shim may be replaced by weak definitions. This already adds support to simplify things in the future.