Issue #1: Back port Trompeloeil to C++11. #67

AndrewPaxie · 2017-11-13T18:20:35Z

Back ported Trompeloeil to C++11.  Changes by file and directory:
* `.travis.yml`
  * Added job for `g++-4.8` C++11 mode.
  * Accepted 4 errors in `check_errors.sh` when in C++11 mode.
* `CMakeLists.txt`
  * Allowed C++ dialect to be specified on command line using
    `CXX_STANDARD="11"` for C++11, default C++14.
  * Fixed Catch at version 1.11.0, given 2.0.1 triggers a defect
    in `g++` when compiling `libc++`.
  * Refined which sanitizers can be used based on given compiler
    and compiler version.
* `README.md`
  * Updated supported compiler list.
  * Added link to `docs/Backward.md`.
  * Added link to `docs/PlatformAndLibraries.md`.
* `include/trompeloeil.hpp`
  * Significantly reworked to accommodate the C++11 API and
    support compilation in a C++11 dialect.
* `check_errors.sh`
  * Require `CXXFLAGS` with either `-std=c++11` or `-std=c++14`.
* `docs/FAQ.md`
  * Updated supported compilers list.
  * Updated answer regarding C++11 support.
* `docs/`
  * New: "Backward compatibility with earlier versions of C++".
  * New: "Platform and library support for Trompeloeil".
* `compilation_errors/`
  * Support for C++11 expectation syntax in test cases.
* `test/`
  * Split `compiling_tests.cpp` into separately compilable files,
    for Catch main, C++11 test cases and C++14 test cases.

First round review.

Avoid static destruction fiasco for recursive_mutex Fix issue 55: restore warnings for Clang builds

Removed TROMPELOEIL_DEBUG_RE_FAILURE after confirming regex_check fixed.

Reorganized environment for g++-4.8 to see if it's recognized. Fix bug in `struct throw_handler_t` found by g++-7 AddressSanitizer.

- Require standard C++ mode without extensions. - g++-7: disabled sanitize-address-use-after-scope ASan check. - clang++4.0: enabled sanitize-address-use-after-scope ASan check. - Added -j 4 to make command line.

coveralls · 2017-11-15T08:06:29Z

Coverage decreased (-4.1%) to 95.0% when pulling 5138911 on AndrewPaxie:c++11 into f78af43 on rollbear:develop.

AndrewPaxie · 2017-11-15T17:43:04Z

I have made some changes to improve Coveralls integration:

v34 of kcov selected so it can compile on Artful Aardvark (glibc 2.26 no longer defines SIGUNUSED).
Tried changing the include pattern to include/trompeloeil.hpp but this might not be the right place to define this for Coveralls integration. I am exploring coveralls.io UI to see what reporting facilities there are. Perhaps this broke when trompeloeil.hpp moved to directory include?

The drop in coverage in g++-4.8 -std=c++11 is due to the way I disabled the regex test failures, among other tests that fail e.g. neg_matcher failures. I have another approach that I would like to try that conditionally uses regex matching; if the test suite is not enabled for regexes then it will fall back to a less precise string comparison method.

Thus there will be further updates as the review progresses.

rollbear · 2017-11-15T23:43:54Z

Apologies my lack of attention, but my ISP (bless their infinite incompetence) has left me without internet connection for a week now, and my fallback over the phone is running thin. I'll get back to you, but I hope you can live with my apparent absense for the moment.

AndrewPaxie · 2017-11-16T00:27:18Z

All is good. Plenty of work to get through on the current feedback.

coveralls · 2017-11-16T08:34:54Z

Coverage decreased (-0.3%) to 98.843% when pulling 8000792 on AndrewPaxie:c++11 into f78af43 on rollbear:develop.

coveralls · 2017-11-16T19:23:45Z

Coverage decreased (-0.08%) to 99.034% when pulling 1d4fa3c on AndrewPaxie:c++11 into f78af43 on rollbear:develop.

coveralls · 2017-11-18T23:58:32Z

Coverage decreased (-0.08%) to 99.034% when pulling 6044957 on AndrewPaxie:c++11 into f78af43 on rollbear:develop.

AndrewPaxie · 2017-11-19T00:52:43Z

I have had a look at the history of Coveralls builds to find where browsing to the source of trompeloeil.hpp stopped working. History:

Working in build #328, branch develop, 5 July 2017.

Working in build #330, branch master, 11 July 2017.
https://coveralls.io/builds/12348365/source?filename=trompeloeil.hpp

Commit e57910f @rollbear rollbear committed on Jul 18
"Host self test programs in cmake" is when trompeloeil.hpp moved to include/.

Broken in build #349, branch cmake, 20 July 2017.
https://coveralls.io/builds/12473361/source?filename=trompeloeil.hpp

Broken in build #354, branch develop, 20 July 2017.

Seems there is some update needed to .travis.yml or the Coveralls service configuration, not sure which.

AndrewPaxie · 2017-11-19T02:37:10Z

I raised issue Make Catch_global_namespace_dummy a complete type in the Catch2 project summarizing the failure to compile Trompeloeil with libc++ and suggesting a fix.

rollbear · 2017-11-19T10:30:02Z

That issue with coveralls not showing the source is so weird. I'm certain it worked after that trompeloeil.hpp move into directory include.

rollbear · 2017-11-19T11:43:41Z

There was an input field on the coveralls.io site, where I could enter a subdirectory to look for the file in. That worked out fine.

rollbear · 2017-11-19T12:39:36Z

include/trompeloeil.hpp

@@ -529,10 +849,23 @@ namespace trompeloeil
  class duck_typed_matcher : public matcher
  {
  public:
+#if TROMPELOEIL_GCC && TROMPELOEIL_GCC_VERSION < 40900


I'm a bit confused by the operator std::string&&() const added here. The comment implies that this is just to make the self test compile, but does this mean that duck typed matchers do not work with g++-4.8? If so, I wonder if it wouldn't be better to exclude those tests when building with g++-4.8.

This is a long story to be told with whisky and tears in equal parts. The code here is the product of many other failed experiments. I am investigating the consequences of removing support for these g++-4.8-specific user-defined conversions in duck_typed_matcher and wildcard on my production code base and will have a better idea of the direction to take in the next day or so.

We live about as far apart as we can on this planet, but should we meet, I'll bring the whisky and you can cry your eyes out ;-)
No worries.

I disabled the non-template user-defined conversions in wildcard and
duck_typed_matcher after finding

No production code was affected by not having support for implicit
conversions from wildcard or duck_typed_matcher to rvalue references.

Only five test cases failed without these user-defined conversions.
These are now guarded with TROMPELOEIL_TEST_RVALUE_REFERENCE_FAILURES.
Interestingly only wildcard test cases failed: I could not find a test
case failure with duck_typed_matcher.

Documentation in Backward.md has been updated to explain the shortcomings
of g++-4.8 should someone stumble upon this issue.

It also appears that the impact on coverage as reported by Coveralls has
been minimal.

rollbear · 2017-11-19T12:40:55Z

include/trompeloeil.hpp

@@ -1279,14 +1615,30 @@ namespace trompeloeil
    noexcept
    {}

+#if TROMPELOEIL_GCC && TROMPELOEIL_GCC_VERSION < 40900


Same confusion here.

rollbear · 2017-11-19T12:44:15Z

include/trompeloeil.hpp

+      typename Sig,
+      typename H,
+      typename R = decltype(::trompeloeil::default_return<return_of_t<Sig>>())>
+    struct throw_handler_t


Is there a reason for this to be a template? The template types are already known in the surrounding type and should be accessible.

It turns out that Sig can be dropped as a template parameter, using signature instead.
R can be made a type alias inside the struct.
But H remains as a template parameter of throw_handler_t as it comes from the
template parameter H of handle_throw.

rollbear · 2017-11-19T12:46:51Z

include/trompeloeil.hpp

@@ -3498,6 +3991,9 @@ namespace trompeloeil
    unsigned long trompeloeil_expectation_line;                                \
    const char *trompeloeil_expectation_string;                                \
                                                                               \
+    using call_params_type_t = ::trompeloeil::call_params_type_t<sig>;         \
+    using return_of_t = ::trompeloeil::return_of_t<sig>;                       \


These types really should have a trompeloeil_ prefix to their names. It's not unimaginable that anyone may want to mock a function named return_of_t, or call_params_type_t.

rollbear · 2017-11-19T12:49:13Z

Added some comments/questions to ´trompeloeil.hpp`. I'll dive into the macros at the end of the file later.

…n step. Actioned review comments: - Simplified class template throw_handler_t. - Renamed internal types in mock function implementation to avoid name clashes.

coveralls · 2017-11-20T19:28:47Z

Coverage decreased (-0.08%) to 99.034% when pulling 096ff87 on AndrewPaxie:c++11 into f78af43 on rollbear:develop.

coveralls · 2017-11-22T08:19:02Z

Coverage decreased (-0.08%) to 99.034% when pulling a2e0af6 on AndrewPaxie:c++11 into f78af43 on rollbear:develop.

rollbear · 2017-11-22T11:12:34Z

OK. I'm falling behind again, since my ISP finally "fixed" the problems (and I now have no internet connection at all instead of just a bad one.) I'll get back to you when things are working to at least some degree.

coveralls · 2017-11-25T07:37:40Z

Coverage decreased (-0.08%) to 99.034% when pulling e3694e3 on AndrewPaxie:c++11 into f78af43 on rollbear:develop.

rollbear · 2017-11-27T18:39:52Z

Internet connection restored. I'll get back to you. There's just a bit of a backlog to wade through...

rollbear · 2017-12-02T13:18:23Z

Phiew. It took some time to read through. I'll let it simmer in the develop branch a while before tagging the big official C++11 compatible release.

One question, though. The _AS_STRING macros. Is there any particular reason why there are two different implementations? The C++11 version surely works just as well in C++14? Unless you tell me not to, I'll merge them. Then there's the problem with the short form name, of course. A macro named AS_STRING only is almost guaranteed to collide with other macros in user code.

AndrewPaxie · 2017-12-03T18:48:45Z

Thanks for the review and merge! I was just drafting this response
but will proceed with pull requests against develop.

Yes it is a good idea to let the code "rest" for a while in develop.
I am still actively seeking a means to improve the TROMPELOEIL_RETURN_ macro
so the -> e_t::trompeloeil_return_of_t trailing return type may be dropped.
Of course, the delay is because I never really understood the differences
between template type deduction, auto type deduction, and decltype type
deduction (and still don't).

I am going to revert the _AS_STRING macros in the next 18 hours.
~~Please do not merge them.~~ Their importance is low and the possibility
of name-clash and of irritating our users is high. I will move the C++11
version to compiling_tests_11.cpp, where it is needed, and drop the
second C++14 version (never used).

As to their necessity and difference, the C++11 expectation macros expand
macros given in the func (and other) arguments before any stringizing
is performed. For C++11, ANY(int) is stringized as
::trompeloeil::any_matcher<int>("int")
whereas for C++14 it is stringized as ANY(int).

This lead to all kinds of failures in the test cases in
compiling_tests_11.cpp.

These utility macros were supposed to hide the actual stringized contents
of macro. So

C++11: macro expand then stringize.
C++14: stringize without macro expansion.

This was another case of seeking but not finding a point-for-point C++11
version of the expectation macros when compared to the C++14 implementation.

I explained this difference in
Macro expansion of ANY matcher before stringizing
I will update this documentation to reflect our moving the responsibility
to providing a workaround to those users who are actually affected.

rollbear and others added 30 commits September 4, 2017 14:22

Add Conan badge again

792a9f7

Document clang++-5 compatibility

fb8d8e9

Fix issue 48: undefined reference with clang++-3.5

d672288

Avoid static destruction fiasco for recursive_mutex Fix issue 55: restore warnings for Clang builds

C++11 backport, first cut

325257f

Disable test cases that fail with g++ 4.8.3

4f61aa8

Ensure disabled tests for g++ 4.8.3 are disabled for that version only.

e5d8af3

Test with Visual Studio 2017.

7917b23

Refine compilation_errors for C++11 mode.

225db95

Removed TROMPELOEIL_DEBUG_RE_FAILURE after confirming regex_check fixed.

Removed compiler warnings

2d2497c

Resolved warnings from clang-check

479524a

Enabled g++-4.8 on Travis

77665eb

Removed kcov from clang++-3.7 and clang++-3.9 builds.

2ea6cf4

Reorganized environment for g++-4.8 to see if it's recognized. Fix bug in `struct throw_handler_t` found by g++-7 AddressSanitizer.

Fixed indentation level of 'env:' for g++-4.8 .

21c8fa5

Expect four failures from C++11 build

8e1c282

Another attempt at failure counting for g++-4.8

2c08af3

Removed trailing backslashes for g++-4.8

d105c66

Split compiling_tests.cpp into smaller pieces

9897631

Created C++11 version two API

b326f70

Changed functions to internal linkage

4bddce4

Travis changes:

5200b74

- Require standard C++ mode without extensions. - g++-7: disabled sanitize-address-use-after-scope ASan check. - clang++4.0: enabled sanitize-address-use-after-scope ASan check. - Added -j 4 to make command line.

Created C++11 version three API

8b2a076

Cope with build timeout on Travis during make by waiting longer

6171335

Scale back on number of jobs for make

d7f5083

Revert travis_wait

6794815

Revert job count to 1 with make for Travis

3306b0c

Fix accidental tabs and missing fix (move vs forward)

1e15cfe

Avoid unneeded-member-function in unnamed namespace

d316da0

Merge branch 'develop' into c++11

c51b564

Merge branch 'develop' into c++11

cb2a87a

Macros with two arguments in C++11 v3 API are now optional

8894ac1

Enabled more test cases in C++ dialect to increase coverage

8000792

Removed CXXFLAGS from most jobs so run_coverage works

1d4fa3c

Added workarounds for some g++-4.8 compiler defects

6044957

rollbear reviewed Nov 19, 2017

View reviewed changes

Reverted file path change to Trompeloeil header in coverage generatio…

096ff87

…n step. Actioned review comments: - Simplified class template throw_handler_t. - Renamed internal types in mock function implementation to avoid name clashes.

Backed out user-defined conversions for wildcard and duck_typed_matchers

a2e0af6

Resolved warning from clang-check

e3694e3

rollbear merged commit 6bcb8c3 into rollbear:develop Dec 2, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue #1: Back port Trompeloeil to C++11. #67

Issue #1: Back port Trompeloeil to C++11. #67

AndrewPaxie commented Nov 13, 2017

coveralls commented Nov 15, 2017 •

edited

Loading

AndrewPaxie commented Nov 15, 2017

rollbear commented Nov 15, 2017

AndrewPaxie commented Nov 16, 2017

coveralls commented Nov 16, 2017 •

edited

Loading

coveralls commented Nov 16, 2017 •

edited

Loading

coveralls commented Nov 18, 2017 •

edited

Loading

AndrewPaxie commented Nov 19, 2017

AndrewPaxie commented Nov 19, 2017

rollbear commented Nov 19, 2017

rollbear commented Nov 19, 2017

rollbear Nov 19, 2017

AndrewPaxie Nov 20, 2017

rollbear Nov 20, 2017

AndrewPaxie Nov 22, 2017

rollbear Nov 19, 2017

rollbear Nov 19, 2017

AndrewPaxie Nov 20, 2017

rollbear Nov 19, 2017

AndrewPaxie Nov 20, 2017

rollbear commented Nov 19, 2017

coveralls commented Nov 20, 2017 •

edited

Loading

coveralls commented Nov 22, 2017 •

edited

Loading

rollbear commented Nov 22, 2017

coveralls commented Nov 25, 2017 •

edited

Loading

rollbear commented Nov 27, 2017

rollbear commented Dec 2, 2017

AndrewPaxie commented Dec 3, 2017

Issue #1: Back port Trompeloeil to C++11. #67

Issue #1: Back port Trompeloeil to C++11. #67

Conversation

AndrewPaxie commented Nov 13, 2017

coveralls commented Nov 15, 2017 • edited Loading

AndrewPaxie commented Nov 15, 2017

rollbear commented Nov 15, 2017

AndrewPaxie commented Nov 16, 2017

coveralls commented Nov 16, 2017 • edited Loading

coveralls commented Nov 16, 2017 • edited Loading

coveralls commented Nov 18, 2017 • edited Loading

AndrewPaxie commented Nov 19, 2017

AndrewPaxie commented Nov 19, 2017

rollbear commented Nov 19, 2017

rollbear commented Nov 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rollbear commented Nov 19, 2017

coveralls commented Nov 20, 2017 • edited Loading

coveralls commented Nov 22, 2017 • edited Loading

rollbear commented Nov 22, 2017

coveralls commented Nov 25, 2017 • edited Loading

rollbear commented Nov 27, 2017

rollbear commented Dec 2, 2017

AndrewPaxie commented Dec 3, 2017

coveralls commented Nov 15, 2017 •

edited

Loading

coveralls commented Nov 16, 2017 •

edited

Loading

coveralls commented Nov 16, 2017 •

edited

Loading

coveralls commented Nov 18, 2017 •

edited

Loading

coveralls commented Nov 20, 2017 •

edited

Loading

coveralls commented Nov 22, 2017 •

edited

Loading

coveralls commented Nov 25, 2017 •

edited

Loading