Skip to content
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

Add testcase for issue-32948 #36832

Merged
merged 1 commit into from
Oct 6, 2016
Merged

Add testcase for issue-32948 #36832

merged 1 commit into from
Oct 6, 2016

Conversation

ParkHanbum
Copy link
Contributor

@ParkHanbum ParkHanbum commented Sep 29, 2016

#32948

issue-32948 is similar to issue-32554.

issue-32948 : Symbol names for monomorphized trait impls are not stable across crates
issue-32554 : Symbol names for generics are not stable across crates

so, I append issue-32948's testcase to issue-32554's testcase.
thanks!

@rust-highfive
Copy link
Collaborator

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.

@nikomatsakis
Copy link
Contributor

r? @michaelwoerister

@michaelwoerister
Copy link
Member

Thanks a lot, @ParkHanbum!
This sed script is getting a bit unreadable (due to no fault of yours). Can you replace it with the following?:

# The following command will:
#  1. dump the symbols of a library using `nm`
#  2. extract only those lines that we are interested in via `grep`
#  3. from those lines, extract just the symbol name via `sed`
#     (symbol names always start with "_ZN" and end with "E")
#  4. sort those symbol names for deterministic comparison
#  5. write the result into a file

dump-symbols = nm "$(TMPDIR)/lib$(1).rlib" \
             | grep -E "some_test_function|Bar|bar" \
             | sed "s/.*\(_ZN.*E\).*/\1/" \
             | sort \
             > "$(TMPDIR)/$(1).nm"

That would make it easier for future maintainers. If you do that, I'd be happy to approve the PR.

@ParkHanbum
Copy link
Contributor Author

@michaelwoerister cosider it done!! I'll be better then this PR next. thanks a lot !!

@michaelwoerister
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Oct 6, 2016

📌 Commit cb700e7 has been approved by michaelwoerister

@michaelwoerister
Copy link
Member

Thanks a lot, @ParkHanbum! My comment wasn't meant as criticism. sed scripts are just hard to decipher :)

@michaelwoerister
Copy link
Member

@bors p=rollup

Let's see if that works...

@michaelwoerister
Copy link
Member

@bors rollup

sophiajt pushed a commit to sophiajt/rust that referenced this pull request Oct 6, 2016
Add testcase for issue-32948

issue-32948 is similar to issue-32554.

issue-32948 : Symbol names for monomorphized trait impls are not stable across crates
issue-32554 : Symbol names for generics are not stable across crates

so, I append issue-32948's testcase to issue-32554's testcase.
thanks!
bors added a commit that referenced this pull request Oct 6, 2016
@bors bors merged commit cb700e7 into rust-lang:master Oct 6, 2016
@michaelwoerister
Copy link
Member

🎉

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.

5 participants