search

frenetic-lang / frenetic

The Frenetic Programming Language and Runtime System
http://www.frenetic-lang.org/
Other
223 stars 51 forks source link

type bytes in Frenetic_Packet.mli causes confusion #431

Closed arjunguha closed 9 years ago

arjunguha commented 9 years ago

OCaml 4.02 introduced a new type called bytes. Core.Std exposes it too:

https://ocaml.org/releases/4.02.html

craig-riecke commented 9 years ago

Is this still causing compilation problems? If so, there's a fix in Issue #425.

arjunguha commented 9 years ago

That's part of the fix. I think we shouldn't create our own type called bytes either. Perhaps we could rename our type bytes = to type cstruct?

craig-riecke commented 9 years ago

Oh yeah ... that is definitely confusing. I can take care of this if you don't mind.

craig-riecke commented 9 years ago

Blurrgh. Turns out you can't use 

type cstruct = ...

Because the camlp4 syntax extension for CStruct sees cstruct as a reserved word and tries to expand it. You got around that for your changes in ppx_pretty branch because you turned off camlp4 for files there.

I could change it to "cstruct_bytes". That's slightly less confusing than "bytes", but more verbose.

arjunguha commented 9 years ago

Why not cbtyes?

:)

On 08/03/2015 01:56 PM, Craig Riecke wrote:

Blurrgh. Turns out you can't use

type cstruct = ...

Because the camlp4 syntax extension for CStruct sees cstruct as a reserved word and tries to expand it. You got around that for your changes in ppx_pretty branch because you turned off camlp4 for files there.

I could change it to "cstruct_bytes". That's slightly less confusing than "bytes", but more verbose.

— Reply to this email directly or view it on GitHub https://github.com/frenetic-lang/frenetic/issues/431#issuecomment-127350313.

craig-riecke commented 9 years ago

cbytes works for me!

arjunguha commented 9 years ago

not serious!!

On 08/03/2015 02:00 PM, Craig Riecke wrote:

cbytes works for me!

— Reply to this email directly or view it on GitHub https://github.com/frenetic-lang/frenetic/issues/431#issuecomment-127351947.

craig-riecke commented 9 years ago

:-) All righty. We could also just forgo using a type alias and use Cstruct.t everywhere. Or just leave it as bytes for now and wait for Cstruct to get converted to ppx. That sounds imminent anyway...

craig-riecke commented 9 years ago

I'd like to get this issue resolved. Turns out leaving it as bytes is causing pain. Pinning Core to 111.24 has caused fresh installs to sputter and require manual intervention.

My pref would be to forgo the type alias and just use Cstruct.t. But a better type alias name would be fine as well.

arjunguha commented 9 years ago

yeah agreed

craig-riecke commented 9 years ago

Fixed in pull request #439.