Re: [boost] [Fit] Review

Subject: Re: [boost] [Fit] Review
From: Paul Fultz II (pfultz2_at_[hidden])
Date: 2016-03-13 17:55:36

On Sunday, March 13, 2016 at 5:43:43 AM UTC-5, Vicente J. Botet Escriba 
wrote:
>
> Le 13/03/2016 03:35, Lee Clagett a écrit : 
> > On Fri, 11 Mar 2016 19:41:22 +0000 (UTC) 
> > paul Fultz <pfu..._at_[hidden] <javascript:>> wrote: 
> > 
> >>> On Friday, March 11, 2016 12:30 PM, Bjorn Reese 
> >>> <bre..._at_[hidden] <javascript:>> wrote: 
> >>>> On 03/11/2016 01:50 AM, Paul Fultz II wrote: 
> >>>     
> >>>>   library. I do, however, need to discuss some of the misperceived 
> >>>> issues 
> >>> with 
> >>>>   using global function objects in the documentation. As the issues 
> >>>> raised in the review were: 
> >>> The destruction order of global objects in different translation 
> >>> units is undefined. This means that I cannot use a global fit 
> >>> function in the destructor of one of my own global objects because 
> >>> the former may have 
> >>> been destroyed before the latter. 
> >> This is not a problem, because it is initialized at compile-time, 
> >> that means it is a literal type: 
> >> 
> >> http://en.cppreference.com/w/cpp/concept/LiteralType 
> >> 
> >> 
> >> So besides the constructor not having side-effects, the destructor 
> >> must be trivial, as well. For example, this cannot compile: 
> >> 
> >> 
> >> struct foo_fn 
> >> { 
> >>      template<class... Ts> 
> >>      auto operator()(Ts&&...) const 
> >>      {} 
> >> 
> >>      ~foo_fn() 
> >>      { 
> >>          std::cout << "Hello"; 
> >>      } 
> >> }; 
> >> 
> >> BOOST_FIT_STATIC_FUNCTION(foo) = foo_fn(); 
> >> 
> >> int main() { 
> >>          foo(); 
> >> } 
> >> 
> >> So with trivial destructors, the order of destruction doesn't matter. 
> >> Furthermore, on most compilers(that is everything but MSVC), the 
> >> function is the same across all translation units. So there is only 
> >> one function in the executable, not one per translation unit. Taking 
> >> the address of the function will yield the same address across all 
> >> translation units. This is the same way functions work. 
> >> 
> > The following program throws an exception and aborts in Clang 3.4 and 
> > Gcc 4.8, both on Ubuntu 14.04 
> > 
> >      #include <iostream> 
> >      #include <stdexcept> 
> >      #include <fit/function.hpp> 
> >      #include <fit/static.hpp> 
> > 
> >      class log_ { 
> >      public: 
> >        log_() 
> >          : active_(true) { 
> >          std::cout << "constructing log" << std::endl; 
> >          // or some equally terrible global state changing 
> >          // code such as a file open 
> >        } 
> > 
> >        ~log_() { 
> >          std::cout << "destructing log" << std::endl; 
> >          active_ = false; 
> >        } 
> > 
> >        void operator()(const char* const msg) const { 
> >          if (active_) { 
> >            std::cout << msg << std::endl; 
> >          } else { 
> >            throw std::runtime_error{"not active"}; 
> >          } 
> >        } 
> >      private: 
> >        bool active_; 
> >      }; 
> > 
> >      FIT_STATIC_FUNCTION(log) = fit::static_<log_>{}; 
> > 
> >      struct one_ { 
> >        ~one_() { 
> >          log("destructing one"); 
> >        } 
> >      }; 
> > 
> >      struct two_ { 
> >        two_() { 
> >          log("constructing two"); 
> >        } 
> >      }; 
> > 
> >      const one_ one{}; 
> >      const two_ two{}; 
> > 
> >      int main() { 
> >        return 0; 
> >      } 
> > 
> > Both Gcc and Clang print: 
> > 
> >      constructing log 
> >      constructing two 
> >      destructing log 
> >      terminate called after throwing an instance of 'std::runtime_error' 
> >        what():  not active 
> > 
> > The generated program conforms to the C++ standard because `log_` was 
> > created as a function local static, and the call to `operator()` after 
> > destruction can do _anything_ (undefined behavior). Also, `log_` can be 
> > constructed before `one_` and `two_` (and therefore destructed after 
> > both) as well, and still conform to the C++ specifications. Or at least 
> > that is my understanding of static construction/destruction, which is 
> > complex in C++. 
> > 
> > Lee 
>
> Tanks Lee for this example. 
>
> I believe that static_ needs to require that the the parameter is 
> is_trivially_destructible [1]. 
>
> In [2] we can read " The |static_| adaptor is a static function adaptor 
> that allows any default-constructible function object to be 
> static-initialized.". 
>
> It seems the implementation doesn't do the check. Please, could you add 
> an issue on Github? 
>

The `static_` adaptor is designed to initialize functions that may not have
trivial or constexpr constructors. Ideally `static_<F>` should be the
equivalent to this:

    template<class... Ts>
    auto static_f(Ts&&... xs) -> decltype(F()(std::forward<Ts>(xs)...))
    {
        static F f;
        return f(std::forward<Ts>(xs)...);
    }

I believe, the same problem would occur if the function was defined as 
above.

However, a use case for `static_` is when defining a thread-safe memoization
function. I am not sure if this will break the trivial destructor 
requirement.
I will need to investigate this more to see what would be the best action.
 

>
> Vicente 
>
>
>
> [1] 
> http://www.cplusplus.com/reference/type_traits/is_trivially_destructible/ 
> [2] http://pfultz2.github.io/Fit/doc/html/static/index.html 
>
>
> _______________________________________________ 
> Unsubscribe & other changes: 
> http://lists.boost.org/mailman/listinfo.cgi/boost 
>