Discussion:
Questions about GLib version
Matthew Brush
2013-08-18 00:40:32 UTC
Permalink
Hi Neil,

Is there a minimum version of Glib that Scintilla supports? I mean if
one wanted to use something from say v2.30, would one have to guard out
the code using version checking macros? What if it meant not compiling a
file on certain version?

The reason I ask is that currently Scintilla has undefined behaviour in
this (generated) file here:

https://sourceforge.net/p/scintilla/code/ci/default/tree/gtk/scintilla-marshal.c#l78

And Geany also has same UB (elsewhere) due to using marshaller like this
(casting void* to function ptr). I would like to fix this (in both
projects, if you'd like a patch), but the change here:

https://git.gnome.org/browse/glib/commit/?id=88ab35f3cb6127036361e421987a127bddb989c8

Which, if I understand correctly, allows setting the c_marshaller
function to NULL in g_signal_new*() and uses libffi instead to deal with
this, is not available until GLib v2.30.

Please advise, or say whether you'd even want such a change. The UB is
really quite pedantic and is actually defined behaviour on POSIX
platforms AFAICT (see dlsym() docs), just not ISO C (and so causes
compiler warnings with strict/pedantic enough flags).

As data points, my Ubuntu 13.04 computer has GLib v2.36 and I think
Geany supports as far back as v2.20.

Cheers,
Matthew Brush
--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/groups/opt_out.
Neil Hodgson
2013-08-18 01:22:49 UTC
Permalink
Post by Matthew Brush
Is there a minimum version of Glib that Scintilla supports?
The minimum GTK+ version is 2.8 and I think glib version numbers correspond so glib 2.8. Long-term Linux distributions are quite common.

You could try to get agreement for moving the minimum version forward but there's no big simplification unless 2.x is dropped and that won't be viable for years.
Post by Matthew Brush
I mean if one wanted to use something from say v2.30, would one have to guard out the code using version checking macros?
Yes. There are already many version guards in the GTK+ platform layer.
Post by Matthew Brush
What if it meant not compiling a file on certain version?
Please don't complexify the make file. If possible, use the preprocessor within the existing files.

Neil
--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/groups/opt_out.
Matthew Brush
2013-08-18 04:28:33 UTC
Permalink
Post by Neil Hodgson
Post by Matthew Brush
Is there a minimum version of Glib that Scintilla supports?
The minimum GTK+ version is 2.8 and I think glib version numbers
correspond so glib 2.8. Long-term Linux distributions are quite
common.
FWIW, it's not 1:1,
https://git.gnome.org/browse/gtk+/tree/configure.in?id=GTK_2_8_0#n34
Post by Neil Hodgson
You could try to get agreement for moving the minimum version forward
but there's no big simplification unless 2.x is dropped and that
won't be viable for years.
Nah, I bikeshed this issue enough in Geany :) (gtk2.16)
Post by Neil Hodgson
Post by Matthew Brush
I mean if one wanted to use something from say v2.30, would one
have to guard out the code using version checking macros?
Yes. There are already many version guards in the GTK+ platform layer.
Post by Matthew Brush
What if it meant not compiling a file on certain version?
Please don't complexify the make file. If possible, use the
preprocessor within the existing files.
The difference is compiling-in an unused translation unit (no idea
whether that matters), vs complexifying the makefile. If that's OK, then
there shouldn't be a need to touch the makefile.

Thanks for quick response.

Cheers,
Matthew Brush
--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/groups/opt_out.
Matthew Brush
2013-08-18 04:35:14 UTC
Permalink
Post by Matthew Brush
[...]
The difference is compiling-in an unused translation unit (no idea
whether that matters), vs complexifying the makefile. If that's OK, then
there shouldn't be a need to touch the makefile.
I should mention, compiling in the TU anyway will probably keep the ISO
warning about UB, thus defeating the purpose (for me).

Cheers,
Matthew Brush
--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/groups/opt_out.
Neil Hodgson
2013-08-21 14:09:06 UTC
Permalink
I should mention, compiling in the TU anyway will probably keep the ISO warning about UB, thus defeating the purpose (for me).
If its inactive because of the preprocessor then you won't see compiler warnings.

C++11 allows (5.2.10.8) reinterpret_cast from void* to function pointer as 'conditionally supported' instead of undefined behaviour. Older compilers are happier with using a union as done for lexers in shared libraries for Qt platform.

Neil
--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/groups/opt_out.
Matthew Brush
2013-08-21 21:48:50 UTC
Permalink
Post by Neil Hodgson
Post by Matthew Brush
I should mention, compiling in the TU anyway will probably keep the
ISO warning about UB, thus defeating the purpose (for me).
If its inactive because of the preprocessor then you won't see
compiler warnings.
True, the whole file could in theory be included into another new file
with guards around the include to prevent inclusion under certain
versions, otherwise we would have to modify the file generated by
glib-genmarshal after it's generated. Not sure it matters since I doubt
it's re-generated often (if ever), but any changes would have to
be-reapplied if it was and would still require touching the makefile.
Post by Neil Hodgson
C++11 allows (5.2.10.8) reinterpret_cast from void* to function
pointer as 'conditionally supported' instead of undefined behaviour.
Neat, sadly in both the Scintilla makefile and Geany's autotools build
system, this file is compiled using a C compiler (although it probably
would compile fine in a C++ compiler with `extern "C"` around it).
Post by Neil Hodgson
Older compilers are happier with using a union as done for lexers in
shared libraries for Qt platform.
Heh, it's funny because I actually considered mentioning using the union
hack but figured you wouldn't accept it since not only is it a dirty
hack and touches generated code, but it only tricks the compiler into
not being able to warn, I don't think it actually gets rid of the
"undefined behaviour".

Anyway it's not a big deal, the sky won't fall will this well-defined
undefined behaviour that's depended upon in GTK+ itself in a lot of
places and if it ends up bugging us in Geany, we can just deal with it
locally in our build system.

Thanks for the advice/info.

Cheers,
Matthew Brush
--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/groups/opt_out.
Loading...