Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

Update <experimental/optional> to implement LFTS v2. #30044

Open Quuxplusone opened 7 years ago

Quuxplusone commented 7 years ago
Bugzilla Link PR31071
Status NEW
Importance P normal
Reported by matt.rajca@me.com
Reported on 2016-11-19 10:59:56 -0800
Last modified on 2017-03-03 13:01:16 -0800
Version unspecified
Hardware Macintosh MacOS X
CC dgregor@apple.com, eric@efcs.ca, hocheung20@gmail.com, llvm-bugs@lists.llvm.org, mclow.lists@gmail.com, richard-llvm@metafoo.co.uk
Fixed by commit(s)
Attachments OptionalFunctions.zip (16964 bytes, application/zip)
Blocks
Blocked by
See also

Created attachment 17615 Sample project

Suppose we a function that takes a completion handler in the form of an optional:

using Handler = function<void(void)>;

void doSomething(int flags, optional<Handler> callback = optional<Handler>()) {
    if (callback) {
        (*callback)();
    }
}

Since a default value of an empty optional is provided, I can invoke this without passing a value for the final parameter just fine:

doSomething(0);

Moreover, I can invoke doSomething if I explicitly cast a lambda expression to a Handler and pass it for the final parameter:

doSomething(0, Handler([]{
    cout << "hello world" << endl;
}));

However, passing a lambda expression directly does not work (the following does not compile with "No matching function for call to 'doSomething'"):

doSomething(0, []{
   cout << "hello world" << endl;
});

I assume this is because the type checker doesn't know we can assign a lambda expression to an optional (even if it's an optional and lambda expression can be assigned to functions).

In other words, if a lambda expression L can be assigned to a function F, we should be able to assign L to an optional of type optional.

A sample project that demonstrates this is attached. I am testing with Xcode 8.1 GM.

Quuxplusone commented 7 years ago

Attached OptionalFunctions.zip (16964 bytes, application/zip): Sample project

Quuxplusone commented 7 years ago

The following code compiles w/o error with ToT:

include

include

include

using namespace std;

using Handler = function<void(void)>;    
void doSomething(int flags, optional<Handler> callback = optional<Handler>()) {
    if (callback)
        (*callback)();
    }

void func() {}

int main () { doSomething(0); doSomething(0, func); doSomething(0, [](){}); doSomething(0, [](){cout << "Hello World\n"; }); }

Note that with Apple's shipping Xcode, the last three calls to doSomething fail to compile.

Quuxplusone commented 7 years ago
Apparently we've got some divergence.
This fails with std::experimental::optional, but works with std::optional.
Quuxplusone commented 7 years ago

Re-title this bug. The reproducer fails to compile because we only implement LFTS v1, and the constructor needed to accept the example was added in LFTS v2. See https://rawgit.com/cplusplus/fundamentals-ts/v2/main.html#optional.object.ctor

I looked into using to provide <experimental/optional> but it doesn't look easy. My plan was to re-name std::optional to std::__optional and providing std::optional as a alias template. That way we could expose the implementation before C++17.

However there are a couple of problems:

1.experimental::optional uses it's own in_place_t, so it's incompatible with std::in_place_t.

  1. std::optional requires C++14 constexpr, so we can't possibly implement it in C++11 fully.

  2. uses C++14 type-traits foo_t aliases and C++17 foo_v and all of these would need to be changed.

I tried pretty hard to make this work, and it almost does, but in the end it's going to be too much trouble.

Unfortunately <experimental/optional> isn't going to get much love now that is implemented.

Quuxplusone commented 7 years ago

_Bug 32133 has been marked as a duplicate of this bug._