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

[libc++] Fix incomplete user-defined ctype specialization in test #74630

Merged
merged 1 commit into from
Dec 13, 2023
Merged

[libc++] Fix incomplete user-defined ctype specialization in test #74630

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 44 additions & 8 deletions ...ization/locale.categories/category.numeric/locale.num.get/user_defined_char_type.pass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@
#include <locale>
#include <string>

#include "test_macros.h"

struct Char {
Char() = default;
Char(char c) : underlying_(c) {}
Expand Down Expand Up @@ -73,15 +71,53 @@ struct char_traits<Char> {
static int_type eof() { return char_traits<char>::eof(); }
};

// This ctype specialization treats all characters as spaces
template <>
class ctype<Char> : public locale::facet {
class ctype<Char> : public locale::facet, public ctype_base {
Copy link
Member

Choose a reason for hiding this comment

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

I'm getting errors here:

D:\GitHub\STL\llvm-project\libcxx\test\std\localization\locale.categories\category.numeric\locale.num.get\user_defined_char_type.pass.cpp(76,21): error: direct base 'locale::facet' is inaccessible due to ambiguity:
    class std::ctype<Char> -> class locale::facet
    class std::ctype<Char> -> ctype_base -> class locale::facet [-Werror,-Winaccessible-base]
class ctype<Char> : public locale::facet, public ctype_base {
                    ^~~~~~~~~~~~~~~~~~~~

D:\GitHub\STL\out\x64\out\inc\xlocale(454,12): error: ambiguous cast from base 'std::locale::facet' to derived 'std::ctype<Char>':
    class locale::facet -> const class std::ctype<Char>
    class locale::facet -> ctype_base -> const class std::ctype<Char>
    return static_cast<const _Facet&>(*_Pf); // should be dynamic_cast
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I see that your PR is imitating [locale.ctype.general] which depicts template<class charT> class ctype : public locale::facet, public ctype_base. Unfortunately, microsoft/STL is weird - we have ctype_base deriving from locale::facet (which we shouldn't, but fixing it would break ABI), then our ctype derives from only ctype_base.

I'm not sure how to work around this divergence without adding an ifdef for MSVC's STL to this test. I'd be okay with you merging this PR as-is, then I'll look into what else is needed to get it to pass for us.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I noticed that you folks had locale::facet deriving from ctype_base (or the other way around) when I tested this on Godbolt. I think it makes it impossible for users to implement a specialization of ctype in a conforming way, but I understand this is not really something you can fix.

I'd be OK with an #ifdef for this case.

public:
using char_type = Char;
static locale::id id;
Char toupper(Char c) const { return Char(std::toupper(c.underlying_)); }
const char* widen(const char* first, const char* last, Char* dst) const {
for (; first != last;)
*dst++ = Char(*first++);
return last;
explicit ctype(std::size_t refs = 0) : locale::facet(refs) {}

bool is(mask m, char_type) const { return m & ctype_base::space; }
const char_type* is(const char_type* low, const char_type* high, mask* vec) const {
for (; low != high; ++low)
*vec++ = ctype_base::space;
return high;
}

const char_type* scan_is(mask m, const char_type* beg, const char_type* end) const {
for (; beg != end; ++beg)
if (this->is(m, *beg))
return beg;
return end;
}

const char_type* scan_not(mask m, const char_type* beg, const char_type* end) const {
for (; beg != end; ++beg)
if (!this->is(m, *beg))
return beg;
return end;
}

char_type toupper(char_type c) const { return c; }
const char_type* toupper(char_type*, const char_type* end) const { return end; }

char_type tolower(char_type c) const { return c; }
const char_type* tolower(char_type*, const char_type* end) const { return end; }

char_type widen(char c) const { return char_type(c); }
const char* widen(const char* beg, const char* end, char_type* dst) const {
for (; beg != end; ++beg, ++dst)
*dst = char_type(*beg);
return end;
}

char narrow(char_type c, char /*dflt*/) const { return c.underlying_; }
const char_type* narrow(const char_type* beg, const char_type* end, char /*dflt*/, char* dst) const {
for (; beg != end; ++beg, ++dst)
*dst = beg->underlying_;
return end;
}
};

Expand Down
Loading