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

Jump to bottom

[libc++] std::bitset has non-uglified member typedefs size_type, difference_type, and const_reference causing ambiguity in name lookup #121618

Open
winner245 opened this issue Jan 4, 2025 · 11 comments · May be fixed by #121664
Open

[libc++] std::bitset has non-uglified member typedefs size_type, difference_type, and const_reference causing ambiguity in name lookup #121618

winner245 opened this issue Jan 4, 2025 · 11 comments · May be fixed by #121664
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Comments

@winner245
Copy link
Contributor

winner245 commented Jan 4, 2025 •
edited
Loading

Due to the presence of non-uglified member typedef-names in std::bitset, the following program fails to compile in libc++ (while it compiles successfully in libstdc++ and MSVC):

https://godbolt.org/z/4EGY13eah

#include <bitset>

struct MyBase {
  using const_reference = const int&;
  using size_type = unsigned;
  using difference_type = int;
};

struct MyDerived : MyBase, std::bitset<42> {};

int main() {
  MyDerived::const_reference r{};
  MyDerived::size_type s{};
  MyDerived::difference_type d{};
}

Affected Lines:

llvm-project/libcxx/include/bitset

Lines 170 to 174 in e3dafa8

template <size_t _N_words, size_t _Size>
class __bitset {
public:
typedef ptrdiff_t difference_type;
typedef size_t size_type;

llvm-project/libcxx/include/bitset

Lines 433 to 436 in e3dafa8

class __bitset<1, _Size> {
public:
typedef ptrdiff_t difference_type;
typedef size_t size_type;

llvm-project/libcxx/include/bitset

Lines 550 to 553 in e3dafa8

class __bitset<0, 0> {
public:
typedef ptrdiff_t difference_type;
typedef size_t size_type;

llvm-project/libcxx/include/bitset

Line 622 in e3dafa8

typedef typename __base::const_reference const_reference;

Proposed Solution:

In my opinion, if the standard does not specify a member typedef, the library should try to avoid or minimize the introduction of extra typedefs, especially public ones, to prevent potential clashes with user-defined classes, as demonstrated above. From the user's perspective, they have the freedom to utilize any member typedefs not specified by the C++ standards in their own code.

If extra typedefs are deemed necessary, I recommend using a naming convention like __ugly_name for those not specified in the standards, as we have previously implemented.
@winner245 winner245 changed the title std::bitset has non-conforming member typedefs size_type, difference_type, and const_reference [libc++] std::bitset has non-conforming member typedefs size_type, difference_type, and const_reference Jan 4, 2025
@EugeneZelenko EugeneZelenko added libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. and removed new issue labels Jan 4, 2025
@winner245 winner245 mentioned this issue Jan 4, 2025
Open
@philnik777
Copy link
Contributor

Which part of the standard are we not conforming to here?

@winner245
Copy link
Contributor Author

@philnik777 According to [template.bitset.general], std::bitset is supposed to have only one public member typedef, reference, which indicates that the public typedef const_reference currently available in libc++'s bitset implementation is non-conforming. The other two typdefs, size_type and difference_type (despite being private), could cause failure in name lookup, as reported earlier due to similar private member typedefs such as base (#80706, #112843) and iterator/const_iterator (#111127).

@philnik777
Copy link
Contributor

@philnik777 According to [template.bitset.general], std::bitset is supposed to have only one public member typedef, reference, which indicates that the public typedef const_reference currently available in libc++'s bitset implementation is non-conforming.

No, that describes what public members std::bitset should have. It doesn't say that these should be the only members. I don't see any justification for why they think it's non-conforming in the linked issues either. Maybe @frederick-vs-ja can provide something that says this is non-conforming.

@winner245
Copy link
Contributor Author

In my opinion, if the standard does not specify a member typedef, the library should try to avoid or minimize the introduction of extra typedefs, especially public ones, to prevent clashes with user-defined classes. From the user's perspective, they have the freedom to utilize any member typedefs not specified by the C++ standards in their own code, which raises my concern. If extra typedefs are deemed necessary, I recommend using a naming convention like __ugly_name for those not specified in the standards, as we have previously implemented.

@philnik777
Copy link
Contributor

That's a valid concern, but very different from "non-conforming".

@frederick-vs-ja
Copy link
Contributor

[objects.within.classes]/3 is arguably related - "equivalent observable behavior" can mean that no additional conflict on (non-reserved) member names is permitted.

@philnik777
Copy link
Contributor

I don't see how. This is about object representation and exposition-only members. This has nothing to do with names in general.

@winner245
Copy link
Contributor Author

That's a valid concern, but very different from "non-conforming".

I tend to think this is not standard-conforming because it causes ambiguity in name lookup. If it were standard conforming, it shouldn't clash with user code, unless the standard itself is defective. This is my personal thought, and you may disagree.

While I couldn't find a strong argument regarding whether defining additional non-standard member typedefs (I mean member typedefs that are not specified by the standard, in case you disagree with my expression) is standard-conforming or not, I believe we all agree that defining these additional member typedefs could cause potential clashes with name lookup in user-defined classes. Therefore, the issue is valid. However, I will rephrase the issue to focus only on the name lookup ambiguity caused by these non-uglified typedefs.

@winner245 winner245 changed the title [libc++] std::bitset has non-conforming member typedefs size_type, difference_type, and const_reference [libc++] std::bitset has non-uglified member typedefs size_type, difference_type, and const_reference causing ambiguity in name lookup Jan 4, 2025
@frederick-vs-ja
Copy link
Contributor

I don't see how. This is about object representation and exposition-only members. This has nothing to do with names in general.

How about [intro.abstract]/5? I don't think this is relaxed for private class members.

@philnik777
Copy link
Contributor

That's a valid concern, but very different from "non-conforming".

I tend to think this is not standard-conforming because it causes ambiguity in name lookup. If it were standard conforming, it shouldn't clash with user code, unless the standard itself is defective. This is my personal thought, and you may disagree.

Being non-conforming means that the standard says something we're not conforming to. Whether it's a good idea to do something in a specific way if the standard isn't saying what to do is entirely different - that's QoI. Since we test QoI separately from conformance, that's an important distinction for us.

I don't see how. This is about object representation and exposition-only members. This has nothing to do with names in general.

How about [intro.abstract]/5? I don't think this is relaxed for private class members.

Are you throwing paragraphs at the wall until one sticks? This is about execution in the abstract machine and has nothing to do with whether we're allowed to provide additional symbols in classes.

@winner245
Copy link
Contributor Author

Thank you, @philnik777, for clarifying the distinction between conformance and QoI. Based on your feedback, I have rephrased this issue by removing descriptions regarding standard conformance, and we should have no further disagreements. Since we all agree that resolving the ambiguity issue improves QoI, I will proceed to fix it. I appreciate the discussion and the perspectives you've shared.

Thank you, @frederick-vs-ja, for your valuable input and references. Your insights have been instrumental in highlighting the importance of addressing name lookup ambiguities and ensuring our implementation does not cause conflicts with user-defined classes.

@winner245 winner245 linked a pull request Jan 4, 2025 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[libc++] Fix ambiguity due to non-uglified member typedefs winner245/llvm-project
5 participants
@EugeneZelenko @winner245 @frederick-vs-ja @philnik777 @llvmbot