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

Added std:: where it was missing #544

Closed
wants to merge 11 commits into from
Closed

Added std:: where it was missing #544

wants to merge 11 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Dec 3, 2015

This is in relation to issue #543 and covers the main places where std:: is missing but there may still be further calls.

@nabijaczleweli
Copy link
Contributor

👍

@ghost
Copy link
Author

ghost commented Dec 3, 2015

Okay, so currently Travis doesn't like it if I changed ::tolower to std::tolower. Some other fix is evidently necessary to fix that issue. In my gcc 4.4.2 compiler I am getting:

/../external/clara.h:235: error: '::tolower' has not been declared

@refi64
Copy link

refi64 commented Dec 3, 2015

Is there a #include <ctype.h>?

@ghost
Copy link
Author

ghost commented Dec 3, 2015

Good call @kirbyfan64 will add the missing include into clara.h and catch_common.hpp and put the std:: back in :
#include < cctype >

Q) Is there an easy way to roll back changes on github? I just re-made the changes instead for now!

@nabijaczleweli
Copy link
Contributor

@crog You can use the standard set of git commands

@ghost
Copy link
Author

ghost commented Dec 3, 2015

Thanks, I will take a look at Git commands soon. For now I can't solve the issue of ::tolower as Travis build system doesn't have a test that replicates a strict conforming environment like I have here :(

@ghost
Copy link
Author

ghost commented Dec 4, 2015

Cracked it. GCC reports nicely in some versions about std::tolower having ambiguous overloads. Fixing the ambiguity with the following tells the compiler which function to use and this now builds under QNX and all the Travis Test :)

@nabijaczleweli
Copy link
Contributor

👍

@@ -232,7 +233,7 @@ namespace Clara {
}
inline void convertInto( std::string const& _source, bool& _dest ) {
std::string sourceLC = _source;
std::transform( sourceLC.begin(), sourceLC.end(), sourceLC.begin(), ::tolower );
std::transform( sourceLC.begin(), sourceLC.end(), sourceLC.begin(), static_cast<int(*)(int)>(std::tolower) );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this doesn't look good to me...

@philsquared philsquared closed this Dec 9, 2015
@philsquared
Copy link
Collaborator

Apologies for the premature closure - I deleted the branch and it auto-closed all associated PRs - it wasn't intentional.
I also can't re-open it at this point - but I'll come back and review shortly.

@ghost ghost mentioned this pull request Dec 17, 2015
@ghost
Copy link
Author

ghost commented Dec 21, 2015

I have opened this request again under #559

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

Successfully merging this pull request may close these issues.

5 participants