smallfunction icon indicating copy to clipboard operation
smallfunction copied to clipboard

changes

Open user706 opened this issue 7 years ago • 7 comments

  • fix gcc warning warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]

    with 2 lines splitup:

     Concept *cpt = reinterpret_cast<Concept*>(memory);
     cpt->~Concept();
  • use std::size_t instead of unsigned
  • align according to alignof(std::max_align_t)
  • use mutable F to get the following functor compiled:
    struct Func1
    {
        int operator()(int val) // const
        {
           return val * 2;
        }

    int a[10];
    };

(used here: https://github.com/user706/CxxFunctionBenchmark). I knew what to do here, thanks to @arobenko - ref

  • use static_assert instead of enable_if
  • add: SmallFun& operator=(F const&f)

user706 avatar Jul 14 '18 22:07 user706

You can see it in action in the following benchmark: https://github.com/user706/CxxFunctionBenchmark (used in various.cpp)

PS: what commands do I have to use, to get this buckaroo thing working?

Thanks!

user706 avatar Jul 14 '18 22:07 user706

I'm compiling with gcc under Linux. Which compiler do you use? Does it compile with your compiler?

Are my changes with static_asserts working in your setup?

user706 avatar Jul 14 '18 22:07 user706

Thanks for your efforts!

Are my changes with static_asserts working in your setup?

The enable_if vs static_assert is always an interesting discussion. Although static_assert is easier to read and understand, it has one big disadvantage: You cannot use it in a SFINAE context. Hence we cannot use it here and have to patiently wait for the Concepts-TS to be adopted before we can make it more sane and readable.

use mutable F to get the following functor compiled:

It works for your use case but it would break other cases. Imagine a functor that has different implementations of operator(). Using mutable will always prefer the non-const variant even if in a const context.

Checkout the godbolt example and see how the return value of T::operator() changes based on the presence of the mutable keyword.

In order to handle both situations correctly, we need a smarter approach.

As of the alignment issue, good spot! Would love to get that merged. Can you make a PR with only the following changes:

  • fix gcc warning warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
  • use std::size_t instead of unsigned
  • align according to alignof(std::max_align_t)

nikhedonia avatar Jul 15 '18 11:07 nikhedonia

The enable_if vs static_assert is always an interesting discussion. Although static_assert is easier to read and understand, it has one big disadvantage: You cannot use it in a SFINAE context.

? Which SFINAE use-case do you mean exactly? Please can you give an example where your enable_if is useful for a user using the smallfun library.

user706 avatar Jul 17 '18 18:07 user706

Imagine a functor that has different implementations of operator(). Using mutable will always prefer the non-const variant even if in a const context.

Checkout the godbolt example and see how the return value of T::operator() changes based on the presence of the mutable keyword.

(edited)

~~Your example needs fixing ;)~~

    {
        T t{};
        smallfun::SmallFun<int()> f(t);
        std::cout << f() << "\t should get " << t() << std::endl;
    }
    {
        const T t{};
        smallfun::SmallFun<int()> f(t);
        std::cout << f() << "\t should get " << t() << std::endl;
    }

~~See it here on wandbox, to see how your original code (on the master branch) also does not return what you want.~~

~~I'll need to research, to see how to get it to do what we actually want...~~

Sorry... I just noticed that this is probably not a problem, since std::function behaves exactly the same way. See here

(Oh and by the way... std::function also does not behave the way you suspected in your godbolt example. See here)

Interesting reading here: "std::function does not enforce const-correctness: it allows you to store mutable callables"

user706 avatar Jul 17 '18 18:07 user706

(edited)

I should also mention that the following

add: SmallFun& operator=(F const&f)

(ref) is important!

It's one way to fix a bug in your original, where a destructor is not called. See it your original here -- prints dtor only once, but should come twice.

The fix is to uncomment line 3

#include "smallfun_changed.hpp" /* mine */

-- it should then print dtor twice.

user706 avatar Jul 17 '18 20:07 user706

I've got a branch called my_experiments: https://github.com/user706/smallfunction/tree/my_experiments Still working on this (when I find time). (Still need to experiment some more, and fix some stuff that's still open... such as fixing the rhs after move operations, etc. )

user706 avatar Jul 22 '18 20:07 user706