trompeloeil icon indicating copy to clipboard operation
trompeloeil copied to clipboard

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

Open SimonKagstrom opened this issue 1 year ago • 7 comments

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?

SimonKagstrom avatar Oct 08 '24 09:10 SimonKagstrom

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.

rollbear avatar Oct 08 '24 20:10 rollbear

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
)

SimonKagstrom avatar Oct 09 '24 08:10 SimonKagstrom

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.

rollbear avatar Oct 09 '24 10:10 rollbear

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. :-)

SimonKagstrom avatar Oct 09 '24 10:10 SimonKagstrom

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.

AndrewPaxie avatar Oct 09 '24 18:10 AndrewPaxie

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?

rollbear avatar Oct 16 '24 22:10 rollbear

Have a look at branch capture_this_copy.

It has new macros, MEM_WITH(), MEM_RETURN(), etc. MEM for "member". I'm not thrilled about the name, so suggestions for alternatives are very welcome. No documentation yet.

Let me know if this works for you.

rollbear avatar Mar 30 '25 20:03 rollbear