xavierleroy / camlidl

Stub code generator for OCaml/C interface
Other
34 stars 8 forks source link

plplot buildfailure with transition from camlidl 1.05 -> 1.08 #18

Closed opoplawski closed 1 year ago

opoplawski commented 4 years ago

With the shift from ocaml-camlidl 1.05 -> 1.08 in Fedora rawhide we are getting a build failure in plplot:


[  4%] Generating plplot_core.idl, plplot_core.mli, plplot_core.ml, plplot_core_stubs.c, plplot_core.h
cd /builddir/build/BUILD/plplot-5.15.0/fedora/bindings/ocaml && /usr/bin/cmake -E copy /builddir/build/BUILD/plplot-5.15.0/bindings/ocaml/plplot_core.idl /builddir/build/BUILD/plplot-5.15.0/fedora/bindings/ocaml/plplot_core.idl
cd /builddir/build/BUILD/plplot-5.15.0/fedora/bindings/ocaml && /usr/bin/camlidl -I /builddir/build/BUILD/plplot-5.15.0/bindings/ocaml -header plplot_core.idl
File plplot_core.idl, line 369, column 13: Illegal character #

This line is:

RAW_ML(external plgriddata : float array -> float array -> float array -> float array -> float array -> plplot_grid_method_type -> float -> float array array = "ml_plgriddata_bytecode" "ml_plgriddata")

The macros involved are:

#define QUOTEME(x) #x
#define RAW_ML(x) quote(mlmli, QUOTEME(x));

Now, I know nothing about ocaml and camlidl so I'm hoping someone can clue us in on what has changed.

rwmjones commented 4 years ago

I am able to reproduce this easily:

$ camlidl -I . -header plplot_core.idl 
File plplot_core.idl, line 369, column 13: Illegal character #

However I don't understand how it can happen because when I pass the file through cpp myself there is no # character in any column except 1 (for line numbering).

rwmjones commented 4 years ago

OK I see why this happens. In camlidl the default definition for cpp is:

CPP=cpp -traditional

We don't change this in Fedora. Using the -traditional flag causes the macro expansion to happen differently.

Without -traditional:

quote(mlmli, "external plgriddata : float array -> float array -> float array -> float array -> float array -> plplot_grid_method_type -> float -> float array array = \"ml_plgriddata_bytecode\" \"ml_plgriddata\"");

With -traditional:

quote(mlmli, #external plgriddata : float array -> float array -> float array -> float array -> float array -> plplot_grid_method_type -> float -> float array array = "ml_plgriddata_bytecode" "ml_plgriddata");

We could easily change the Fedora build to remove this flag. However I presume the flag is added for a reason? If we leave the flag in, then plplot would have to change instead. Also I don't want to be different from what Debian & opam are doing as that will introduce unnecessary incompatibility that everyone will need to work around.

xavierleroy commented 4 years ago

Right, the change of behavior is due to this commit: 13f05c0dcdd58f82be8401437109b1cbe0d0a26a.

I was nervous that running MIDL code (which is not C code) through a standard-conformant C preprocessor could cause errors, so I suggested the -traditional flag that makes the GNU C preprocessor much more lenient. I was not expecting that it would break legitimate use of C macros...

Should i just revert the change? Advice welcome.

rwmjones commented 4 years ago

The only examples of MSFT idl files I could easily find are from: https://docs.microsoft.com/en-us/windows/win32/com/anatomy-of-an-idl-file The second example uses // comments. These are not removed when using -traditional but they are removed when using just cpp.

That would appear to be an argument to revert the change.

xavierleroy commented 4 years ago

These are compelling arguments. Reverted in commit e7593d0. Will make a new release ASAP.

opoplawski commented 4 years ago

@xavierleroy @rwmjones thanks for handling this so quickly.

xavierleroy commented 1 year ago

This was fixed in release 1.09 in 2020, closing this issue.