search

BNFC / bnfc

BNF Converter
http://bnfc.digitalgrammars.com/
586 stars 165 forks source link

get rid of the deprecated string.set function in OCaml backend #139

Closed ghost closed 7 years ago

ghost commented 9 years ago

Replace it with bytes.set.

Refer to http://caml.inria.fr/pub/docs/manual-ocaml/libref/String.html

gdetrez commented 9 years ago

Thanks for the pull request (and sorry for the delay). I just tried this and I get an error when running make, can you take a look?

❯❯❯ bnfc --ocaml -m Calc.cf

8 rules accepted

writing new file ./AbsCalc.ml
writing new file ./LexCalc.mll
writing new file ./ParCalc.mly
writing new file ./SkelCalc.ml
writing new file ./PrintCalc.ml
writing new file ./ShowCalc.ml
writing new file ./TestCalc.ml
writing new file ./BNFC_Util.ml
writing new file ./Makefile
❯❯❯ make
ocamlyacc ParCalc.mly
ocamllex LexCalc.mll
19 states, 558 transitions, table size 2346 bytes
ocamlc -o TestCalc BNFC_Util.ml AbsCalc.ml SkelCalc.ml ShowCalc.ml PrintCalc.ml ParCalc.mli ParCalc.ml LexCalc.ml TestCalc.ml
File "PrintCalc.ml", line 24, characters 4-13:
Error: Unbound module Bytes
make: *** [all] Error 2
ghost commented 9 years ago

Can you check your OCaml version? Mine:

$ ocaml -version
The OCaml toplevel, version 4.02.1
gdetrez commented 9 years ago

I see, apparently the Bytes package is new in 4.02. (My version is 4.00.1)

Since this breaks compatibility with older versions of ocaml maybe there should be a command line switch to enable/disable it (maybe something like --safe-string/--no-safe-string).

I took a quick look at the ocaml versions available in different distributions and it seems that fedora just added 4.02 in the latest release while debian doesn't even have it in testing, so it's probably a bit early to make it the default.

As far as I understood, the string module is not really deprecated but should only be used for immutable strings. An other solution would be to only use immutable string, that way we should still be backward compatible while avoiding deprecated function, isn't it?

ghost commented 9 years ago

Yep, let me try to work on a solution that only use immutable strings then. Let's keep this PR open, I will reuse it for my new changes.

gdetrez commented 9 years ago

Cool, thanks!

gdetrez commented 8 years ago

@dorafmon: any progresses? are you still working on this or planning to?

gdetrez commented 7 years ago

This is fixed by 0f38dac but using Buffer instead of Bytes which seems more backward compatible.