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

Make IpvXAddr::new const fns and the well known addresses associated constants #52872

Merged
merged 8 commits into from
Aug 8, 2018

Conversation

faern
Copy link
Contributor

@faern faern commented Jul 30, 2018

Implements/fixes #44582

I just got a PR towards libc (rust-lang/libc#1044) merged. With the new feature added in that PR it is now possible to create in6_addr instances as consts. This enables us to finally make the constructors of the IP structs const fns and to make the localhost/unspecified addresses associated constants, as agreed in the above mentioned tracking issue.

I also added a BROADCAST constant. Personally this is the well known address I tend to need the most often.

@rust-highfive
Copy link
Collaborator

r? @TimNN

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive
Copy link
Collaborator

⚠️ Warning ⚠️

  • These commits modify submodules.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 30, 2018
@faern faern force-pushed the use-modern-alignment-libc branch from 58982c8 to 312cdb4 Compare July 30, 2018 18:53
@faern
Copy link
Contributor Author

faern commented Jul 30, 2018

I realize now we might want to use the same feature gate name for the associated constants and the constructors. I did not think about that before. But I'll wait for some feedback before doing anything about it.

@faern
Copy link
Contributor Author

faern commented Jul 31, 2018

It should be noted that we can get away without unlocking #![feature(const_int_ops)] if desired. We could have a local const fn hton(u32) -> u32 that takes care of converting to big endian.

We can also get the associated constants without making the constructors const fns. We can just have private const constructors that we use in the normal constructors and for creating the associated constants, but the public constructors can be non-const if that is desired for some reason. I went with making the constructors const since I think that would be very useful in general. Having the IP constructors const is what drove me to implement this :)

@TimNN
Copy link
Contributor

TimNN commented Aug 1, 2018

Thanks for your PR! This looks generally good, I'll take a detailed look tomorrow. Having an unstable const-constructor and using const_int_ops is fine, I think.

@TimNN
Copy link
Contributor

TimNN commented Aug 3, 2018

@bors r+

@bors
Copy link
Contributor

bors commented Aug 3, 2018

📌 Commit 312cdb4 has been approved by TimNN

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 3, 2018
@bors
Copy link
Contributor

bors commented Aug 3, 2018

⌛ Testing commit 312cdb4 with merge 362961dac287cb17ecc0e79ed39dbd90092f82ce...

@bors
Copy link
Contributor

bors commented Aug 3, 2018

💔 Test failed - status-travis

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 3, 2018
@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-aux of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
[00:29:56] [RUSTC-TIMING] proc_macro test:false 18.180
[00:29:56]    Compiling syntax_ext v0.0.0 (file:///checkout/src/libsyntax_ext)
[00:30:33] [RUSTC-TIMING] syntax_ext test:false 36.797

Broadcast message from root@travis-job-21a0d1b5-843e-408b-a860-9ddf81b9ea94
 (unknown) at 17:44 ...
The system is going down for power off NOW!
[00:35:02] Makefile:28: recipe for target 'all' failed
[00:35:02] Makefile:28: recipe for target 'all' failed
[00:35:02] Session terminated, terminating shell... ...terminated.
[00:35:02] make: *** [all] Terminated

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 143.
travis_time:start:08ef844a
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)

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 @TimNN. (Feature Requests)

@TimNN
Copy link
Contributor

TimNN commented Aug 3, 2018

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 3, 2018
cramertj added a commit to cramertj/rust that referenced this pull request Aug 3, 2018
…TimNN

Make IpvXAddr::new const fns and the well known addresses associated constants

Implements/fixes rust-lang#44582

I just got a PR towards libc (rust-lang/libc#1044) merged. With the new feature added in that PR it is now possible to create `in6_addr` instances as consts. This enables us to finally make the constructors of the IP structs const fns and to make the localhost/unspecified addresses associated constants, as agreed in the above mentioned tracking issue.

I also added a BROADCAST constant. Personally this is the well known address I tend to need the most often.
@bors
Copy link
Contributor

bors commented Aug 4, 2018

⌛ Testing commit 312cdb4 with merge facbc12...

bors added a commit that referenced this pull request Aug 4, 2018
Make IpvXAddr::new const fns and the well known addresses associated constants

Implements/fixes #44582

I just got a PR towards libc (rust-lang/libc#1044) merged. With the new feature added in that PR it is now possible to create `in6_addr` instances as consts. This enables us to finally make the constructors of the IP structs const fns and to make the localhost/unspecified addresses associated constants, as agreed in the above mentioned tracking issue.

I also added a BROADCAST constant. Personally this is the well known address I tend to need the most often.
@bors
Copy link
Contributor

bors commented Aug 4, 2018

💔 Test failed - status-travis

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 4, 2018
@kennytm kennytm added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Aug 8, 2018
@faern
Copy link
Contributor Author

faern commented Aug 8, 2018

@TimNN There, I found it with the help of some Redox people on IRC. Apparently Redox does not use in6_addr from libc, but rather define their own version deep down in libstd. This should hopefully build now!

@TimNN
Copy link
Contributor

TimNN commented Aug 8, 2018

Awesome work, @faern!

@bors r+

@bors
Copy link
Contributor

bors commented Aug 8, 2018

📌 Commit 6e7bbff6f66ab74a49b4f2aacb5d0073b0ebf1ee has been approved by TimNN

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 8, 2018
@bors
Copy link
Contributor

bors commented Aug 8, 2018

🔒 Merge conflict

This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again.

How do I rebase?

Assuming self is your fork and upstream is this repository, you can resolve the conflict following these steps:

  1. git checkout use-modern-alignment-libc (switch to your branch)
  2. git fetch upstream master (retrieve the latest master)
  3. git rebase upstream/master -p (rebase on top of it)
  4. Follow the on-screen instruction to resolve conflicts (check git status if you got lost).
  5. git push self use-modern-alignment-libc --force-with-lease (update this PR)

You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial.

Please avoid the "Resolve conflicts" button on GitHub. It uses git merge instead of git rebase which makes the PR commit history more difficult to read.

Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Cargo.lock conflict is handled during merge and rebase. This is normal, and you should still perform step 5 to update this PR.

Error message
warning: Cannot merge binary files: src/Cargo.lock (HEAD vs. heads/homu-tmp)
Auto-merging src/Cargo.lock
CONFLICT (content): Merge conflict in src/Cargo.lock
Automatic merge failed; fix conflicts and then commit the result.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 8, 2018
@faern faern force-pushed the use-modern-alignment-libc branch from 6e7bbff to c0041f4 Compare August 8, 2018 08:53
@faern
Copy link
Contributor Author

faern commented Aug 8, 2018

Trying to get anything merged in Cargo.lock is a pain 😅 And I strictly don't need that any more, since the fix was to change in6_addr in libstd itself. So if it fails one more time because of a merge conflict I'll skip trying to bump libc in Cargo.lock.

@TimNN
Copy link
Contributor

TimNN commented Aug 8, 2018

@bors r+

@bors
Copy link
Contributor

bors commented Aug 8, 2018

📌 Commit c0041f4 has been approved by TimNN

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 8, 2018
@bors
Copy link
Contributor

bors commented Aug 8, 2018

⌛ Testing commit c0041f4 with merge ebe8df4...

bors added a commit that referenced this pull request Aug 8, 2018
Make IpvXAddr::new const fns and the well known addresses associated constants

Implements/fixes #44582

I just got a PR towards libc (rust-lang/libc#1044) merged. With the new feature added in that PR it is now possible to create `in6_addr` instances as consts. This enables us to finally make the constructors of the IP structs const fns and to make the localhost/unspecified addresses associated constants, as agreed in the above mentioned tracking issue.

I also added a BROADCAST constant. Personally this is the well known address I tend to need the most often.
@bors
Copy link
Contributor

bors commented Aug 8, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: TimNN
Pushing ebe8df4 to master...

@bors bors merged commit c0041f4 into rust-lang:master Aug 8, 2018
@faern faern deleted the use-modern-alignment-libc branch January 31, 2019 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tracking issue for IP constructors
5 participants