use __COUNTER__ instead of __LINE__ for unique name in INFO macro #210

Closed
wants to merge 5 commits into
from

Conversation

Projects
None yet
7 participants
@Slonegg

Slonegg commented Oct 27, 2013

Resolve #209

trifonov-bc added some commits Oct 27, 2013

Merge remote-tracking branch 'origin/master'
Conflicts:
	README.md
	include/internal/catch_version.hpp
	single_include/catch.hpp
@philsquared

This comment has been minimized.

Show comment Hide comment
@philsquared

philsquared Nov 7, 2013

Collaborator

Thanks for this - it's something I've been meaning to add for a while.
The small obstacle is that __COUNTER__ is a non-standard extension. It's supported by gcc/ clang and msvc - and no doubt some other compilers too - but it's by no means ubiquitous. So I'd need to use it conditionally, based on detection of a compiler that supports it. This complicates things a little (especially since I've already got separate pairs of definitions for most of the macros, predicated on whether a compiler supports variadic macros or not). I'm sure, in this case, the conditional code can be contained to the INTERNAL_CATCH_UNIQUE_NAME definition, so it shouldn't be so bad - but I'd need to have access to a compiler that support it to be sure.

Collaborator

philsquared commented Nov 7, 2013

Thanks for this - it's something I've been meaning to add for a while.
The small obstacle is that __COUNTER__ is a non-standard extension. It's supported by gcc/ clang and msvc - and no doubt some other compilers too - but it's by no means ubiquitous. So I'd need to use it conditionally, based on detection of a compiler that supports it. This complicates things a little (especially since I've already got separate pairs of definitions for most of the macros, predicated on whether a compiler supports variadic macros or not). I'm sure, in this case, the conditional code can be contained to the INTERNAL_CATCH_UNIQUE_NAME definition, so it shouldn't be so bad - but I'd need to have access to a compiler that support it to be sure.

@Slonegg

This comment has been minimized.

Show comment Hide comment
@Slonegg

Slonegg Nov 7, 2013

@philsquared I've added compiler checks, they are not ideal, but basically now it should compile everywhere.

I can't modify INTERNAL_CATCH_UNIQUE_NAME to use COUNTER where applicable, there are plenty of code that relies on fact you get same name on same line.

Slonegg commented Nov 7, 2013

@philsquared I've added compiler checks, they are not ideal, but basically now it should compile everywhere.

I can't modify INTERNAL_CATCH_UNIQUE_NAME to use COUNTER where applicable, there are plenty of code that relies on fact you get same name on same line.

@grahamboree

This comment has been minimized.

Show comment Hide comment
@grahamboree

grahamboree Feb 13, 2014

Could you just concatenate FILE to LINE in the declaration of INTERNAL_CATCH_UNIQUE_NAME(...) to form a pretty-darn-unique name? It's a bit safer than COUNTER because it's using standardized macro names, though it's not as complete a fix. The test case names will still collide if defined on the same line in either the same file or two files with the same name. It'd take me about 5 minutes to make this change and send the PR if you think this is a good interim fix.

Could you just concatenate FILE to LINE in the declaration of INTERNAL_CATCH_UNIQUE_NAME(...) to form a pretty-darn-unique name? It's a bit safer than COUNTER because it's using standardized macro names, though it's not as complete a fix. The test case names will still collide if defined on the same line in either the same file or two files with the same name. It'd take me about 5 minutes to make this change and send the PR if you think this is a good interim fix.

@philsquared

This comment has been minimized.

Show comment Hide comment
@philsquared

philsquared Feb 14, 2014

Collaborator

Sadly not. Filenames are usually of the form "Filename.cpp" - and you can't have the dot in a C++ identifier.

Collaborator

philsquared commented Feb 14, 2014

Sadly not. Filenames are usually of the form "Filename.cpp" - and you can't have the dot in a C++ identifier.

@colonelsammy

This comment has been minimized.

Show comment Hide comment
@colonelsammy

colonelsammy Feb 14, 2014

Contributor

You can't have the quotes either - quotes can be easily added with the preprocessor but sadly, not removed :-(

Contributor

colonelsammy commented Feb 14, 2014

You can't have the quotes either - quotes can be easily added with the preprocessor but sadly, not removed :-(

@PureAbstract

This comment has been minimized.

Show comment Hide comment
@PureAbstract

PureAbstract Feb 14, 2014

Contributor

FILE gives you a string literal, with the potential for all sorts of
havoc:

"../src/foo bar ++ baz xyzzy.cpp;[123]" is highly unlikely - but
certainly possible.

Contributor

PureAbstract commented Feb 14, 2014

FILE gives you a string literal, with the potential for all sorts of
havoc:

"../src/foo bar ++ baz xyzzy.cpp;[123]" is highly unlikely - but
certainly possible.

@ned14

This comment has been minimized.

Show comment Hide comment
@ned14

ned14 Nov 5, 2014

I'm currently making an emulation of Boost.Test using CATCH and a mutex. A big problem is that one can use #include to include many test cases into a single file, and then of course LINE collides.

I have implemented a hack solution based on COUNTER instead which looks like the above, but you need to define INTERNAL_CATCH_TESTCASE a bit differently to avoid the iteration of COUNTER:

    #define INTERNAL_CATCH_TESTCASE_UNIQUE_NAME( UniqueName, ... ) \
        static void UniqueName(); \
        namespace{ Catch::AutoReg INTERNAL_CATCH_UNIQUE_NAME( autoRegistrar )( &UniqueName, CATCH_INTERNAL_LINEINFO, Catch::NameAndDesc( __VA_ARGS__ ) ); }\
        static void UniqueName()
    #define INTERNAL_CATCH_TESTCASE( ... ) INTERNAL_CATCH_TESTCASE_UNIQUE_NAME( INTERNAL_CATCH_UNIQUE_NAME( ____C_A_T_C_H____T_E_S_T____ ), __VA_ARGS__ )

This seems to work well here. Any reason why this pull request hasn't been merged yet?

ned14 commented Nov 5, 2014

I'm currently making an emulation of Boost.Test using CATCH and a mutex. A big problem is that one can use #include to include many test cases into a single file, and then of course LINE collides.

I have implemented a hack solution based on COUNTER instead which looks like the above, but you need to define INTERNAL_CATCH_TESTCASE a bit differently to avoid the iteration of COUNTER:

    #define INTERNAL_CATCH_TESTCASE_UNIQUE_NAME( UniqueName, ... ) \
        static void UniqueName(); \
        namespace{ Catch::AutoReg INTERNAL_CATCH_UNIQUE_NAME( autoRegistrar )( &UniqueName, CATCH_INTERNAL_LINEINFO, Catch::NameAndDesc( __VA_ARGS__ ) ); }\
        static void UniqueName()
    #define INTERNAL_CATCH_TESTCASE( ... ) INTERNAL_CATCH_TESTCASE_UNIQUE_NAME( INTERNAL_CATCH_UNIQUE_NAME( ____C_A_T_C_H____T_E_S_T____ ), __VA_ARGS__ )

This seems to work well here. Any reason why this pull request hasn't been merged yet?

@philsquared

This comment has been minimized.

Show comment Hide comment
@philsquared

philsquared Mar 15, 2016

Collaborator

I implemented a version of this a while ago on the develop-v2 branch (see #351) and hadn't realised I never got around to merging it into master. I've now done that.

Collaborator

philsquared commented Mar 15, 2016

I implemented a version of this a while ago on the develop-v2 branch (see #351) and hadn't realised I never got around to merging it into master. I've now done that.

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