tim: Tim with short hair, smiling, wearing a black jacket over a white T-shirt (Default)
[personal profile] tim
I haven't written anything under this tag in a long while. For most of that time, I've been head-down working on the let/match refactoring in trans. I feel like I've learned a ton about trans in the meantime that I will be able to translate into comments that will help future trans hackers (pun possibly intended). But I also feel embarrassed about not having a lot of visible work to show for the past month or month-and-a-half.

The middle of this week, though, I decided to set that aside since we have the 0.5 release coming up soon, and I didn't think I was going to finish #3235 in time. Instead, I decided to just try to fix as many release blockers as possible. I'm particularly satisfied to have fixed #3121 just now (well, it's not committed yet, still waiting for tests to run). I spent a good chunk of time trying to isolate this bug when it was newer, and I gave up because I didn't understand trans::alt well enough. But thanks to that time I've spent trying to refactor, when I looked at it again today, I was able to find the buggy code in just a few minutes. The bug is even simple enough to explain in terms of code (I think):
fn enter_default(bcx: block, dm: DefMap, m: &[@Match/&r],
                 col: uint, val: ValueRef)
    -> ~[@Match/&r]
    do enter_match(bcx, dm, m, col, val) |p| {
        match p.node {
          ast::pat_wild | ast::pat_rec(_, _) | ast::pat_tup(_) |
          ast::pat_struct(*) => Some(~[]),
          ast::pat_ident(_, _, None) if pat_is_binding(dm, p) => Some(~[]),
          _ => None

This is the code that takes a list of Matchs (structures representing patterns that may match) and narrows down that list to patterns that are irrefutable: that match any value of the right type. Examples are _, the wildcard pattern that matches anything, and x if x is a free variable (which just binds any value to the name x. enter_match is a more general function -- the details aren't important -- but the point is that it takes a higher-order function that classifies patterns. Here, the match on p says that the following kinds of patterns are defaults: wildcards (_); record, struct, and tuple patterns (that is, matches on constructors for product types), and variable patterns. The higher-order function returns Some(~[]) (again, the reason why doesn't matter) for defaults, and None for non-defaults.

That's all fine and good, but in Rust we have pattern guards; to use the example from #3121:
match *m {
      to_go(_) => { }
      for_here(_) if cond => {}
      for_here(hamburger) => {}
      for_here(fries(s)) => {}
      for_here(shake) => {}

Normally, for_here(_) would match any value of type order (see the bug report for the type definitions), since the _ nested pattern matches anything. But the pattern guard -- written if cond -- means this pattern only matches if cond is true. So the _ here isn't really a default, because a guard makes the entire pattern that it's attached to refutable.

Once I realized this, the fix was quite simple. And happily, I got to close several other issues that turned out to be instances of the very same bug: #2869, #3257, and #3895.

There's no greater moral here, no fancy theory; just another mundane, prosaic day in the life of a compiler writer. The real issue was that pattern guards were added on after the fact, and probably not tested all that thoroughly -- but I certainly can't place any blame for that, since I generally don't put as much effort as I should into test coverage either.

ETA: Well, the fix wasn't as simple as that after all, sadly. Don't want to have an "I am so smart" moment here. :P Still working on it.
Anonymous( )Anonymous This account has disabled anonymous posting.
OpenID( )OpenID You can comment on this post while signed in with an account from many other sites, once you have confirmed your email address. Sign in using OpenID.
User (will be screened if not on Access List)
Account name:
If you don't have an account you can create one now.
HTML doesn't work in the subject.


Notice: This account is set to log the IP addresses of everyone who comments.
Links will be displayed as unclickable URLs to help prevent spam.


tim: Tim with short hair, smiling, wearing a black jacket over a white T-shirt (Default)
Tim Chevalier

October 2017

8910 11121314
15 161718192021

Most Popular Tags

Style Credit

Expand Cut Tags

No cut tags