luser-dr00g / xpost

A PostScript interpreter in C
Other
93 stars 12 forks source link

-Wall has a new nuisance warning #51

Open luser-dr00g opened 2 years ago

luser-dr00g commented 2 years ago

I think we should add -Wno-cast-function-type to the CFLAGS but I forget where to do it.

I looked at configure.ac and Makefile.am and various Makefile.mk files but I didn't see other warning flags.

vtorri commented 2 years ago

what is Xpost_Op_Func has var args ?

vtorri commented 2 years ago

like

typedef int (Xpost_Op_Func)(Xpost_Context ctx,...);

it seems to compile without warnings but the devices must be tested

vtorri commented 2 years ago

ha no, error :

src/lib/xpost_operator.c: In function 'xpost_operator_cons':
src/lib/xpost_operator.c:483:23: error: assignment to 'Xpost_Op_Func' {aka 'int (*)(struct _Xpost_Context *, ...)'} from incompatible pointer type 'int (*)(Xpost_Context *)' {aka 'int (*)(struct _Xpost_Context *)'} [-Wincompatible-pointer-types]
  483 |             sp[si].fp = (int(*)(Xpost_Context *))fp;
      |                       ^
vtorri commented 2 years ago

and with

            sp[si].fp = (int(*)(Xpost_Context *, ...))fp;

it seems good

vtorri commented 2 years ago

i let you test and commit ?

luser-dr00g commented 2 years ago

Hmm. I don't know. If we're changing the code instead of the warning flag, then I'm inclined to change the stored type to just int(*)() with no args hence no prototype. It's always casted to the correct type before calling. But I think going back to no prototype will be less prone to tripping future "helpful warnings" from gcc.

vtorri commented 2 years ago

It's up to you :-)

luser-dr00g commented 2 years ago

Changing to unprototyped produces a different warning:

  CC       src/lib/src_lib_libxpost_la-xpost_dev_bgr.lo
In file included from src/lib/xpost_dev_bgr.c:52:
src/lib/xpost_operator.h:68:1: warning: function declaration isn’t a prototype [-Wstrict-prototypes]
   68 | typedef int (*Xpost_Op_Func)();
      | ^~~~~~~

But this happens only once per file instead of every operator function.

vtorri commented 2 years ago

do you consider that xpost should be c89 compliant, or c99 ?

vtorri commented 2 years ago

hmm, c99 according to configure.ac

luser-dr00g commented 2 years ago

Hmm. I forget! :) At one point we were trying to stay at c89. That's where autotools really gave a big help in getting alloca() to work portably.

I guess we're going for c99 except for VLAs, so it can still compile with MVSC.

vtorri commented 2 years ago

alloca check has been simplified : https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.70/autoconf.html#Particular-Functions

do you want me to simplify this in xpost code ?

vtorri commented 2 years ago

alloca simplification is done

vtorri commented 2 years ago

@luser-dr00g so, what about

typedef int (*Xpost_Op_Func)(Xpost_Context *ctx,...);

? no warning, and we keep the warning flag

luser-dr00g commented 2 years ago

Excellent.

luser-dr00g commented 2 years ago

Hmm. I don't know about specifying a varargs function. That feel more like lying to the compiler. With the unprototyped function it's more like withholding information from the compiler because it would only freak out about it. Calling it a varargs functions seems like we're making an implementation assumption which may not be guaranteed by the standard.

vtorri commented 2 years ago

maybe that is something that we should ask to gcc devs

vtorri commented 2 years ago

according to guy in #c (irc chan), cast to int (*)() or int (*)(Xpost_Context *ctx,...) is correct