|
|
Descriptionhttps://sourceforge.net/p/testlilyissues/issues/5400/
This suppresses warnings that g++ 8 issues about casts that the Guile interface requires.
Patch Set 1 #Patch Set 2 : Compile with -Wno-cast-function-type #
Total comments: 1
Patch Set 3 : Explain #MessagesTotal messages: 10
I'd use scm_func_cast<scm_t_blabla> (actual_func) in order to mimic what reinterpret_cast does, but the implementation looks awfully heavy-handed and GCC-8 specific. Any chance that an implementation via a double cast (with something like void * in between) will work? Strictly speaking that's worse than casting from one function pointer to another since function and data pointers need not be the same size on segmented architectures, but lots of code does stuff like that anyway. Or we just disable that specific warning completely: casting from one function pointer to another is "I mean it" territory. It's part of the Guile API and we cannot really expect people to circumvent it every time.
Sign in to reply to this message.
On 2018/08/13 07:11:38, dak wrote: > I'd use scm_func_cast<scm_t_blabla> (actual_func) I'll do that. > Any chance that an implementation via a double cast (with something like void * > in between) will work? Strictly speaking that's worse than casting from one > function pointer to another since function and data pointers need not be the > same size on segmented architectures, but lots of code does stuff like that > anyway. The first thing I tried was an intermediate (void*), and that did eliminate the warning. The reason you cite is the reason I didn't propose it, but if you're comfortable with it, I'll do it. Interaction with Guile is not a place where I care to assert my preferences.
Sign in to reply to this message.
On 2018/08/13 12:29:46, Dan Eble wrote: > On 2018/08/13 07:11:38, dak wrote: > > I'd use scm_func_cast<scm_t_blabla> (actual_func) > > I'll do that. > > > Any chance that an implementation via a double cast (with something like void > * > > in between) will work? Strictly speaking that's worse than casting from one > > function pointer to another since function and data pointers need not be the > > same size on segmented architectures, but lots of code does stuff like that > > anyway. > > The first thing I tried was an intermediate (void*), and that did eliminate the > warning. The reason you cite is the reason I didn't propose it, but if you're > comfortable with it, I'll do it. Interaction with Guile is not a place where I > care to assert my preferences. Personally, I think that we should rather disable this warning globally. An explicit cast is an explicit cast. There would be a weak argument that a C-style cast may expect an actual working conversion to take place (and I have little enough sympathy with that argument), but an explicit reinterpret_cast quite clearly states "shut up, I know what I am doing". So I don't really think that it's warranted to tap-dance around GCC here and I'd rather stop GCC on the command line from flagging this. Apropos: what have you done about the warnings regarding SCM_UNPACK ? Those use something like (0 ? *(SCM*)0 : ...) for type coercion/checking and the resulting warnings clutter everything for me. I'll likely change my local Guile-1.8 source for quieting those.
Sign in to reply to this message.
On 2018/08/13 12:55:39, dak wrote: > Apropos: what have you done about the warnings regarding SCM_UNPACK ? Those use > something like (0 ? *(SCM*)0 : ...) for type coercion/checking and the resulting > warnings clutter everything for me. I'll likely change my local Guile-1.8 > source for quieting those. There are a few other kinds of warnings that I see a lot of, and which I haven't looked into yet, but I don't see any warnings that seem to be related to SCM_UNPACK. The specific version of g++ that I am using is g++ (Ubuntu 8-20180414-1ubuntu2) 8.0.1 20180414 (experimental) [trunk revision 259383]
Sign in to reply to this message.
Compile with -Wno-cast-function-type
Sign in to reply to this message.
On 2018/08/14 02:12:59, Dan Eble wrote: > Compile with -Wno-cast-function-type Sorry for noticing this late in the game, but one thing worth noting here is that in Guile 1.8 the comments would appear to indicate that we are defining scm_t_subr ourselves in lily/include/lily-guile-macros.hh as /* For backward compatability with Guile 1.8 */ #if !HAVE_GUILE_SUBR_TYPE typedef SCM (*scm_t_subr) (GUILE_ELLIPSIS); #endif While I have my doubts that we could provide a definition that would work better with regard to the warning (there are prototypes like libguile/gsubr.c:scm_c_define_gsubr (const char *name, int req, int opt, int rst, SCM (*fcn)()) so indeed scm_t_subr appears to be defined in line with what's used in the headers and it's the cast that is the problem), that seems worth pointing out. Possibly typedef void* scm_t_subr; could perversely work. I think I prefer disabling the warning. So this is more for completeness' sake than anything else.
Sign in to reply to this message.
https://codereview.appspot.com/357770043/diff/20001/configure.ac File configure.ac (right): https://codereview.appspot.com/357770043/diff/20001/configure.ac#newcode220 configure.ac:220: CXXFLAGS=-Wcast-function-type This is more a sort of logic nitpick than of actual relevance given how GCC is wired, but if we want to be using -Wno-cast-function-type , that's what we should announce and check for rather than -Wcast-function-type .
Sign in to reply to this message.
On 2018/08/15 16:43:27, dak wrote: > This is more a sort of logic nitpick than of actual relevance given how GCC is > wired, but if we want to be using -Wno-cast-function-type , that's what we > should announce and check for rather than -Wcast-function-type . I couldn't figure out any other way to check for -Wno-cast-function-type reliably. Attempts involving -Werror were defeated by the fact that some preprocessor variable is redefined somewhere, leading to an error in all cases. After spending much time to figure that much out, I was not interested in spending more to figure out how to resolve it (if that is even possible). I could reword the message, something like "whether the compiler might warn about casting functions to incompatible types." Would that be better?
Sign in to reply to this message.
On 2018/08/15 21:51:46, Dan Eble wrote: > On 2018/08/15 16:43:27, dak wrote: > > This is more a sort of logic nitpick than of actual relevance given how GCC is > > wired, but if we want to be using -Wno-cast-function-type , that's what we > > should announce and check for rather than -Wcast-function-type . > > I couldn't figure out any other way to check for -Wno-cast-function-type > reliably. Attempts involving -Werror were defeated by the fact that some > preprocessor variable is redefined somewhere, leading to an error in all cases. > After spending much time to figure that much out, I was not interested in > spending more to figure out how to resolve it (if that is even possible). > > I could reword the message, something like "whether the compiler might warn > about casting functions to incompatible types." Would that be better? I was "Huh?" until I tried out a few things. g++ -Wno-blubber produces no diagnostic while g++ -Wblubber bombs out. Just add a comment like # g++ -Wno-whatever compiles so we need to check for the positive option because otherwise nobody will be able to guess what is up here. In general, what looks like a mistake or oversight and/or what just became obvious through testing warrants a comment. Don't keep things secret between you and the compiler.
Sign in to reply to this message.
|