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

WITH with this causes warning: implicit capture of 'this' with recent clangs #342

Open
SimonKagstrom opened this issue Oct 8, 2024 · 6 comments

Comments

@SimonKagstrom
Copy link

With a recent Apple clang update (Apple clang version 16.0.0 (clang-1600.0.26.3)) we've started getting deprecation warnings for some unit tests, related to how the trompeloeil WITH macro is used:

<source>:27:47: warning: implicit capture of 'this' with a capture default of '=' is deprecated [-Wdeprecated-this-capture]

A short example to reproduce this with regular clang-17 (https://godbolt.org/z/x6YKzb6Gb):

#include <doctest.h>
#include <doctest/trompeloeil.hpp>

using trompeloeil::_;

namespace
{

class Fixture
{
public:
    MAKE_MOCK1(MyMock, void(int));
    int my_side_effect = 0;

    int Helper(int i)
    {
        return i;
    }
};

} // namespace


TEST_CASE_FIXTURE(Fixture, "WITH side effect test")
{
    // error: implicit capture of 'this' with a capture default of '=' is deprecated [-Werror,-Wdeprecated-this-capture]
    REQUIRE_CALL(*this, MyMock(_)).WITH(_1 == Helper(_1));
    // Works:
    // REQUIRE_CALL(*this, MyMock(_)).LR_WITH(_1 == Helper(_1));

    MyMock(5);
}

As in the example, we can use LR_WITH to correct this. However, capturing this with [&] seems unnecessary, so is there some trompeloeil change that should be done for this, or should we start using LR_WITH for these?

@rollbear
Copy link
Owner

rollbear commented Oct 8, 2024

I am surprised that it has taken this long for anyone to report this. Yes, the implicit capture of this with [=] is deprecated since C++20. Unfortunately I really don't know what to do about it. This problem arises when test frameworks implement tests in member functions. A way around this would be to change the macro to capture [=,this], but then sources wouldn't compile for test frameworks that do not implement tests as member functions since there is no this to capture, and those that do implement tests as member functions would still give warnings if the test doesn't in fact access any member.

Suggestions are very welcome.

@SimonKagstrom
Copy link
Author

Would it be possible to special-case this for e.g., doctest and catch2 (I guess), if doctest/trompeloeil.hpp is included? I.e., with some e.g., ifdef so [=,this] in that case?

For other people encountering this and using cmake+conan, I've worked around the issue by disabling the warning for Trompeloeil and it's dependencies:

target_compile_options(trompeloeil::trompeloeil
INTERFACE
    -Wno-deprecated-this-capture
)

@rollbear
Copy link
Owner

rollbear commented Oct 9, 2024

Maybe possible, but not easily. Catch2, and I believe doctest also, only implements tests as member functions when you use fixtures, otherwise they are freestanding functions. So the macro would have to expand to different things depending on where it is expanded.

@SimonKagstrom
Copy link
Author

Looking at the metaprogramming library and constraints and concept pages on cppreference, I get a feeling there's a solution to this hiding there somewhere. :-)

@AndrewPaxie
Copy link
Collaborator

AndrewPaxie commented Oct 9, 2024

How does a C++ library writer respond to an existential threat from the C++ committee? This warning could become a hard error as soon as C++26. Nobody said breaking backward compatibility in C++ would make life easier for library writers! /rant

@SimonKagstrom : I believe there is too, starting with whether decltype(EXPR) is well-formed or not in the same context the WITH appears, where EXPR is the argument to WITH. SFINAE might tell us which expressions are ill-formed: those are the expressions that might work if a lambda capture containing this is provided. The well-formed ones get a lambda with just [=] capture as current code has. There is the case where the EXPR remains ill-formed even after trying to form a lambda with a [=,this] capture, but those WITH clauses are ill-formed in the current implementation anyway. All this mechanism is active for C++20 or later. Business as usual for previous language dialects.

Of course we remain reliant on C++ compiler maintainers to write conforming implementations, that is not to remove support for a construct for previous language dialects "by accident", but that has always been the a part of this life. /rant

Sorry to be so sketchy here, I would have liked to present an implementation. I am hopeful @rollbear or yourself can see what I am talking about.

@rollbear
Copy link
Owner

My current thoughts are on introducing yet a new set of macros, with [=,this] captures. I'm not sure how to name them, though. For WITH(), it'll work out nicely as WITH_THIS(), but less great for SIDE_EFFECT_THIS() and especially poorly for RETURN_THIS().

Naming suggestions?

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

3 participants