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.

Sign up for GitHub

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

Jump to bottom

REQUIRE does not compile when operator== in different namespace #443 . #468

Merged
merged 4 commits into from
Mar 21, 2021
Merged

REQUIRE does not compile when operator== in different namespace #443 . #468

merged 4 commits into from
Mar 21, 2021

Conversation

navinp0304
Copy link
Contributor

@navinp0304 navinp0304 commented Feb 11, 2021 •
edited by onqtam
Loading

Description

  • This was a valid test case when you have global operator the comparision succeeds but doctest fails.
  • This is due to forced instantiation of template member function Expression_lhs.operator==(const R& rhs).
  • The template member function contains lhs == rhs for equality operator. This comparison fails since the global operator is defined after the template and it doesn't take part in lookup.
  • In that case the function itself is not instantiated due to sfinae and the global operator is visible and due to user defined conversion the operator succeeds.

GitHub Issues

Closes #443

@navinp0304
REQUIRE does not compile when operator== in different namespace #443 .
1195a52 
Expression_lhs.op member method is not instantiated when it is missing
a member operator and the user defined conversion is able to apply the
global operator.
@onqtam
Copy link
Member

onqtam commented Feb 18, 2021 •
edited
Loading

This solution adds SFINAE on the common instantiation path for all asserts and that would slow compile times - I'll leave this PR open for now and decide a bit later what to do with it.

I also wonder if declval can be implemented using only C++ without including <utility> - perhaps something like what's posted here. Not including any headers would be really great.

navinp0304
navinp0304 commented Feb 19, 2021
View reviewed changes
Copy link
Contributor Author

@navinp0304 navinp0304 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I also wonder if declval can be implemented using only C++ without including - perhaps something like what's posted here. Not including any headers would be really great.

I removed utility and used the faster version of declval as per the link. Please pull the changes.

@navinp0304
Copy link
Contributor Author

I also wonder if declval can be implemented using only C++ without including - perhaps something like what's posted here. Not including any headers would be really great.

I removed utility and used the faster version of declval as per the link. Please pull the changes.

@navinp0304
Pulling from onqtam INFO changes and others.
870d4d8 
Merge branch 'dev' of https://github.com/onqtam/doctest into dev
@navinp0304
Using templated operator== inside TEST_CASE changes deduced types of …
4cd696c 
…forwarding references #399 . This is fixed by using rvalues as function argument and using forward for the right type of reference. Now both gcc and doctest either fails or either compiles but not like one compiles and the other fails
@onqtam
Copy link
Member

onqtam commented Feb 23, 2021

sorry for the delay - will look at this again soon.

@navinp0304
Copy link
Contributor Author

navinp0304 commented Feb 23, 2021 •
edited
Loading

sorry for the delay - will look at this again soon.

This change is on top of the #443.
This fixes #399 as well. Using rvalue refs than existing lvalue in func arguments.

@onqtam
Copy link
Member

onqtam commented Mar 21, 2021

Thanks for this - I'll merge it as it is and fix 1 tiny thing later on (doctest already has remove_reference in namespace detail)

@onqtam onqtam merged commit 01546b2 into doctest:dev Mar 21, 2021
This was referenced Mar 21, 2021
Closed
Closed
onqtam added a commit that referenced this pull request Mar 21, 2021
@onqtam
fixing clang 3.5 build after merging PR #468
31cd8b2
onqtam added a commit that referenced this pull request Mar 21, 2021
@onqtam
moving the implementation of declval into the detail namespace inside…
3acf667 
… doctest - this is a fixup after merging #468
onqtam pushed a commit that referenced this pull request Mar 22, 2021
@navinp0304 @onqtam
REQUIRE does not compile when operator== in different namespace #443 . (
318e1df 
#468)

* REQUIRE does not compile when operator== in different namespace #443 .
Expression_lhs.op member method is not instantiated when it is missing
a member operator and the user defined conversion is able to apply the
global operator.

* Removing utility and using an overloaded version of declval which is faster in doctest_fwd.h .

* Using templated operator== inside TEST_CASE changes deduced types of forwarding references #399 . This is fixed by using rvalues as function argument and using forward for the right type of reference. Now both gcc and doctest either fails or either compiles but not like one compiles and the other fails
onqtam added a commit that referenced this pull request Mar 22, 2021
@onqtam
fixing clang 3.5 build after merging PR #468
7a01409
onqtam added a commit that referenced this pull request Mar 22, 2021
@onqtam
moving the implementation of declval into the detail namespace inside…
62855a1 
… doctest - this is a fixup after merging #468
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@navinp0304 @onqtam