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

[Sema][CTAD] Allow user defined conversion for copy-list-initialization #94752

Merged
merged 5 commits into from
Jun 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,12 @@ C++20 Feature Support
to update the ``__cpp_concepts`` macro to `202002L`. This enables
``<expected>`` from libstdc++ to work correctly with Clang.

- User defined constructors are allowed for copy-list-initialization with CTAD.
The example code for deduction guides for std::map in
(`cppreference <https://en.cppreference.com/w/cpp/container/map/deduction_guides>`_)
will now work.
(#GH62925).

Comment on lines +202 to +207
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure the link adds useful information. If you think the example is useful, feel free to add the examples in the release note directly (but the issue link is enough)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will change it so only the issue link is there.

C++23 Feature Support
^^^^^^^^^^^^^^^^^^^^^

Expand Down
2 changes: 1 addition & 1 deletion clang/include/clang/Sema/Initialization.h
Original file line number Diff line number Diff line change
Expand Up @@ -603,7 +603,7 @@ class InitializationKind {
/// Normal context
IC_Normal,

/// Normal context, but allows explicit conversion functionss
/// Normal context, but allows explicit conversion functions
IC_ExplicitConvs,

/// Implicit context (value initialization)
Expand Down
6 changes: 2 additions & 4 deletions clang/lib/Sema/SemaInit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10892,8 +10892,6 @@ QualType Sema::DeduceTemplateSpecializationFromInitializer(
// FIXME: The "second phase of [over.match.list] case can also
// theoretically happen here, but it's not clear whether we can
// ever have a parameter of the right type.
Comment on lines 10892 to 10894
Copy link
Contributor

Choose a reason for hiding this comment

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

Is that comment still useful?

Copy link
Contributor Author

@spaits spaits Jun 20, 2024

Choose a reason for hiding this comment

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

It wasn't useful (or at least I could definitely not understand why it was there) even when I was debugging to find the solution. Since I did not understand it I did not touch it.

What do you think, could we remove it?

bool SuppressUserConversions = Kind.isCopyInit();

if (TD) {
SmallVector<Expr *, 8> TmpInits;
for (Expr *E : Inits)
Expand All @@ -10903,12 +10901,12 @@ QualType Sema::DeduceTemplateSpecializationFromInitializer(
TmpInits.push_back(E);
AddTemplateOverloadCandidate(
TD, FoundDecl, /*ExplicitArgs=*/nullptr, TmpInits, Candidates,
SuppressUserConversions,
/*SuppressUserConversions=*/false,
/*PartialOverloading=*/false, AllowExplicit, ADLCallKind::NotADL,
/*PO=*/{}, AllowAggregateDeductionCandidate);
} else {
AddOverloadCandidate(GD, FoundDecl, Inits, Candidates,
SuppressUserConversions,
/*SuppressUserConversions=*/false,
/*PartialOverloading=*/false, AllowExplicit);
}
};
Expand Down
46 changes: 46 additions & 0 deletions clang/test/SemaCXX/ctad-copy-init-list-narrowing.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// RUN: %clang_cc1 -fsyntax-only -verify -Wno-unused-value -std=c++20 %s
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: please format the test file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Applied clang format, nothing has really changed. I have changed the formatting by hand at some places.

Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah, that's expected, we disable the clang-format for all lit test files (see the clang/test/.clang-format).


namespace std {
typedef decltype(sizeof(int)) size_t;

template <typename E>
struct initializer_list {
const E *p;
size_t n;
initializer_list(const E *p, size_t n) : p(p), n(n) {}
};

// Classes to use to reproduce the exact scenario present in #62925.
template<class T, class Y>
class pair {
public:
pair(T f, Y s) {}
};

template<class T, class Y>
class map {
public:
map(std::initializer_list<pair<T, Y>>) {}
map(std::initializer_list<pair<T, Y>>, int a) {}
};

} // namespace std

// This is the almost the exact code that was in issue #62925.
void testOneLevelNesting() {
std::map mOk = {std::pair{5, 'a'}, {6, 'b'}, {7, 'c'}};

// Verify that narrowing conversion is disabled in the first level of nesting.
std::map mNarrow = {std::pair{5, 'a'}, {6.0f, 'b'}, {7, 'c'}}; // expected-error {{type 'float' cannot be narrowed to 'int' in initializer list}} // expected-note {{insert an explicit cast to silence this issue}}
}

void testMultipleLevelNesting() {
std::map aOk = {{std::pair{5, 'c'}, {5, 'c'}}, 5};

// Verify that narrowing conversion is disabled when it is not in a nested
// in another std::initializer_list, but it happens in the most outer one.
std::map aNarrowNested = {{std::pair{5, 'c'}, {5.0f, 'c'}}, 5}; // expected-error {{type 'float' cannot be narrowed to 'int' in initializer list}} // expected-note {{insert an explicit cast to silence this issue}}

// Verify that narrowing conversion is disabled in the first level of nesting.
std::map aNarrow = {{std::pair{5, 'c'}, {5, 'c'}}, 5.0f}; // expected-error {{type 'float' cannot be narrowed to 'int' in initializer list}} // expected-note {{insert an explicit cast to silence this issue}}
}
Loading