namespace problem

[st...@sjssoftware.com]

Have a look below.  It appears that something in fltk has grabbed the symbol 'Status' for itself.
If this has been fixed since version 1.3.1, great.
Best regards,
Stan
 
stan@vernon:~/hugh$ fltk-config --version
1.3.1

stan@vernon:~/hugh$ g++ --version
g++ (Ubuntu 4.8.4-2ubuntu1~14.04.1) 4.8.4

file junk.C:

// This program will not compile
// Commenting out this line will fix the problem
#include "/usr/include/FL/Fl.H"

namespace MineAllMine {

class Thing {
public:
    // Replacing this line with the following one will also fix it
    enum Status { Good, Bad };
    //enum MyStatus { Good, Bad };
};

} // namespace

int main()
{
    return 0;
}

stan@vernon:~/hugh$ g++ junk.C
In file included from /usr/include/FL/Xutf8.h:25:0,
                 from /usr/include/FL/fl_utf8.h:64,
                 from /usr/include/FL/Fl.H:30,
                 from junk.C:4:
junk.C:11:10: error: expected identifier before ‘int’
     enum Status { Good, Bad };
          ^
junk.C:11:17: error: expected unqualified-id before ‘{’ token
     enum Status { Good, Bad };

[Albrecht Schlosser]

Hi Stan,

On 28.02.2016 14:45 stan@... wrote:
> Have a look below. It appears that something in fltk has grabbed the
> symbol 'Status' for itself.
> If this has been fixed since version 1.3.1, great.

No, unfortunately it hasn't been "fixed", but it's not (directly) FLTK's
fault.

Status is typedef'd as int somewhere in the Xlib headers. :-(

You can say it's FLTK's fault to expose the Xlib headers if you #include
FLTK headers, but currently (in FLTK 1.3.x) this can likely not be
changed. We also expose the definitions of "Window" and all such X11
stuff (see below [1] for more...)

I'm confident (although not 100% sure) that we can change this in FLTK
1.4.0, because we can better separate platform dependent code from user
space, i.e. public header files. Note that Windows.h defines such things
like "grp", "grp1", etc. if you #include it. (!) :-(

Here is the "proof":

$ cat status.cxx
// #include <FL/x.H>
#include <X11/Xlib.h>

int main (int argc, char **argv) {
Status x = "abc";
}

$ gcc -c status.cxx
status.cxx: In function ‘int main(int, char**)’:
status.cxx:5:13: error: invalid conversion from ‘const char*’ to ‘int’
[-fpermissive]
Status x = "abc";
^
--- end of "proof" ---

I intentionally used the error message to let the compiler display the
"real" type.

----- looking at the code, testing, ... -----

[1] ... after some tests:

Meanwhile I found the culprit, but I don't know if this can/should be
"fixed" in FLTK 1.3.4. IMHO we should not expose (#include) system
specific stuff (headers) in public header files, [maybe] unless "requested".

Currently we #include X11 (and other platform specific headers) at least
in two places in the public headers (X11 definitely only theses two):

(1) FL/x.H
(2) FL/fl_utf8.h

The former (x.H) is the one that I meant when I wrote "unless
requested". You should be aware that you get system stuff when you
#include it. OTOH (currently) we #include FL/x.H in several other
headers as well. Sigh.

The latter (fl_utf8.h) is #include'd in FL/Fl.H which seems to cause the
issue you're seeing. It should supposedly not #include the X11 headers
if #include'd from user code. Here is a quick fix:

--- patch -p1 ... ---
diff --git a/FL/fl_utf8.h b/FL/fl_utf8.h
index 1551991..0ce1f49 100644
--- a/FL/fl_utf8.h
+++ b/FL/fl_utf8.h
@@ -60,8 +60,10 @@
#else /* X11 */
# include <sys/types.h>
# include <sys/stat.h>
+#ifdef FL_LIBRARY
# include <X11/Xlocale.h>
# include <X11/Xlib.h>
+#endif /* FL_LIBRARY */
# include <locale.h>
# define xchar unsigned short
#endif
--- end of patch ---

Does this work for you?

Note: this is a quick shot, and the library compiles and links well with
it (since the library code doesn't change anyway), but this may break
other users' code - although this is not very likely and supposedly easy
to fix by #including the related platform specific header files. I don't
know if we should fix the headers in this way short before a release.

[Edzard Egberts]

Albrecht Schlosser wrote:

> #include <X11/Xlib.h>

#undef Status

> int main (int argc, char **argv) {
> Status x = "abc";
> }

../main.cpp:7: error: »Status« was not defined

> Here is a quick fix:
>
> --- patch -p1 ... ---
> diff --git a/FL/fl_utf8.h b/FL/fl_utf8.h
> index 1551991..0ce1f49 100644
> --- a/FL/fl_utf8.h
> +++ b/FL/fl_utf8.h
> @@ -60,8 +60,10 @@
> #else /* X11 */
> # include <sys/types.h>
> # include <sys/stat.h>
> +#ifdef FL_LIBRARY
> # include <X11/Xlocale.h>
> # include <X11/Xlib.h>
> +#endif /* FL_LIBRARY */
> # include <locale.h>
> # define xchar unsigned short
> #endif
> --- end of patch ---

Does this fix also make usage of "Status" possible, or did I miss the
problem at all? First I also thought it regards to namespaces, but the
problem comes from a C definition.

[Albrecht Schlosser]

On 29.02.2016 08:46 schrieb Edzard Egberts wrote:
> Albrecht Schlosser wrote:
>
>> #include <X11/Xlib.h>
>
> #undef Status

#undef Status is not possible because Status is a typedef introduced by

#include <X11/Xlib.h>

Maybe I should have been more explicit.

>> int main (int argc, char **argv) {
>> Status x = "abc";
>> }
>
> ../main.cpp:7: error: »Status« was not defined

Is this from your code, i.e. from '#undef Status'?

>> Here is a quick fix:
>>
>> --- patch -p1 ... ---
>> diff --git a/FL/fl_utf8.h b/FL/fl_utf8.h
>> index 1551991..0ce1f49 100644
>> --- a/FL/fl_utf8.h
>> +++ b/FL/fl_utf8.h
>> @@ -60,8 +60,10 @@
>> #else /* X11 */
>> # include <sys/types.h>
>> # include <sys/stat.h>
>> +#ifdef FL_LIBRARY
>> # include <X11/Xlocale.h>
>> # include <X11/Xlib.h>
>> +#endif /* FL_LIBRARY */
>> # include <locale.h>
>> # define xchar unsigned short
>> #endif
>> --- end of patch ---
>
> Does this fix also make usage of "Status" possible, or did I miss the
> problem at all? First I also thought it regards to namespaces, but the
> problem comes from a C definition.

Yes (usage of Status will be possible), and yes (it comes from a C
definition (typedef)).

The patch avoids including X11 headers in user code (the macro
FL_LIBRARY must only be defined when the library is built). Hence X11
specific definitions (Window, Status, ...) will not pollute your user
code, and you will be able to use it.

My test ("proof") shows when compiled with gcc:

(1) if #include <X11/Xlib.h> is active, you see:

status.cxx:5:13: error: invalid conversion from ‘const char*’ to ‘int’
[-fpermissive]
Status x = "abc";
^

This means that Status is defined (typedef int Status) so the conversion
from ‘const char*’ to ‘int’ is invalid.


(2) If you don't #include the X11 header, then "Status x" is not
defined, and you'll get another error message.


The conclusion: if we avoid to #include <X11/*> in the FLTK header files
when compiled for user code, then Status will not be defined and you can
use it freely.


A usable patch for FLTK will be more involved though. There are other
platform includes in the same and other files, and we must avoid to
#include all platform specific headers in user code. Some users might
have "relied" on the inclusion of - for instance - <Windows.h> (under
Windows) or <X11/Xlib.h> (under Linux), and this change would have a
(very unlikely?) side effect to break the code of these users. But this
is easy to fix, so I tend to change the FLTK headers, as suggested by
the posted patch.

Could you please open an STR that shows the issue with your code (or my
short test program below) and refer to this thread? Google link:
https://groups.google.com/forum/#!topic/fltkgeneral/gzmdRk2LvAk

Please use HIGH priority for the STR.


Here is "my" test program:

$ cat status.cxx
#include <FL/Fl.H>

int main (int argc, char **argv) {

enum Status { Good, Bad };
return Good;
}

$ fltk-config --compile status.cxx && ./status ; echo $?
g++ -I/home/albrecht/git/fltk -I/usr/include/freetype2
-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_THREAD_SAFE -D_REENTRANT -o
'status' 'status.cxx' /home/albrecht/git/fltk/lib/libfltk.a -lXrender
-lXcursor -lXfixes -lXext -lXft -lfontconfig -lXinerama -lpthread -ldl
-lm -lX11

status.cxx: In function ‘int main(int, char**)’:

status.cxx:5:3: error: expected primary-expression before ‘enum’

enum Status { Good, Bad };
^

status.cxx:5:3: error: expected ‘;’ before ‘enum’
status.cxx:6:10: error: ‘Good’ was not declared in this scope
return Good;
^
1

--- end of test ---

With the patch it works as expected:

$ fltk-config --compile status.cxx && ./status ; echo $?
g++ -I/home/albrecht/git/fltk -I/usr/include/freetype2
-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_THREAD_SAFE -D_REENTRANT -o
'status' 'status.cxx' /home/albrecht/git/fltk/lib/libfltk.a -lXrender
-lXcursor -lXfixes -lXext -lXft -lfontconfig -lXinerama -lpthread -ldl
-lm -lX11
0

The same is true if you remove the inclusion of <FL/Fl.H>, and it also
works flawlessly under Windows.

[Edzard Egberts]

Albrecht Schlosser wrote:
> On 29.02.2016 08:46 schrieb Edzard Egberts wrote:
>> Albrecht Schlosser wrote:
>>
>>> #include <X11/Xlib.h>
>>
>> #undef Status
>
> #undef Status is not possible because Status is a typedef introduced by
>
> #include <X11/Xlib.h>

This seems to depend, on my system (CentOS 6.7) in "Xlib.h" the
definition is:

#define Status int

But now I understand, that you are in fact resolving another problem. ;o)

[Albrecht Schlosser]

On 29.02.2016 14:34 Edzard Egberts wrote:
> Albrecht Schlosser wrote:
>> On 29.02.2016 08:46 schrieb Edzard Egberts wrote:
>>> Albrecht Schlosser wrote:
>>>
>>>> #include <X11/Xlib.h>
>>>
>>> #undef Status
>>
>> #undef Status is not possible because Status is a typedef introduced by
>>
>> #include <X11/Xlib.h>
>
> This seems to depend, on my system (CentOS 6.7) in "Xlib.h" the
> definition is:
>
> #define Status int

Ah, well, I didn't realize that you were not the OP, hence my partial
confusion regarding your code and error messages.

I didn't mean to ask you (Edzard) to open an STR, but meant to ask the
OP to do it. But if this concerns you as well, feel free to do it. ;)

> But now I understand, that you are in fact resolving another problem. ;o)

Yep, it's a more general issue. IMHO we must not #include platform
specific header files in user code, unless really required.

However I don't know if it is required in any case right now, but maybe
we need it in <FL/x.H> because this IS the platform specific #include
file, and maybe(!) the platform headers must be #include'd because
something is documented in the "OS issues" chapter. That is the
reasoning why I don't go ahead and change the code - we need to
investigate and do the right thing.

[Albrecht Schlosser]

On 29.02.2016 14:07 Albrecht Schlosser wrote:

>>> Here is a quick fix:
>>>
>>> --- patch -p1 ... ---
>>> diff --git a/FL/fl_utf8.h b/FL/fl_utf8.h
>>> index 1551991..0ce1f49 100644
>>> --- a/FL/fl_utf8.h
>>> +++ b/FL/fl_utf8.h
>>> @@ -60,8 +60,10 @@
>>> #else /* X11 */
>>> # include <sys/types.h>
>>> # include <sys/stat.h>
>>> +#ifdef FL_LIBRARY
>>> # include <X11/Xlocale.h>
>>> # include <X11/Xlib.h>
>>> +#endif /* FL_LIBRARY */
>>> # include <locale.h>
>>> # define xchar unsigned short
>>> #endif
>>> --- end of patch ---

This is now fixed in svn (r 11266) and will be in release 1.3.4.

> Could you please open an STR that shows the issue with your code (or my
> short test program below) and refer to this thread? Google link:
> https://groups.google.com/forum/#!topic/fltkgeneral/gzmdRk2LvAk

Please do not open an STR, since it is already fixed.