Skip to content

Possible bug in implementation of P3310 #125290

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

Closed
higher-performance opened this issue Jan 31, 2025 · 2 comments · Fixed by #125372 or #125791
Closed

Possible bug in implementation of P3310 #125290

higher-performance opened this issue Jan 31, 2025 · 2 comments · Fixed by #125372 or #125791
Assignees
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" confirmed Verified by a second party

Comments

@higher-performance
Copy link
Contributor

I think #124137 may have introduced a bug. The following code no longer compiles: https://godbolt.org/z/G77fMj1nh

struct None {};
template<class T>
struct Node { using Tail = T; };

template <template<class> class Fixture, typename List>
struct Recursive {
  static void foo() {
    return Recursive<Fixture, typename List::Tail>::foo();
  }
};

template <template<class> class Fixture>
struct Recursive<Fixture, None> {
  static void foo() {}
};

template <class>
class Monadic {};

template <class...>
class Variadic {};

int main() {
  Recursive<Monadic, Node<None> >::foo();  // success
  Recursive<Variadic, Node<None> >::foo();  // error
}

It seems that the partial specialization struct Recursive<Fixture, None> is completely missed by the compiler now... but only when the Fixture is variadic. I assume this is not supposed to be the case?

@llvmbot
Copy link
Member

llvmbot commented Jan 31, 2025

@llvm/issue-subscribers-clang-frontend

Author: None (higher-performance)

I think #124137 may have introduced a bug. The following code no longer compiles: https://godbolt.org/z/G77fMj1nh
struct None {};
template&lt;class T&gt;
struct Node { using Tail = T; };

template &lt;template&lt;class&gt; class Fixture, typename List&gt;
struct Recursive {
  static void foo() {
    return Recursive&lt;Fixture, typename List::Tail&gt;::foo();
  }
};

template &lt;template&lt;class&gt; class Fixture&gt;
struct Recursive&lt;Fixture, None&gt; {
  static void foo() {}
};

template &lt;class&gt;
class Monadic {};

template &lt;class...&gt;
class Variadic {};

int main() {
  Recursive&lt;Monadic, Node&lt;None&gt; &gt;::foo();  // success
  Recursive&lt;Variadic, Node&lt;None&gt; &gt;::foo();  // error
}

It seems that the partial specialization struct Recursive&lt;Fixture, None&gt; is completely missed by the compiler now... but only when the Fixture is variadic. I assume this is not supposed to be the case?

@mizvekov mizvekov self-assigned this Feb 1, 2025
@mizvekov mizvekov added the confirmed Verified by a second party label Feb 1, 2025
@mizvekov
Copy link
Contributor

mizvekov commented Feb 1, 2025

Yeah, that is not supposed to happen.

There is some plumbing missing here, in this case we instantiate 'Recursive' only when it needs to be complete, but we match the template arguments against the primary template as soon as we see the specialization. The new flag which modifies the overload resolution behavior needs to be plumbed to a different point in time.

mizvekov added a commit that referenced this issue Feb 2, 2025
Class templates might be only instantiated when they are required
to be complete, but checking the template args against the primary
template is immediate.

This result is cached so that later when the class is instantiated,
checking against the primary template is not repeated.

The 'MatchedPackOnParmToNonPackOnArg' flag is also produced upon
checking against the primary template, so it needs to be cached
in the specialziation as well.

This fixes a bug which has not been in any release, so there are no
release notes.

Fixes #125290
mizvekov added a commit that referenced this issue Feb 2, 2025
Class templates might be only instantiated when they are required
to be complete, but checking the template args against the primary
template is immediate.

This result is cached so that later when the class is instantiated,
checking against the primary template is not repeated.

The 'MatchedPackOnParmToNonPackOnArg' flag is also produced upon
checking against the primary template, so it needs to be cached
in the specialziation as well.

This fixes a bug which has not been in any release, so there are no
release notes.

Fixes #125290
mizvekov added a commit that referenced this issue Feb 2, 2025
Class templates might be only instantiated when they are required
to be complete, but checking the template args against the primary
template is immediate.

This result is cached so that later when the class is instantiated,
checking against the primary template is not repeated.

The 'MatchedPackOnParmToNonPackOnArg' flag is also produced upon
checking against the primary template, so it needs to be cached
in the specialziation as well.

This fixes a bug which has not been in any release, so there are no
release notes.

Fixes #125290
mizvekov added a commit that referenced this issue Feb 2, 2025
Class templates might be only instantiated when they are required
to be complete, but checking the template args against the primary
template is immediate.

This result is cached so that later when the class is instantiated,
checking against the primary template is not repeated.

The 'MatchedPackOnParmToNonPackOnArg' flag is also produced upon
checking against the primary template, so it needs to be cached
in the specialziation as well.

This fixes a bug which has not been in any release, so there are no
release notes.

Fixes #125290
mizvekov added a commit that referenced this issue Feb 4, 2025
Class templates might be only instantiated when they are required
to be complete, but checking the template args against the primary
template is immediate.

This result is cached so that later when the class is instantiated,
checking against the primary template is not repeated.

The 'MatchedPackOnParmToNonPackOnArg' flag is also produced upon
checking against the primary template, so it needs to be cached
in the specialziation as well.

This fixes a bug which has not been in any release, so there are no
release notes.

Fixes #125290
mizvekov added a commit that referenced this issue Feb 4, 2025
Class templates might be only instantiated when they are required
to be complete, but checking the template args against the primary
template is immediate.

This result is cached so that later when the class is instantiated,
checking against the primary template is not repeated.

The 'MatchedPackOnParmToNonPackOnArg' flag is also produced upon
checking against the primary template, so it needs to be cached
in the specialziation as well.

This fixes a bug which has not been in any release, so there are no
release notes.

Fixes #125290
mizvekov added a commit that referenced this issue Feb 4, 2025
Class templates might be only instantiated when they are required
to be complete, but checking the template args against the primary
template is immediate.

This result is cached so that later when the class is instantiated,
checking against the primary template is not repeated.

The 'MatchedPackOnParmToNonPackOnArg' flag is also produced upon
checking against the primary template, so it needs to be cached
in the specialziation as well.

This fixes a bug which has not been in any release, so there are no
release notes.

Fixes #125290
mizvekov added a commit that referenced this issue Feb 5, 2025
Class templates might be only instantiated when they are required
to be complete, but checking the template args against the primary
template is immediate.

This result is cached so that later when the class is instantiated,
checking against the primary template is not repeated.

The 'MatchedPackOnParmToNonPackOnArg' flag is also produced upon
checking against the primary template, so it needs to be cached
in the specialziation as well.

This fixes a bug which has not been in any release, so there are no
release notes.

Fixes #125290
mizvekov added a commit that referenced this issue Feb 5, 2025
Class templates might be only instantiated when they are required to be
complete, but checking the template args against the primary template is
immediate.

This result is cached so that later when the class is instantiated,
checking against the primary template is not repeated.

The 'MatchedPackOnParmToNonPackOnArg' flag is also produced upon
checking against the primary template, so it needs to be cached in the
specialziation as well.

This fixes a bug which has not been in any release, so there are no
release notes.

Fixes #125290
Icohedron pushed a commit to Icohedron/llvm-project that referenced this issue Feb 11, 2025
…125791)

Class templates might be only instantiated when they are required to be
complete, but checking the template args against the primary template is
immediate.

This result is cached so that later when the class is instantiated,
checking against the primary template is not repeated.

The 'MatchedPackOnParmToNonPackOnArg' flag is also produced upon
checking against the primary template, so it needs to be cached in the
specialziation as well.

This fixes a bug which has not been in any release, so there are no
release notes.

Fixes llvm#125290
mizvekov added a commit that referenced this issue Feb 19, 2025
…5791)

Class templates might be only instantiated when they are required to be
complete, but checking the template args against the primary template is
immediate.

This result is cached so that later when the class is instantiated,
checking against the primary template is not repeated.

The 'MatchedPackOnParmToNonPackOnArg' flag is also produced upon
checking against the primary template, so it needs to be cached in the
specialziation as well.

This fixes a bug which has not been in any release, so there are no
release notes.

Fixes #125290
swift-ci pushed a commit to swiftlang/llvm-project that referenced this issue Feb 20, 2025
…m#125791)

Class templates might be only instantiated when they are required to be
complete, but checking the template args against the primary template is
immediate.

This result is cached so that later when the class is instantiated,
checking against the primary template is not repeated.

The 'MatchedPackOnParmToNonPackOnArg' flag is also produced upon
checking against the primary template, so it needs to be cached in the
specialziation as well.

This fixes a bug which has not been in any release, so there are no
release notes.

Fixes llvm#125290
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" confirmed Verified by a second party
Projects
None yet
4 participants