softdevteam / lrpar

Rust LR parser
Other
1 stars 0 forks source link

Be more careful with compatible repairs. #65

Closed ltratt closed 6 years ago

ltratt commented 6 years ago

I forgot that when checking for compatible repairs we need to take into account this:

  // We follow Corcheulo et al.'s suggestions and never follow Deletes with
  // Inserts.

For example with the MF recoverer and this Java program:

  class C {
    void x(T ... y) { }
  }

before this patch we would find 4 repair sequences:

  Insert "IDENTIFIER", Delete ".", Delete "."
  Delete ".", Delete ".", Keep "y", Insert "IDENTIFIER"
  Insert "IDENTIFIER", Delete ".", Keep ".", Keep "y", Insert "IDENTIFIER"
  Insert "IDENTIFIER", Keep ".", Delete ".", Keep "y", Insert "IDENTIFIER"

The solution is fairly simple: we add an additional clause that if one repair sequence ends with a Delete the other must too. That allows us to find a further 4 repair sequences:

  Insert "IDENTIFIER", Keep ".", Insert "IDENTIFIER", Delete "."
  Insert "IDENTIFIER", Delete ".", Keep ".", Insert "IDENTIFIER"
  Insert "IDENTIFIER", Keep ".", Insert "IDENTIFIER", Keep ".", Insert "IDENTIFIER"
  Insert "IDENTIFIER", Keep ".", Insert "IDENTIFIER", Keep ".", Keep "y", Insert "IDENTIFIER"

While here, tidy up the compatibility check to make it easier to read.

ptersilie commented 6 years ago

This example is a bit unclear if we don't know what Java version we are targeting:

class C {
    void x(T ... y) { }
  }

In Java7 upwards the three dots are lexed as a single token ELLIPSIS. So the repair sequences might look a bit different. On that note, the Java7 grammar is ready and there's a PR waiting for you. So you can test this example with Java7 if you like.

ltratt commented 6 years ago

I was trying with the 1.5 grammar, so it didn't parse this at all. I'll look at the other PR first though.

ltratt commented 6 years ago

So should I update the comment to make clear the example was for Java 1.5? I'll have to do a force push.

ptersilie commented 6 years ago

I don't mind. I just thought I bring it to your attention that the repair is probably different in Java7, i.e. it's gonna be something like 1 delete (...) and then one insert (.).

ltratt commented 6 years ago

Clarified the commit message to make clear it's relative to the Java 1.5 grammar.

ptersilie commented 6 years ago

Good. And merged!