Skip to content

Cannot derive from std::function #20376

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
llvmbot opened this issue Jun 11, 2014 · 13 comments
Closed

Cannot derive from std::function #20376

llvmbot opened this issue Jun 11, 2014 · 13 comments
Labels
bugzilla Issues migrated from bugzilla libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Comments

@llvmbot
Copy link
Member

llvmbot commented Jun 11, 2014

Bugzilla Link 20002
Resolution FIXED
Resolved on Jul 20, 2016 00:21
Version 3.4
OS All
Reporter LLVM Bugzilla Contributor
CC @dwblaikie,@mclow,@zygoloid

Extended Description

#include

struct s : std::function< void() > {
using function::function;
};

s q( std::function< void() >( []{} ) );

Nothing says std::function can't or shouldn't be used as a base class. However, this simple test yields an error,

c++/v1/functional:1454:41: error: no type named 'type' in 'std::__1::enable_if<false, void>'; 'enable_if' cannot be used to disable this declaration

I'm not sure why SFINAE isn't allowed in that context, but it might be because of the phrasing of 12.9/1.2:

— for each non-template constructor of X that has at least one parameter with a default argument, the set of constructors that results from omitting any ellipsis parameter specification and successively omitting parameters with a default argument from the end of the parameter-type-list

Omitting the default argument strips SFINAE from the constructor. The fix is to move the SFINAE condition to the exception specification.

@llvmbot
Copy link
Member Author

llvmbot commented Jun 11, 2014

Oops, exception specifications don't work as well as default template arguments in this context. Anyway, it's up to you, the implementer :) .

@dwblaikie
Copy link
Collaborator

I believe there are no guarantees that any C++ library member function doesn't have extra undocumented/hidden parameters, so long as existing call sites work. This would make the behavior your observing is within the rights of the implementation.

@llvmbot
Copy link
Member Author

llvmbot commented Jun 12, 2014

No, the behavior is not equivalent to the spec under the as-if rule.

@dwblaikie
Copy link
Collaborator

The standard says:

"An implementation shall not declare a global or non-member function signature with additional default arguments."

That doesn't appear to cover your example of inheriting the constructor.

@llvmbot
Copy link
Member Author

llvmbot commented Jun 12, 2014

Default arguments are allowed as long as they don't break something else. They are allowed as an implementation detail, such that the library may be used "as if" they did not exist.

Are you seriously saying that because default arguments are allowed, inheritance from standard classes is not?

@zygoloid
Copy link
Mannequin

zygoloid mannequin commented Jun 12, 2014

Default arguments are allowed as long as they don't break something else.

The actual rule is more permissive than that. [member.functions] (17.6.5.5)/2 says:

"An implementation may declare additional non-virtual member function signatures within a class:
-- by adding arguments with default values to a member function signature
-- [...]"

See also http://cplusplus.github.io/LWG/lwg-active.html#2259, which fixes some wording issues in this paragraph, but doesn't fundamentally change the rule in this case.

But I think this is largely academic: the original code seems reasonable, and I think we should accept it. My preferred fix is to fix the specification for inheriting constructors, but failing that, changing libc++ as a QoI issue seems reasonable to me.

@llvmbot
Copy link
Member Author

llvmbot commented Jun 27, 2014

@llvmbot
Copy link
Member Author

llvmbot commented Jun 27, 2014

Sorry, Wrong bug :(

@llvmbot
Copy link
Member Author

llvmbot commented Jun 30, 2014

Ok, so I agree that this is a problem and think we should move the SFINAE to the template parameters. Moving it to the template parameters does work, but it won't make std::function accept this code. My reading of the standard suggests that this code should be accepted since it says nothing about disabling the template constructor when T == std::function. I think this was probably omitted since 12.8.6 disables that constructor in the case where T == std::function anyway.

HOWEVER, this example is one hell of a corner case and I'm worried about allowing this. I just want to reiterate what David's example code should do because it took me a long time to figure out.

12.8.6 says:
A declaration of a constructor for a class X is ill-formed if its first parameter is of type (optionally cv-qualified)
X and either there are no other parameters or else all other parameters have default arguments. A member
function template is never instantiated to produce such a constructor signature. [ Example:
struct S {
template S(T);
S();
};
S g;
void h() {
S a(g); // does not instantiate the member template to produce S::S(S);
// uses the implicitly declared copy constructor
}
—end example ]

So what happen in this case is that the templated constructor gets imported into the class S. Then when that constructor is called the compiler doesn't instantiate the body and instead forwards the argument to the copy or move constructor in std::function. I think this is particularly insidious because the move/copy constructors are not actually in S's scope.

Anyway I'll leave it up to the experts to decide what change should be made. My opinion is to move the SFINAE into the template arguments but keep this constructor disabled in this case (when T == std::function).

@llvmbot
Copy link
Member Author

llvmbot commented Jun 30, 2014

Huh, I thought a change was needed to accept the following code based on 12.9/1.2 as discussed. But this case seems to be accepted without any changes:

#include

struct A : std::function<int()> {
using function::function;
};

int main() {
std::function<long()> f(nullptr);
A a(f);
}

Not sure why though... It seems like the default argument should be omitted.

@llvmbot
Copy link
Member Author

llvmbot commented Jun 30, 2014

Perhaps because passing an lvalue causes a reference type to be deduced, which might jiggle the SFINAE differently. The SFINAE might contain unnecessary logic to prevent instantiating a copy constructor, which went unnoticed and untested until now. (A constructor template will usually not generate a copy constructor, because the implicit one is always more specialized. I seem to recall there's a strict rule that forbids it from ever happening, but that shouldn't be relevant here.)

On the other hand, I realize now that I've been abusing inheriting constructors in this case, so thank you. I'll file a core language defect report, and fix my own project.

As of now, I plan to propose that copy and move constructors should be inherited as deleted rather than ignored. (Of course, a user-defined alias would override this.)

@llvmbot
Copy link
Member Author

llvmbot commented Jun 15, 2015

By the way, I've written a std::function compatible library https://github.com/potswa/cxx_function which does support inheriting constructors (and uses them internally). An issue did arise which fits the description of the previous comment and the CWG DR. The fix was a deleted overload:

https://github.com/potswa/cxx_function/blob/7a54cfbde6bb5717f1b8ef8c6083a3a6499f5513/cxx_function.hpp#L791

@llvmbot
Copy link
Member Author

llvmbot commented Jul 20, 2016

This has been fixed in 2 different ways:

  1. I implemented a fix in libc++ even though it's no longer needed (see below)

  2. Inheriting constructors were fixed in the standard and this fix has been implemented in Clang as of 3.9.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 9, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

No branches or pull requests

2 participants