Skip to content

Fix some of the portability issues in lest_expect_abort #79

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.

Sign up for GitHub

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

Jump to bottom
Merged
martinmoene merged 1 commit into from
Jan 10, 2025
Merged

Fix some of the portability issues in lest_expect_abort #79

martinmoene merged 1 commit into from
Jan 10, 2025

Conversation

@musicinmybrain
Copy link
Contributor

@musicinmybrain musicinmybrain commented Jan 10, 2025

Unconditionally include cstdio for fflush and friends, and cstdlib for abort. When _WIN32 is not defined, include unistd.h instead of io.h for dup and dup2.

This fixes some of the portability issues in lest_expect_abort contrib library, and I guess we can consider that it fixes #39, but it does not entirely fix building lest_expect_abort:

In file included from /home/ben/src/forks/lest/contrib/lest_expect_abort/test/lest_expect_abort.t.cpp:9:
/home/ben/src/forks/lest/contrib/../contrib/lest_expect_abort/lest_expect_abort_cpp03.hpp:85:62: error: expected initializer before ‘abort’
   85 | #  define lest_ABORT_SIGNATURE()  lest_NORETURN void __cdecl abort()
      |                                                              ^~~~~
/home/ben/src/forks/lest/contrib/../contrib/lest_expect_abort/lest_expect_abort_cpp03.hpp:167:1: note: in expansion of macro ‘lest_ABORT_SIGNATURE’
  167 | lest_ABORT_SIGNATURE()
      | ^~~~~~~~~~~~~~~~~~~~
gmake[2]: *** [contrib/lest_expect_abort/test/CMakeFiles/contrib-abort-lest_expect_abort.t_cpp03.dir/build.make:79: contrib/lest_expect_abort/test/CMakeFiles/contrib-abort-lest_expect_abort.t_cpp03.dir/lest_expect_abort.t.cpp.o] Error 1
gmake[2]: Leaving directory '/home/ben/src/forks/lest/build'
gmake[1]: *** [CMakeFiles/Makefile2:601: contrib/lest_expect_abort/test/CMakeFiles/contrib-abort-lest_expect_abort.t_cpp03.dir/all] Error 2
gmake[1]: *** Waiting for unfinished jobs....
In file included from /home/ben/src/forks/lest/contrib/lest_expect_abort/example/00_basic.cpp:2:
/home/ben/src/forks/lest/contrib/../contrib/lest_expect_abort/lest_expect_abort.hpp:87:62: error: expected initializer before ‘abort’
   87 | #  define lest_ABORT_SIGNATURE()  lest_NORETURN void __cdecl abort()
      |                                                              ^~~~~
/home/ben/src/forks/lest/contrib/../contrib/lest_expect_abort/lest_expect_abort.hpp:160:1: note: in expansion of macro ‘lest_ABORT_SIGNATURE’
  160 | lest_ABORT_SIGNATURE()
      | ^~~~~~~~~~~~~~~~~~~~
In file included from /home/ben/src/forks/lest/contrib/lest_expect_abort/example/01_specification.cpp:7:
/home/ben/src/forks/lest/contrib/../contrib/lest_expect_abort/lest_expect_abort.hpp:87:62: error: expected initializer before ‘abort’
   87 | #  define lest_ABORT_SIGNATURE()  lest_NORETURN void __cdecl abort()
      |                                                              ^~~~~
/home/ben/src/forks/lest/contrib/../contrib/lest_expect_abort/lest_expect_abort.hpp:160:1: note: in expansion of macro ‘lest_ABORT_SIGNATURE’
  160 | lest_ABORT_SIGNATURE()
      | ^~~~~~~~~~~~~~~~~~~~
In file included from /home/ben/src/forks/lest/contrib/lest_expect_abort/test/lest_expect_abort.t.cpp:7:
/home/ben/src/forks/lest/contrib/../contrib/lest_expect_abort/lest_expect_abort.hpp:87:62: error: expected initializer before ‘abort’
   87 | #  define lest_ABORT_SIGNATURE()  lest_NORETURN void __cdecl abort()
      |                                                              ^~~~~
/home/ben/src/forks/lest/contrib/../contrib/lest_expect_abort/lest_expect_abort.hpp:160:1: note: in expansion of macro ‘lest_ABORT_SIGNATURE’
  160 | lest_ABORT_SIGNATURE()
      | ^~~~~~~~~~~~~~~~~~~~

Here it looks like the design of lest_expect_abort expects to have the exact signature of abort, and it‘s attempting to satisfy that requirement with some preprocessor conditionals that are both dated and Windows-specific:

lest/contrib/lest_expect_abort/lest_expect_abort.hpp

Lines 75 to 87 in 778597f

#ifndef lest_ABORT_SIGNATURE
# if lest_COMPILER_MSVC_VERSION
# define lest_NORETURN __declspec(noreturn)
# define lest_ABORT_SIGNATURE() /*_[A]CRTIMP*/ lest_NORETURN void __cdecl abort(void)
# else
# define lest_NORETURN [[noreturn]]
# define lest_ABORT_SIGNATURE() lest_NORETURN void __cdecl abort()
# endif
#else
# ifndef lest_NORETURN
# define lest_NORETURN
# endif
#endif

Well, that’s probably why this is in contrib and not in the main library. Still, this PR should be a step in the right direction for anyone who might be trying to make it work. This is probably the last PR I’ll submit related to the contrib libraries.

@musicinmybrain
Make lest_expect_abort more portable to non-Windows platforms
c590a39 
Unconditionally include `cstdio` for `fflush` and friends, and `cstdlib`
for `abort`. When `_WIN32` is not defined, include `unistd.h` instead of
`io.h` for `dup` and `dup2`.
@martinmoene martinmoene merged commit 03ffa52 into martinmoene:master Jan 10, 2025
7 of 9 checks passed
@martinmoene
Copy link
Owner

martinmoene commented Jan 10, 2025

Thank you for you efforts!

Contents of contrib are the result of several learning attempts which were 'not entirely' convincing, and the products of which I did not want to throw away at the time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Build fails due to missing <io.h> header in contrib/lest_expect_abort

2 participants

@musicinmybrain @martinmoene