Fix #[linkage] propagation though generic functions #52635
Merged
Conversation
This file contains 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
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nikomatsakis (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
rust-highfive
added
the
S-waiting-on-review
oli-obk
reviewed
Jul 23, 2018
src/librustc_codegen_llvm/consts.rs
Outdated
|
|
||
| let codegen_fn_attrs = cx.tcx.codegen_fn_attrs(def_id); | ||
| let llty = cx.layout_of(ty).llvm_type(cx); | ||
| let g = if let Some(linkage) = codegen_fn_attrs.linkage { |
There was a problem hiding this comment.
can you pull this code out into a function so it isn't duplicated?
|
@bors r+ thanks! |
|
📌 Commit 7e6f2f660edf9d3abade64e0b76904ff77d5952c has been approved by |
bors
added
S-waiting-on-bors
|
⌛ Testing commit 7e6f2f660edf9d3abade64e0b76904ff77d5952c with merge 9134499fb5d75da87f2c76cb7ab3d7d32089791f... |
|
💔 Test failed - status-travis |
bors
added
S-waiting-on-review
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
kennytm
added
the
S-waiting-on-author
yes. Maybe also disable for wasm? Not sure if that can cope with it? |
Looking at the homu log it seems wasm succeeded which I was not expecting when seeing this failure. Looking at it closer it seems that might be because it is building with |
Signed-off-by: Gabriel Smith <ga29smith@gmail.com>
Fixes issue rust-lang#18804 Signed-off-by: Gabriel Smith <ga29smith@gmail.com>
Signed-off-by: Gabriel Smith <ga29smith@gmail.com>
…smjs The Emscripten compiler does not support weak symbols at the moment. Signed-off-by: Gabriel Smith <ga29smith@gmail.com>
|
@oli-obk Everything is rebased to be able to merge and should be ready to go. |
oli-obk
reviewed
Jul 25, 2018
| // Test for issue #18804, #[linkage] does not propagate thorugh generic | ||
| // functions. Failure results in a linker error. | ||
|
|
||
| // aux-build:lib.rs |
There was a problem hiding this comment.
doesn't this file also need the //ignore-* comments? Or is compiletest smart enough to figure out that if an aux-build isn't built we also shouldn't run the test depending on it?
r=me with that question resolved
There was a problem hiding this comment.
You are correct. I put the comments in the wrong file while not thinking. The main test file needs the comments while the auxiliary file does not.
Signed-off-by: Gabriel Smith <ga29smith@gmail.com>
|
@bors r+ |
|
📌 Commit be5b668 has been approved by |
bors
added
S-waiting-on-bors
|
Thanks! I didn't know the aux files don't need the comments, but it kinda makes sense. They are only compiled if a file referencing them is compiled. |
Mark-Simulacrum
added a commit
to Mark-Simulacrum/rust
that referenced
this pull request
Jul 26, 2018
Fix #[linkage] propagation though generic functions Fixes rust-lang#18804 In the non-local branch of `get_static` (where the fix was implemented) `span_fatal` had to be replaced with `bug!` as we have no span in that case.
bors
added a commit
that referenced
this pull request
Jul 26, 2018
Rollup of 16 pull requests Successful merges: - #52558 (Add tests for ICEs which no longer repro) - #52610 (Clarify what a task is) - #52617 (Don't match on region kinds when reporting NLL errors) - #52635 (Fix #[linkage] propagation though generic functions) - #52647 (Suggest to take and ignore args while closure args count mismatching) - #52649 (Point spans to inner elements of format strings) - #52654 (Format linker args in a way that works for gcc and ld) - #52667 (update the stdsimd submodule) - #52674 (Impl Executor for Box<E: Executor>) - #52690 (ARM: expose `rclass` and `dsp` target features) - #52692 (Improve readability in a few sorts) - #52695 (Hide some lints which are not quite right the way they are reported to the user) - #52718 (State default capacity for BufReader/BufWriter) - #52721 (std::ops::Try impl for std::task::Poll) - #52723 (rustc: Register crates under their real names) - #52734 (sparc ABI issue - structure returning from function is returned in 64bit registers (with tests)) Failed merges: - #52678 ([NLL] Use better spans in some errors) r? @ghost
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.
Fixes #18804
In the non-local branch of
get_static(where the fix was implemented)span_fatalhad to be replaced withbug!as we have no span in that case.