Project

General

Profile

Actions

Feature #3753

closed

Backport std::optional

Added by Junxiao Shi over 7 years ago. Updated almost 6 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Base
Target version:
Start date:
Due date:
% Done:

100%

Estimated time:
2.00 h

Description

Create ndn::optional as an alias of std::experimental::optional or boost::optional.

Actions #1

Updated by Junxiao Shi over 7 years ago

I added this snippet to .waf-tools/compiler-features.py:

STD_EXPERIMENTAL_OPTIONAL = '''
#include <tuple>
#include <experimental/optional>
int
main()
{
  std::experimental::optional<int> a(std::experimental::nullopt);
  try {
    a.value();
  }
  catch (const std::experimental::bad_optional_access&) {
  }
  auto b = std::experimental::make_optional(2);
  auto c = std::experimental::make_optional<std::tuple<int, int>>(3, 4);
  c = std::experimental::nullopt;
  return 0;
}
'''

@conf
def check_experimental_optional(self):
    if self.check_cxx(msg='Checking for std::experimental::optional',
                      fragment=STD_EXPERIMENTAL_OPTIONAL,
                      features='cxx', mandatory=False):
        self.define('HAVE_STD_EXPERIMENTAL_OPTIONAL', 1)

But it returns "no" on both Ubuntu 16.04 and OSX 10.11.

Ubuntu 16.04 build/config.log:

ESC[01mESC[K/usr/include/c++/5/bits/c++14_warning.h:32:2:ESC[mESC[K ESC[01;31mESC[Kerror: ESC[mESC[K#error This file requires compiler and library support for the forthcoming ISO C++ 2014 standard. This support is currently experimental, and must be enabled with the -std=c++1y or -std=gnu++1y compiler options.
 #error This file requires compiler and library support for the forthcoming \

OSX 10.11 build/config.log:

err: ESC[1m../test.cpp:7:22: ESC[0mESC[0;1;31merror: ESC[0mESC[1mno member named 'optional' in namespace 'std::experimental'ESC[0m
  std::experimental::optional<int> a(std::experimental::nullopt);
ESC[0;1;32m  ~~~~~~~~~~~~~~~~~~~^

/Applications/Xcode.app/Contents//Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/experimental/optional contains
#if _LIBCPP_STD_VER > 11
which disables std::experimental::optional despite I'm allowed to include the header.

Thus, I'll only use Boost.Optional.

Actions #2

Updated by Junxiao Shi over 7 years ago

  • Status changed from In Progress to Code review
  • % Done changed from 0 to 100
Actions #3

Updated by Junxiao Shi over 7 years ago

  • Status changed from Code review to In Progress
  • % Done changed from 100 to 50

https://gerrit.named-data.net/3126 patchset2 is not compiling on Ubuntu 14.04.
I only noticed difference in nullopt and make_optional when preparing that patchset.

There are more differences between Boost.Optional and C++17 std::optional:

C++17 Boost 1.54 Boost 1.61
nullopt none none
in_place TypedInPlaceFactory TypedInPlaceFactory
has_value operator bool operator bool
value get (w/assertion) value
value_or get_value_or value_or

Here are two choices:

  • Implement ndn::optional class template according to C++17 spec. Make it implicitly convertible from and to boost::optional. All calling code should use C++17 syntax.
  • Make ndn::optional an alias of boost::optional. All calling code should use Boost 1.54 syntax; this implies calling deprecated get_value_or in Boost 1.61 which could cause a warning.
Actions #4

Updated by Junxiao Shi over 7 years ago

  • % Done changed from 50 to 80

20160823 NFD call decides to go with C++17 syntax. https://gerrit.named-data.net/3126 patchset3 implements so.

The following are in C++17 spec but I couldn't get them to work so they are not in current implementation:

  • construct or emplace with std::initializer_list
  • compare with nullopt or plain value
  • move constructor
Actions #5

Updated by Alex Afanasyev over 7 years ago

This issue talk about backporting, but doesn't give any example on how you planning to use optional. I would have no opinion on the matter until then...

Actions #6

Updated by Davide Pesavento over 7 years ago

From http://en.cppreference.com/w/cpp/utility/optional :

A common use case for optional is the return value of a function that may fail. As opposed to other approaches, such as std::pair<T, bool>, optional handles expensive to construct objects well and is more readable, as the intent is expressed explicitly.

We could potentially replace every such usage of std::pair<T, bool> with optional<T> in our APIs.

Actions #7

Updated by Alex Afanasyev over 7 years ago

ok. Is it different much from using std::unique_ptr?

Actions #8

Updated by Davide Pesavento over 7 years ago

std::unique_ptr models a pointer (with exclusive ownership semantics), the pointed-to object has to be dynamically allocated somehow (by the user code). std::optional models an object, no dynamic memory allocation ever takes place.

Actions #9

Updated by Alex Afanasyev over 7 years ago

Are you sure about no dynamic allocation? May be I'm misunderstanding the mechanism optional is implemented, but it seem that with make_optional call there is somewhere inside a call for new. No?

Actions #10

Updated by Davide Pesavento over 7 years ago

Yes I'm sure. optional uses "placement new" to construct the contained object in place (as a member variable of the optional object). make_optional just perfectly forwards the arguments to optional constructor.

[edit: I'm talking about a standard implementation, I haven't looked at how Junxiao implemented the backported version.]

Actions #11

Updated by Alex Afanasyev over 7 years ago

Got it.

As per note 1, the backported version is using only Boost.Optional, unless we allow C++17 syntax, which would require more changes in build scripts.

Actions #12

Updated by Davide Pesavento over 7 years ago

Alex Afanasyev wrote:

As per note 1, the backported version is using only Boost.Optional, unless we allow C++17 syntax, which would require more changes in build scripts.

Yeah I'm afraid that's the most viable option for now. We could use std::experimental::optional with gcc-6 and later (e.g. in upcoming Ubuntu 16.10), once #3076 is implemented.

Actions #13

Updated by Junxiao Shi over 7 years ago

  • Status changed from In Progress to Closed
  • % Done changed from 80 to 100
Actions #14

Updated by Junxiao Shi almost 6 years ago

  • Status changed from Closed to Resolved

I notice optional::reset is missing and it's added in https://gerrit.named-data.net/4669

Actions #15

Updated by Junxiao Shi almost 6 years ago

I don't see much value in this TBH, just use foo = nullopt;

The backport has to match C++17 standard, with all differences noted in \file Doxygen. I'm surprised when reset is missing.
I can either add the missing function, or add it as a "difference" in \file Doxygen, and I chose the former.

Actions #16

Updated by Davide Pesavento almost 6 years ago

I don't see much value in this TBH, just use foo = nullopt;

I said that because we will soon be able to rely on std::experimental::optional directly, thus the boost-based backport will soon die.

The backport has to match C++17 standard, with all differences noted in \file Doxygen.

You just made this up.

Actions #17

Updated by Davide Pesavento almost 6 years ago

  • Status changed from Resolved to Closed
Actions

Also available in: Atom PDF