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

Add & to REQUIRE_THROWS_AS type? #542

Closed
joto opened this issue Dec 1, 2015 · 1 comment
Closed

Add & to REQUIRE_THROWS_AS type? #542

joto opened this issue Dec 1, 2015 · 1 comment

Comments

@joto
Copy link

joto commented Dec 1, 2015

Exceptions should generally be caught by reference (catch(some_type&)). But if you give a simple type to REQUIRE_THROWS_AS({...}, some_type);, the macro will generate a catch by value (catch(some_type)). Not really a problem, but clang-tidy complains. Of course I can just use references myself, but it is not clear to the user that he always has to do this. Maybe we could change the INTERNAL_CATCH_THROWS_AS macro):

-            catch( exceptionType ) { \
+            catch( std::add_lvalue_reference<exceptionType>::type ) { \

Unfortunately this solution only works in C++11.

@nabijaczleweli
Copy link
Contributor

add_*_reference<> can be trivially implemented in usercode, so you could just have:

namespace catch {
#if cpluspluseleven
  template<class T>
  using add_lvalue_reference = std::add_lvalue_reference<T>;
#else
  template <typename T>
  struct add_lvalue_reference {
    typedef T & type;
  };
  template <typename T>
  struct add_lvalue_reference<T &> {
    typedef T & type;
  };
  template <typename T>
  struct add_lvalue_reference<T &&> {
    typedef T & type;
  };
#endif
}

To test:

CHECK(std::is_same<catch::add_lvalue_reference<int &&>::type, int &>::value);
CHECK(std::is_same<catch::add_lvalue_reference<int &>::type, int &>::value);
CHECK(std::is_same<catch::add_lvalue_reference<int>::type, int &>::value);

horenmar added a commit that referenced this issue Mar 23, 2017
Effectively a revert of previous commit, fixing #542, where this was
added to stop linters complaining about `REQUIRE_THROWS_AS` used like
`REQUIRE_THROWS_AS(expr, std::exception);`, which would be slicing the
caught exception. Now it is user's responsibility to pass us proper
exception type.

Closes #833 which wanted to add `typename`, so that the construct works
in a template, but that would not work with MSVC and older GCC's, as
having `typename` outside of a template is allowed only from C++11
onward.
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