RazrFalcon / rustybuzz

A complete harfbuzz's shaping algorithm port to Rust
MIT License
498 stars 34 forks source link

Use ragel to generate machine code #77

Closed notgull closed 10 months ago

notgull commented 10 months ago

I managed to get the latest master for ragel up and running and got it to generate code... although it looks like tests are failing? Not sure why that's happening.

RazrFalcon commented 10 months ago

Nice! Can you run rustfmt on those files (not the whole rustybuzz).

RazrFalcon commented 10 months ago

As for failing tests, which ragel files did you used? The current master one or 2.7.1 one?

RazrFalcon commented 10 months ago

I guess you did used the master one, because the 2.7.1 universal is slightly different.

notgull commented 10 months ago

I guess you did used the master one, because the 2.7.1 universal is slightly different.

I've synced with 2.7.1, although tests are still failing, specifically for the Indic tests.

RazrFalcon commented 10 months ago

Hmm... no idea what could be wrong. Could be a ragel bug. The suspicious part is that state machine tables are very different compared to what they were. Either newer ragel produces different output or something else.

By the way, what ragel flags have you used? harfbuzz uses -e -F1

I guess the only option is to try to use master ragel on harfbuzz 2.7.1 to make sure it doesn't break harfbuzz tests as well.

notgull commented 10 months ago

I finally figured out how to build harfbuzz. Even after I used master ragel to regenerate the state machines the tests still passed. What?

At this point the only theories I have is that ragel-rust doesn't properly generate state machines (likely) or the tests are set up improperly (less likely). Even after generating state machines with -e -F1 the tests still failed (even though it shouldn't make any change at all, theoretically?). I'll need to investigate this further over the weekend...

RazrFalcon commented 10 months ago

I finally figured out how to build harfbuzz.

meson builddir
ninja -Cbuilddir

Don't remember how to run tests.

Even after I used master ragel to regenerate the state machines the tests still passed.

Did the output changed or is the same? Does tables have the same values in C and Rust output? It can easily be a ragel bug. Rust support is alpha, afaik.

behdad commented 10 months ago

Don't remember how to run tests.

ninja -Cbuilddir test

notgull commented 10 months ago

Strangely, it looks like all of the issues come from indic_machine.rs. After I revert my changes to that file, all of the tests pass. So I guess I'll take a run at that particular file in another PR.

Bobo1239 commented 10 months ago

@notgull There was a simple typo in indic_machine.rl:

macro_rules! found_syllable {
    ($kind:expr) => {{
-        found_syllable(ts, ts, &mut syllable_serial, $kind, buffer)
+        found_syllable(ts, te, &mut syllable_serial, $kind, buffer)
    }}
}

The tests pass with this.

notgull commented 10 months ago

notgull code after spending five minutes in production a CI environment

robot fallx299

lmao fixed

RazrFalcon commented 10 months ago

@Bobo1239 great catch!

RazrFalcon commented 10 months ago

@notgull Thanks! Since it fixes the out-of-bounds bug I will make a release soon.

I also want to write a simple readme of how to use ragel. What exact ragel command did you used to generate Rust files?