Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build failure on current Cygwin, probably caused by force-fed c++98 mode #247

Closed
HBBroeker opened this issue Apr 5, 2019 · 5 comments

Comments

@HBBroeker
Copy link

commented Apr 5, 2019

Current git HEAD fails to build on current Cygwin, because fdopen() is not found. That's because configure.ac injects --std=c++98 into the CXXFLAGS, which has two effects: first, it nails the language version to C++98, which I'll assume is what you want it to do. But secondly it also turns off all non-ISO-C++ language and library features. As a result, POSIX libc functions like fdopen() are no longer visible to the compiled code, and that fails the build of src/util/temp_file.cc.

I see three possible solutions:

  1. don't inject any --std at all --- the default state of that option is most likely better anyway
  2. if you really have to, inject --std=gnu++98 instead, which will not have the unwieldy side effect of turning off all language and library extensions
  3. if you're absolutely sure it has to be --std=c++98, at least consider adding a mechanism like autoconf's standard AC_USE_SYSTEM_EXTENSIONS that re-enable the extensions

skvadrik added a commit that referenced this issue Apr 6, 2019

configure.ac: use AC_USE_SYSTEM_EXTENSIONS, as --std=c++98 disables P…
…OSIX extensions on Cygwin.

Attempted fix for #247.
@skvadrik

This comment has been minimized.

Copy link
Owner

commented Apr 6, 2019

I'm trying to understand why --std=c++98 does not have the same effect on other POSIX systems (Linux for example). If it turns off non-ISO-C++ features, why does the compiler (I assume GCC) succeed on Linux? The declaration of fdopen is in <stdio.h>. I assume the header was found in both cases, but then some macro like _POSIX_C_SOURCE was not defined on Cygwin.

The goal of having --std=c++98 is to increase portability on systems that have old (pre C++11) compilers. Of course, if it starts to be an obstacle, then it has the opposite effect (portability is decreased). But it would be better to understand why the different behaviour on Cygwin.

I'll go with AC_USE_SYSTEM_EXTENSIONS for now: c32d433, let me know if it doesn't work.

@HBBroeker

This comment has been minimized.

Copy link
Author

commented Apr 6, 2019

The goal of having --std=c++98 is to increase portability on systems that have old (pre C++11) compilers.

But these days, it has almost entirely adverse effects:

  1. it dials the C++ standard down to C++98, even if the compiler would ordinarily support C++11 or newer.
  2. it disables all language extensions, including system library functions not part of the ISO C++ standard.

Of course, if it starts to be an obstacle, then it has the opposite effect (portability is decreased).

Portability is in fact increased too much, by setting it to C++98. The fact of the matter is that the source code is not written in strictly pure C++98 --- if it were, you would not need any autoconf tests at all.

But the source does use some POSIX-specific extensions, particularly fdopen(). Once you dialed down all the way to --std=c++98, you have to dial back up to having those available somehow. The absolute minimum on systems that have glibc-style feature test macros is to define _POSIX_C_SOURCE to some suitable value.

AC_USE_SYSTEM_EXTENSIONS effectively turns on _GNU_SOURCE, which brings you back to the same state, library feature-wise, you would have been with --std=gnu++98, or no --std option at all

@trofi

This comment has been minimized.

Copy link
Contributor

commented Apr 8, 2019

None of the available C++ standards define fdopen. -std= option is a distraction here.

fdopen() is a POSIX.1-2001 (and above) library call. Which makes sense as fdopen assumes file descriptors exist and are used in file API.

Mechanically -std=c++98 works like that:

$ diff -U0 <(g++ -x c++ -dM -E - </dev/null) <(g++ -x c++ -std=c++98 -dM -E - </dev/null)
...
#define __STRICT_ANSI__ 1

glibc happens to enable POSIX extensions by default:

/* If nothing (other than _GNU_SOURCE and _DEFAULT_SOURCE) is defined,
   define _DEFAULT_SOURCE.  */
#if (defined _DEFAULT_SOURCE                                    \
     || (!defined __STRICT_ANSI__                               \
         && !defined _ISOC99_SOURCE && !defined _ISOC11_SOURCE  \
         && !defined _POSIX_SOURCE && !defined _POSIX_C_SOURCE  \
         && !defined _XOPEN_SOURCE))
# undef  _DEFAULT_SOURCE
# define _DEFAULT_SOURCE        1
#endif
...
/* If none of the ANSI/POSIX macros are defined, or if _DEFAULT_SOURCE
   is defined, use POSIX.1-2008 (or another version depending on
   _XOPEN_SOURCE).  */
#ifdef _DEFAULT_SOURCE
# if !defined _POSIX_SOURCE && !defined _POSIX_C_SOURCE
#  define __USE_POSIX_IMPLICITLY        1
# endif
# undef  _POSIX_SOURCE
# define _POSIX_SOURCE  1
# undef  _POSIX_C_SOURCE
# define _POSIX_C_SOURCE        200809L
#endif
...
#if (defined _POSIX_SOURCE                                      \
     || (defined _POSIX_C_SOURCE && _POSIX_C_SOURCE >= 1)       \
     || defined _XOPEN_SOURCE)
# define __USE_POSIX    1
#endif
...
#ifdef __USE_POSIX
/* Create a new stream that refers to an existing system file descriptor.  */
extern FILE *fdopen (int __fd, const char *__modes) __THROW __wur;
#endif

Note: __STRICT_ANSI__ disables most of system-specific defaults (desired behaviour for re2c to ease porting). AC_USE_SYSTEM_EXTENSIONS (https://www.gnu.org/software/autoconf/manual/autoconf-2.64/html_node/Posix-Variants.html) is the correct way to use fdopen explicitly.

Also note: all the above is glibc's policy implementation. Other libcs (say, non-POSIX targets) can chose not to export anything out of C++ language unless _POSIX_C_SOURCE is defined by user expicitly.

@HBBroeker

This comment has been minimized.

Copy link
Author

commented Apr 8, 2019

None of the available C++ standards define fdopen. -std= option is a distraction here.

Not really. It's the core of the issue. Without any --std flag, or with --std=gnu++98, the issue would not exist, because that's the primary flag that forcibly turns off all the extensions. If they hadn't been turned off, there would be no need to turn them on again.

glibc happens to enable POSIX extensions by default:

If it does, that's not by any of the code snippets you show.

/* If nothing (other than _GNU_SOURCE and _DEFAULT_SOURCE) is defined,

This doesn't activate, because something is defined: __STRICT_ANSI__

/* If none of the ANSI/POSIX macros are defined, or if _DEFAULT_SOURCE
is defined, use POSIX.1-2008 (or another version depending on
_XOPEN_SOURCE). */

Consequentially, this does not trigger either.

Also note: all the above is glibc's policy implementation.

Precisely. Arguably, it's a bug in glibc if it enables _POSIX_C_SOURCE in the face of __STRICT_ANSI__. At the very least it's a rather wilful interpretation of the documented meaning of --std=c++98, compared to --std=gnu++98. And since Cygwin doesn't use glibc, it's not entirely surprising that its behaviour is, indeed, sufficiently different to cause a build failure.

@skvadrik

This comment has been minimized.

Copy link
Owner

commented Apr 8, 2019

@HBBroeker , as you say, re2c source is not pure C++98, but only library-wise, not language-wise. We do use non-standard library functions, but we also do keep the language subset within C++98 bounds (or try to). So my reasoning here is the following:

  • Dropping --std=... altogether doesn't seem right because it drops language restrictions.
  • Using --std=gnu++98 doesn't seem right because it adds some language features on top of C++98.
  • Using AC_USE_SYSTEM_EXTENSIONS seems ok, because it disentangles language and library extensions and enables only the latter (if I understand it correctly).

However, there is no need to enforce --std=... on re2c users or distro-maintainers. If it becomes too much of a trouble, it can go to some developer build script. Adding it to default flags is primarily to set expectations for all the people who clone or fork re2c, build it from source with simple configure && make and do not necessarily look into every script.

The true Autotools way is probably to test for each particular non-standard function, not for headers as we do.

@skvadrik skvadrik closed this Jun 10, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.