Skip to content
This repository has been archived by the owner on Oct 9, 2018. It is now read-only.

Latest commit

 

History

History
94 lines (75 loc) · 9.57 KB

2015-04-21.md

File metadata and controls

94 lines (75 loc) · 9.57 KB
Raw

Agenda 4/21/2015

Attending

acrichto, larsbergstrom, aturon, steveklabnik, huonw, pnkfelix, brson

Status

  • acrichto: deprecation removal, fs RFC
  • pnkfelix: drop issues (1.0 and beyond)
  • brson: making crater usable, party planning

Servo licensing info

  • larsberg: Servo, of course, has licensing issues. All Rust code is statically linked, which imposes some restrictions. Legal says that MIT, BSD, Apache, and MPL 2 are the set of allowable licenses. Of course, I can just hack up our python scripts to check it. But should we be able to do something with Cargo to check declaratively about licenses?
  • acrichto: We have the info. When you publish, we have to know the license. But there's not a roadmap for imposing this kind of restriction on license mismatch. For now, the best way is probably to post-process Cargo.lock and have a tidy script that hits crates.io for the licenses.
  • acrichto: It'd be a nice feature, but it would be some work to implement. Could even have a subcommand. Glad to help out with a script, but it may be a while before it's fully built in to Cargo.
  • acrichto: Also, Cargo favors statically linking C libraries, and those licenses aren't listed in Cargo.toml. You'll have to figure out something there.

iter methods and overflow 1

  • pnkfelix: A community member has noticed that a number of our iteration methods can overflow. They generally work with usize. Examples are: enumerate, position, rposition, and count. If you're on a 32 bit target and you're working on an iterator that's not bounded by memory size, you could have a problem.
  • pnkfelix: There's a PR to use checked_add for things like count, position, rposition.
  • pnkfelix: May be questionable to use checked_add here. The vast majority of iterators can't possibly exhaust the space.
  • pnkfelix: One option would be to leave as-is, using addition, which will do overflow checks if you have debug_asserts on. So you still get that amount of checking.
  • pnkfelix: There are some other options. One is to add a size-bounded iterator trait. Could be OIBIT-style trait. Iterators would have to implement these traits to get the additional methods. There's some subtlety in terms of composition -- zipping two iterators versus chaining two iterators need to use different logic. That's a sort of baroque solution -- people have to understand these traits, whether to implement or opt out.
  • pnkfelix: An alternative approach would be to add methods to the iterator trait -- static methods -- for doing addition/subtraction and call out to those methods from within the iterator. Then those can be overridden. You could pass in context information about which method was called.
  • pnkfelix: Those are just some preliminary ideas, but I wanted to get feedback since this is a core API.
  • brson: Do you know how this relates to other languages and the choices they made?
  • pnkfelix: I don't know what other languages provide that have this level of genericity that also have overflow checking for integers. E.g. javascript and scheme will promote on overflow.
  • aturon: to be clear, the original PR was going to use checked_add and panic on overflow?
  • pnkfelix: yes, it was originally on all counting methods as well
  • aturon: what was the rationale for the original proposal?
  • pnkfelix: There were two separate goals. One was to have better feedback about why/where the overflow is occurring. Also, they wanted consistent behavior with/without debug assertions. I wasn't too convinced by the latter -- it might make sense in a cross-crate linkage scenario where you can't control the debug assertions.

...

  • aturon: gut reaction is that we should not single out iterators and fix a problem here, a solution should apply more generally. There's also the question of u64 vs usize in these elements, but we've learned that going too far to trying to solve lots of problems with the type system ends up with us backing away from these kinds of solutions. I feel pretty comfortable with these types as-is right now.

Float output formatting

  • pnkfelix: There's an awesome PR from lifthrasiir that was put out to correct our float->decimal formatting routine. The most commonly referenced routine is from Steele and White. (Dragon4?) The reason we haven't done this is that it requires larger precision arithmetic. But there's a cool paper from 2010 that can use integer arithmetic in 95% of the cases, with a fallback to the Dragon algorithm.

  • pnkfelix: The PR as written changes the output in certain ways. It's not just a matter of being more "correct" (objectively better). There are also some other, arguably breaking changes. For example, it changes from %g in C (either prints in exponential or non-exponential depending on what is shorter) -- which is really great when you're expecting it, but not great when you're not -- e.g. our json decoder. I wanted to check in on what kind of changes we'd allow for.

  • pnkfelix: But it turns out that the author is going to make changes to preserve the current behavior as much as possible. I think now the only changes are about the casing for NaN and inf. It never uses mixed-case. So I still wanted feedback.

  • steveklabnik: In the PR, the author was saying it was unclear what was even changed, since the original spec was so unclear.

  • pnkfelix: We may want to run this through crates.io to see how much breakage is involved. Also need to decide whether to get this into the beta.

  • acrichto: But crater doesn't run tests anymore, so this won't turn up much.

  • acrichto: I think this is way better than what we had before. Let's cherry pick it. [General agreement]

  • pnkfelix: The other link is about potentially-breaking changes to our floating point formatting.

  • pnkfelix: This is asking about adding a ".0" suffix for floating point values when displayed using {} even if there are no fractional amounts.

  • aturon: We talked a lot about conventions for Debug/Display. In particular, with Debug you are supposed to make some effort to allow it to be re-parsed as the original type, when applicable, but Display is for "human readability"

  • acrichto: I would not consider this a breaking change at this point. We're not really in a position to freeze the entire implementation of the std library; things like this probably will continue to change a bit.

  • pnkfelix: Also, I mentioned about the %g output format earlier. Is there a way to get that output format in the current fmt traits? Would you have to add a separate letter?

  • acrichto: The # attribute is the "alternate" way of formatting -- we could leverage that. Most other things are taken. But yes, we can add letters as needed.

  • pnkfelix: But we can't add new attributes to go before the letter? I couldn't tell from the fmt module how much was stabilized.

  • acrichto: I suspect as long as we can add something without ambiguity, we should be ok. This is all advisory anyway.

Branching model for beta

http://internals.rust-lang.org/t/release-channels-git-branching-and-the-release-process/1940/4

  • brson: I made a post to rust-internals about the branching model for beta. Wanted to go over what we'll do for PRs for beta in the short term.
  • brson: What Steve and I laid out is pretty similar to what we discussed before. To begin with, it will be manual.
  • brson: The proposal is that reviewers identify PRs that should be cherry-picked for beta. They just tag as "beta nominated". Then the author goes off and gets the PR landed on master
  • brson: Periodically, someone will go through all the beta-nominated PRs that have been closed.
  • acrichto: Would love a bot to produce a weekly triage. But manual for now sounds good.
  • brson: For now we should probably just include these in triage meetings and discuss the outcome.
  • acrichto: We'll have to be vigilant. It helps makes sure that things that do go into beta are good, but doesn't help make sure that things actually do make it in.
  • brson: There should be a checkpoint before release where you have no more nominated tags.
  • pnkfelix: Do we want a record that something has been merged into beta, to make sure that we don't accidentally try to re-merge?
  • steveklabnik: I'm assuming that, like rollups, we reference the original PR, which will show up as a link.
  • acrichto: We decided to push straight to beta and let that test, rather than auto?
  • brson: THe document leaves that pretty unspecced.
  • steveklabnik: It does say push to beta.
  • acrichto: So the merging process is cherry pick commits, make a PR that references all the others -- then you just hit the merge button.... immediately??
  • brson: I like opening a PR for visibility.
  • steveklabnik: At what level do we need to work on Homu to have it pay attention to an extra channel for testing? I'm not sure what the volume is going to be.
  • pnkfelix: We don't have to use homu, we can just hit the merge button as acrichto was saying.
  • acrichto: The artifact build process does run all the tests, so we may end up having to push small fixes, but I think it won't be to bad.
  • pnkfelix: So should anyone feel free to do the beta PRs? Oh nevermind, you said it would go through triage discussion first.
  • steveklabnik: I was thinking the r+ person can choose the tag. But the person doing the backporting is sepaate -- probably a single person who handles that.
  • brson: And who does the cherry pick?
  • steveklabnik: Same group. Didn't want to force pick someone. Well, anyone can make the PR. [some discussion of specific PRs to be backported]