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

naming conflict #40

Closed
wichert opened this issue Jun 22, 2011 · 7 comments
Closed

naming conflict #40

wichert opened this issue Jun 22, 2011 · 7 comments

Comments

@wichert
Copy link
Contributor

wichert commented Jun 22, 2011

Catch appears to be using local variables with very obvious names, making it likely to get conflicts. I ran into this in a test which has a local variable called action. This resulted in the compiler producing warning: declaration of ‘action’ shadows a previous local, and indeed my tests were failing as well.

I would suggest to use a prefix for any variables injected by Catch that is not likely to conflict. Perhaps something as simple as _catch_ ?

@philsquared
Copy link
Collaborator

Hi Wichert,

AFAIK all local variables that Catch uses have fairly obfuscated names (for exactly this reason) - usually including a catch based prefix and line number too. Obviously if I've missed any I'd like to know about it (and fix it).

Could you try and boil your case down to the smallest thing that demonstrates the problem and post it here.

Thanks for the feedback.

@philsquared
Copy link
Collaborator

Actually I just had a feeling I might know where the variable you're talking about might be. Had a quick look and, sure enough, there it is. It's in a scope entirely controlled by Catch, so technically it is safe.
However I can see why it produces the warning you're seeing. Not sure why it might make your tests fail, though.

I'll push a fix up later today.

Thanks again for letting me know.

@philsquared
Copy link
Collaborator

I checked in that change. There were actually three undecorated names that I found (incuding 'action'). All should have been safely within inner-scopes - but at least for the sake of warnings I've now prefixed them.
Please let me know if this fixes the issue for you.
However, if you did manage to get a boiled down example of it causing a test to fail I'd still be very interested to know - not least so I could add it to the self-test suite.

@wichert
Copy link
Contributor Author

wichert commented Jun 23, 2011

I'll try later this week. A somewhat related question: when moving to
the latest catch version it is attractive to use the single-include
method, but I didn't see any documentation on how to do the equivalent
of using catch_with_main.hpp vs catch.hpp using it. Can you add that to
the documetation?

Wichert Akkerman wichert@wiggy.net It is simple to make things.
http://www.wiggy.net/ It is hard to make things simple.

@philsquared
Copy link
Collaborator

The docs need an overhaul + extending (the latter being the main reason for the former!).
But the single-header related stuff is pretty important so, thanks to your prompting, I've made a small update to reflect that.

@philsquared
Copy link
Collaborator

Hi @wichert - have you had a chance to verify this fix (and the docs)?

@philsquared
Copy link
Collaborator

I'm going to close this issue for now as my own view is that the issue is resolved. If you see any other name clashes please reopen (is that even possible with GitHub's system?) or just log a new issue.

Thanks again for reporting it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants