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

core/swap.hpp(81,1): fatal error C1202: recursive type or function dependency context too complex #148

Closed
afabri opened this issue Jul 5, 2023 · 14 comments · Fixed by #149

Comments

@afabri
Copy link

afabri commented Jul 5, 2023

We get a boost_1_82_0\boost/core/swap.hpp(81,1): fatal error C1202: recursive type or function dependency context too complex when compiling this program with VC2019 or VC2022 and stdc++17 enabled. link
Neither g++ nor clang have a problem.
The code is nonsensical and we only boiled down the real code to make it a minimal example. Also #include <boost/swap.hpp> is not used but only needed for getting the error.
Maybe this is more a VC++ issue than a boost issue.

@afabri
Copy link
Author

afabri commented Jul 7, 2023

Hi @Lastique
Do you by any chance have access to the VC++ compiler? I ask because your commit concerning the noexception triggers the problem.

@Lastique
Copy link
Member

Lastique commented Jul 7, 2023

Do you by any chance have access to the VC++ compiler?

Not at the moment or in the coming few weeks.

If you are willing to try something, you could try the following:

  1. Envelop boost::swap definition here in a new namespace within namespace boost, say namespace swap_adl_block.
  2. After that namespace closes, import boost::swap_adl_block::swap into namespace boost by adding using swap_adl_block::swap declaration.
  3. Try if it fixes your problem.

@pdimov
Copy link
Member

pdimov commented Jul 9, 2023

Reduced example:

#include <boost/swap.hpp> // compiles if removed
#include <boost/checked_delete.hpp>

template<class T> class X
{
    X( X const& );

public:

    X() {}
};

int main()
{
    X< boost::checked_deleter<int> > x;
    return noexcept( swap( x, x ) );
}

Fails under all compilers (e.g. https://godbolt.org/z/ea65PWz9b). The reason is that std::swap is SFINAEd away when the type isn't move constructible, so the unqualified swap call ends up calling boost::swap recursively, forever.

@pdimov
Copy link
Member

pdimov commented Jul 9, 2023

Why did we need the change in 8a8738a?

@Lastique
Copy link
Member

Lastique commented Jul 9, 2023 via email

@pdimov
Copy link
Member

pdimov commented Jul 9, 2023

To prevent boost::swap from being found by ADL, we'll have to make it a function object. That might be the right thing to do, but it's a significant change and might have other unintended consequences.

@Lastique
Copy link
Member

Lastique commented Jul 9, 2023

Reduced example:

#include <boost/swap.hpp> // compiles if removed
#include <boost/checked_delete.hpp>

template<class T> class X
{
    X( X const& );

public:

    X() {}
};

int main()
{
    X< boost::checked_deleter<int> > x;
    return noexcept( swap( x, x ) );
}

Fails under all compilers (e.g. https://godbolt.org/z/ea65PWz9b). The reason is that std::swap is SFINAEd away when the type isn't move constructible, so the unqualified swap call ends up calling boost::swap recursively, forever.

This code is incorrect either way. If boost::swap is found via ADL, the call is recursive and fails due to noexcept. Before the addition of noexcept, it would compile into an infinite recursion in runtime. If the boost::swap is not found via ADL, then there is no swap that accepts X - std::swap requires the type to be copyable.

I'm not sure this repro is equivalent to the code posted by the OP. Their code involves std::tuple, and without seeing its implementation I can't tell what triggers swap lookup and why it fails specifically on MSVC and not other compilers. I'm assuming, the OP's code compiles without noexcept specification and fails with it.

Lastique added a commit that referenced this issue Jul 9, 2023
… ADL.

This resolves infinite recursion when boost::swap is called on types that
are defined in namespace boost. The swap_impl implementation and noexcept
specification is using an unqualified call to swap, so if the argument types
are defined in namespace boost then our boost::swap function may be found
and selected to resolve the call.

Fixes #148.
@Lastique
Copy link
Member

Lastique commented Jul 9, 2023

@afabri Could you check if 987ffb3 resolves the problem for you?

@Lastique
Copy link
Member

Lastique commented Jul 9, 2023

To prevent boost::swap from being found by ADL, we'll have to make it a function object.

Not necessarily, a using directive would do.

@pdimov
Copy link
Member

pdimov commented Jul 9, 2023

My guess is that it uses std::is_nothrow_swappable somewhere.

Not necessarily, a using directive would do.

I don't think it would.

Lastique added a commit that referenced this issue Jul 9, 2023
… ADL.

This resolves infinite recursion when boost::swap is called on types that
are defined in namespace boost. The swap_impl implementation and noexcept
specification is using an unqualified call to swap, so if the argument types
are defined in namespace boost then our boost::swap function may be found
and selected to resolve the call.

Fixes #148.
Lastique added a commit that referenced this issue Jul 9, 2023
… ADL.

This resolves infinite recursion when boost::swap is called on types that
are defined in namespace boost. The swap_impl implementation and noexcept
specification is using an unqualified call to swap, so if the argument types
are defined in namespace boost then our boost::swap function may be found
and selected to resolve the call.

Fixes #148.
@Lastique
Copy link
Member

Lastique commented Jul 9, 2023

Not necessarily, a using directive would do.

I don't think it would.

It does work in my local testing, but it turns out it has unexpected side effect that shows as swap_std_vector_of_boost test failure. Basically, if there is an overload of boost::swap for some type (besides Boost.Swap) then the boost::swap from Boost.Swap is no longer found, even if that overload is not suitable for the call arguments. I believe, we would have had the same problem with the function object.

I don't see a way around this. Would we be willing to ban swap overloads in namespace boost other than Boost.Swap, Boost-wide?

@pdimov
Copy link
Member

pdimov commented Jul 9, 2023

Would we be willing to ban swap overloads in namespace boost other than Boost.Swap, Boost-wide?

I don't think we can do that. Types defined in namespace boost have to have their swap functions defined there as well.

@Lastique
Copy link
Member

Would we be willing to ban swap overloads in namespace boost other than Boost.Swap, Boost-wide?

I don't think we can do that. Types defined in namespace boost have to have their swap functions defined there as well.

The idea was to move those types to their own namespaces within boost and then import them into boost with using declarations. But maybe that's too much to ask of maintainers.

Another solution would be to rename swap from Boost.Swap e.g. to adl_swap or move it to a namespace of its own (any naming suggestions?). For backward compatibility, we would keep the boost::swap name available (as a forwarding function or a using declaration) but marked as deprecated, which we would eventually remove. The problem would persist until we remove it.

@pdimov
Copy link
Member

pdimov commented Jul 10, 2023

We can add adl_swap or whatever with proper noexcept forwarding, and leave boost::swap as it was (in addition to deprecating it) to avoid the regression.

boost::core::swap would be the obvious candidate but we can't do that because there are types in boost::core for which it will be found by ADL. So maybe just changing the name to something other than swap would be best. Maybe swap_adl or invoke_swap. (Or even boost::core::invoke_swap.)

Lastique added a commit that referenced this issue Jul 10, 2023
The rename allows to avoid forming an infinite recursion in compile time
(because of noexcept specification that needs to resolve the unqualified call
to swap) or run time (in case if the boost::swap function is the only one
suitable for the passed arguments).

To avoid the compile-time recursion, removed noexcept specification from
boost::swap. The specification is still present in boost::core::invoke_swap.

Deprecated boost::swap and associated headers. boost::core::invoke_swap
is defined in a new boost/core/invoke_swap.hpp header.

Updated docs and tests. Removed tests that check inclusion of deprecated
headers.

Fixes #148.
Lastique added a commit that referenced this issue Jul 12, 2023
The rename allows to avoid forming an infinite recursion in compile time
(because of noexcept specification that needs to resolve the unqualified call
to swap) or run time (in case if the boost::swap function is the only one
suitable for the passed arguments).

To avoid the compile-time recursion, removed noexcept specification from
boost::swap. The specification is still present in boost::core::invoke_swap.

Deprecated boost::swap and associated headers. boost::core::invoke_swap
is defined in a new boost/core/invoke_swap.hpp header.

Updated docs and tests. Removed tests that check inclusion of deprecated
headers.

Fixes #148.
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 a pull request may close this issue.

3 participants