approvals / ApprovalTests.cpp

Native ApprovalTests for C++ on Linux, Mac and Windows
https://approvaltestscpp.readthedocs.io/en/latest/
Apache License 2.0
318 stars 51 forks source link

#define EMPTY causes break in subsequent includes #7

Closed saulthu closed 5 years ago

saulthu commented 5 years ago

My test cpp file starts with:

#include "ApprovalTests.hpp"
#include "Catch.hpp"
#include <opencv2/core.hpp>

which has the following error:

include\opencv2\core\persistence.hpp(506): error C2061: syntax error: identifier 'vector'

and at persistence.hpp(506) there is an enum element called EMPTY:

enum Type
{
    ...
    EMPTY     = 32, //!< empty structure (sequence or mapping)
    ...
};

and the error is caused by CombinationApprovals.h which contains:

#define EMPTY std::vector<Empty>{Empty()}

So, can we #undef EMPTY within CombinationApprovals.h?

claremacrae commented 5 years ago

BPrevent#define_EMPTY_breaking_in_subsequentincludes-fixes#7.txt

claremacrae commented 5 years ago

BRemove#define_EMPTY_which_broke_code_in_laterincludes-fixes#7.txt

claremacrae commented 5 years ago

Thanks for reporting this!

I've been able to reproduce it by adding code based on your enum example above - and have then tried two ways of fixing it.

The first patch above adds the #undef, as suggested.

The second patch above uses a different approach - a helper function that returns the "empty" container. This is cleaner as there's no chance that we stomp on an earlier #define of EMPTY... I'm not super happy with the names though - as empty() returns a container with one value in!

@isidore and I are pairing on this library later this week, so we'll chat about this then and finalise a fix - as well as hopefully doing a new release, to make the fix available...

Note to self: I'd like to try and remove #define STATIC in Macros.h as well - or rename it to make a clash much less likely.

saulthu commented 5 years ago

Thanks for the quick action! I saw your cpponsea talk on youtube and liked what I saw so gave it a try. Have my first approval test running now :)

claremacrae commented 5 years ago

I can't tell you how happy that makes me!!!

claremacrae commented 5 years ago

(that you have an approval test running, I mean...)