Skip to content

Commit

Permalink
Prevent templated path members from accepting args convertible to path.
Browse files Browse the repository at this point in the history 
This forces the non-templated overloads accepting path to be chosen instead
of the templated members that expect arguments converible to Source.

This resolves overload resolution ambiguities, when the argument of a
user-defined type is convertible to path and multiple other types that qualify
as Source. By preferring the conversion to path we avoid testing other
conversion paths that may be ambiguous.

Fixes #326.
  • Loading branch information
@Lastique
Lastique committed Sep 29, 2024
1 parent 5746b3f commit 3134f89
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 24 deletions.
1 change: 1 addition & 0 deletions doc/release_history.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ <h2>1.87.0</h2>
<li>As was announced in 1.84.0, Windows versions prior to 10 are no longer supported.</li>
<li>On Windows, <code>canonical</code> is now based on the <code>GetFinalPathNameByHandleW</code> WinAPI function. As a side effect, drive letters are converted to upper case, which makes the resulting paths more interoperable. (<a href="https://github.com/boostorg/filesystem/issues/325">#325</a>)</li>
<li><b>v4:</b> <code>canonical</code> no longer produces a trailing directory separator in the resulting path, if the input path has one.</li>
<li>If a <code>path</code> constructor or member function is called with an argument of a user-defined type that is convertible to <code>path</code> and one or more <a href="reference.html#Source"><code>Source</code></a> types, the conversion to <code>path</code> is now chosen by default. This may resolve argument conversion ambiguities in some cases, but may also result in a less optimal conversion path. If a different conversion path is desired, users are recommended to use explicit type casts. (<a href="https://github.com/boostorg/filesystem/issues/326">#326</a>)</li>
</ul>

<h2>1.86.0</h2>
Expand Down
45 changes: 37 additions & 8 deletions include/boost/filesystem/detail/path_traits.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include <boost/assert.hpp>
#include <boost/system/error_category.hpp>
#include <boost/iterator/is_iterator.hpp>
#include <boost/filesystem/detail/type_traits/negation.hpp>
#include <boost/filesystem/detail/type_traits/conjunction.hpp>
#if defined(BOOST_FILESYSTEM_DETAIL_CXX23_STRING_VIEW_HAS_IMPLICIT_RANGE_CTOR)
#include <boost/filesystem/detail/type_traits/disjunction.hpp>
Expand All @@ -48,6 +49,7 @@ namespace filesystem {

BOOST_FILESYSTEM_DECL system::error_category const& codecvt_error_category() noexcept;

class path;
class directory_entry;

namespace detail {
Expand Down Expand Up @@ -501,16 +503,34 @@ no_type check_convertible(...);

} // namespace is_convertible_to_path_source_impl

//! The type trait indicates whether the type has a conversion path to one of the path source types
template< typename T >
struct is_convertible_to_path_source :
struct check_is_convertible_to_path_source :
public std::integral_constant<
bool,
sizeof(is_convertible_to_path_source_impl::check_convertible(std::declval< T const& >())) == sizeof(yes_type)
>
{
};

/*!
* \brief The type trait indicates whether the type has a conversion path to one of the path source types.
*
* \note The type trait returns `false` if the type is convertible to `path`. This prevents testing other
* conversion paths and forces the conversion to `path` to be chosen instead, to invoke a non-template
* member of `path` accepting a `path` argument.
*/
template< typename T >
struct is_convertible_to_path_source :
public std::integral_constant<
bool,
detail::conjunction<
detail::negation< std::is_convertible< T, path > >,
check_is_convertible_to_path_source< T >
>::value
>
{
};

#else // !defined(BOOST_FILESYSTEM_DETAIL_CXX23_STRING_VIEW_HAS_IMPLICIT_RANGE_CTOR)

// Note: We use separate checks for convertibility to std::string_view and other types to avoid ambiguity with an implicit range constructor
Expand All @@ -529,7 +549,7 @@ no_type check_convertible(...);
} // namespace is_convertible_to_std_string_view_impl

template< typename T >
struct is_convertible_to_std_string_view :
struct check_is_convertible_to_std_string_view :
public std::integral_constant<
bool,
sizeof(is_convertible_to_std_string_view_impl::check_convertible(std::declval< T const& >())) == sizeof(yes_type)
Expand All @@ -553,22 +573,31 @@ no_type check_convertible(...);
} // namespace is_convertible_to_path_source_non_std_string_view_impl

template< typename T >
struct is_convertible_to_path_source_non_std_string_view :
struct check_is_convertible_to_path_source_non_std_string_view :
public std::integral_constant<
bool,
sizeof(is_convertible_to_path_source_non_std_string_view_impl::check_convertible(std::declval< T const& >())) == sizeof(yes_type)
>
{
};

//! The type trait indicates whether the type has a conversion path to one of the path source types
/*!
* \brief The type trait indicates whether the type has a conversion path to one of the path source types.
*
* \note The type trait returns `false` if the type is convertible to `path`. This prevents testing other
* conversion paths and forces the conversion to `path` to be chosen instead, to invoke a non-template
* member of `path` accepting a `path` argument.
*/
template< typename T >
struct is_convertible_to_path_source :
public std::integral_constant<
bool,
detail::disjunction<
is_convertible_to_std_string_view< T >,
is_convertible_to_path_source_non_std_string_view< T >
detail::conjunction<
detail::negation< std::is_convertible< T, path > >,
detail::disjunction<
check_is_convertible_to_std_string_view< T >,
check_is_convertible_to_path_source_non_std_string_view< T >
>
>::value
>
{
Expand Down
76 changes: 60 additions & 16 deletions test/path_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,43 @@ class convertible_to_path
operator fs::path() const { return m_path; }
};

//! Test type to verify that the conversion to path is preferred (https://github.com/boostorg/filesystem/issues/326)
class convertible_to_path_and_strings
{
private:
fs::path m_path;

public:
convertible_to_path_and_strings() {}
convertible_to_path_and_strings(convertible_to_path_and_strings const& that) : m_path(that.m_path) {}
template< typename T >
convertible_to_path_and_strings(T const& that) : m_path(that) {}

convertible_to_path_and_strings& operator= (convertible_to_path_and_strings const& that)
{
m_path = that.m_path;
return *this;
}
template< typename T >
convertible_to_path_and_strings& operator= (T const& that)
{
m_path = that;
return *this;
}

operator fs::path() const { return m_path; }
operator const fs::path::value_type*() const
{
#if defined(BOOST_WINDOWS_API)
return L"[invalid path]";
#else
return "[invalid path]";
#endif
}
operator fs::path::string_type() const { return fs::path::string_type(static_cast< const fs::path::value_type* >(*this)); }
};


template< typename Char >
class basic_custom_string
{
Expand Down Expand Up @@ -2188,6 +2225,7 @@ void construction_tests()

PATH_TEST_EQ(derived_from_path("foo"), "foo");
PATH_TEST_EQ(convertible_to_path("foo"), "foo");
PATH_TEST_EQ(convertible_to_path_and_strings("foo"), "foo");
PATH_TEST_EQ(fs::path(pcustom_string("foo")), "foo");
PATH_TEST_EQ(boost::string_view("foo"), "foo");
#if !defined(BOOST_NO_CXX17_HDR_STRING_VIEW)
Expand All @@ -2204,9 +2242,9 @@ void construction_tests()

#if !defined(BOOST_NO_CXX17_HDR_STRING_VIEW)
#define APPEND_TEST_STD_STRING_VIEW(appnd, expected)\
path p6(p);\
p6 /= std::string_view(appnd);\
PATH_TEST_EQ(p6, expected);
path p7(p);\
p7 /= std::string_view(appnd);\
PATH_TEST_EQ(p7, expected);
#else
#define APPEND_TEST_STD_STRING_VIEW(appnd, expected)
#endif
Expand All @@ -2229,15 +2267,18 @@ void construction_tests()
p3 /= convertible_to_path(appnd);\
PATH_TEST_EQ(p3, expected);\
path p4(p);\
p4 /= pcustom_string(appnd);\
p4 /= convertible_to_path_and_strings(appnd);\
PATH_TEST_EQ(p4, expected);\
path p5(p);\
p5 /= boost::string_view(appnd);\
p5 /= pcustom_string(appnd);\
PATH_TEST_EQ(p5, expected);\
path p6(p);\
p6 /= boost::string_view(appnd);\
PATH_TEST_EQ(p6, expected);\
APPEND_TEST_STD_STRING_VIEW(appnd, expected)\
path p7(p);\
p7.append(s.begin(), s.end());\
PATH_TEST_EQ(p7.string(), expected);\
path p8(p);\
p8.append(s.begin(), s.end());\
PATH_TEST_EQ(p8.string(), expected);\
}

void append_tests()
Expand Down Expand Up @@ -2350,9 +2391,9 @@ void append_tests()

#if !defined(BOOST_NO_CXX17_HDR_STRING_VIEW)
#define CONCAT_TEST_STD_STRING_VIEW(appnd, expected)\
path p9(p);\
p9 += std::string_view(appnd);\
PATH_TEST_EQ(p9, expected);
path p10(p);\
p10 += std::string_view(appnd);\
PATH_TEST_EQ(p10, expected);
#else
#define CONCAT_TEST_STD_STRING_VIEW(appnd, expected)
#endif
Expand Down Expand Up @@ -2380,15 +2421,18 @@ void append_tests()
p6 += convertible_to_path(appnd);\
PATH_TEST_EQ(p6, expected);\
path p7(p);\
p7 += pcustom_string(appnd);\
p7 += convertible_to_path_and_strings(appnd);\
PATH_TEST_EQ(p7, expected);\
path p8(p);\
p8 += boost::string_view(appnd);\
p8 += pcustom_string(appnd);\
PATH_TEST_EQ(p8, expected);\
path p9(p);\
p9 += boost::string_view(appnd);\
PATH_TEST_EQ(p9, expected);\
CONCAT_TEST_STD_STRING_VIEW(appnd, expected)\
path p10(p);\
p10.concat(s.begin(), s.end());\
PATH_TEST_EQ(p10.string(), expected);\
path p11(p);\
p11.concat(s.begin(), s.end());\
PATH_TEST_EQ(p11.string(), expected);\
}

void concat_tests()
Expand Down

0 comments on commit 3134f89

Please sign in to comment.