Revise constMap() functions

[Peter Kovacs]

One version of the overloaded constMap() function can't be called. It could be used to create a ConstMap instance with inlined constant value.

• ConstMap<int,Const<int,10> > cm; works, but
• constMap<int,Const<int,10> >(); does not work. It causes a compiler error: no matching function for call to ‘constMap()’.

[Balazs Dezso]

The const map with static value should be used in the following syntax: constMap<int, int, 10>();

In my point of view, it is a nicer syntax, but it could get more emphasis in the documentation.

[Peter Kovacs]

Patch for constMap() functions and FunctorToMap?

[Peter Kovacs]

Replying to deba:

The const map with static value should be used in the following syntax: constMap<int, int, 10>();

You helped me a lot with this example.

In my point of view, it is a nicer syntax,

Nicer or not, it works. Now I understand it, but it is not clear about the documentation, as you said.

I made a patch file to solve these problems. I added a new version of the constMap() function without parameter, so there are three versions:

  template<typename K, typename V>
  inline ConstMap<K, V> constMap(const V &v);
  
  template<typename K, typename V>
  inline ConstMap<K, V> constMap();

  template<typename K, typename V, V v>
  inline ConstMap<K, Const<V, v> > constMap();

Now both constMap<int, int, 10>() and constMap<int, Const<int, 10> >() works poperly. Although the former one calls the third version and the latter one calls the second version of the function, they are equivalent.

Apart form that I made another small fix in FunctorToMap. (The test file failed when it was compiled without optimization.)

What is your opinion? I think accepting this patch we needn't modify the documentation, since both syntax versions works. Is it okay?

[Alpar Juttner]

What is the reason for this patch​? Should it be included in the main?

[Balazs Dezso]

I made a patch file to solve these problems. I added a new version of the constMap() function without parameter, so there are three versions:

  template<typename K, typename V>
  inline ConstMap<K, V> constMap(const V &v);
  
  template<typename K, typename V>
  inline ConstMap<K, V> constMap();

  template<typename K, typename V, V v>
  inline ConstMap<K, Const<V, v> > constMap();

Now both constMap<int, int, 10>() and constMap<int, Const<int, 10> >() works poperly. Although the former one calls the third version and the latter one calls the second version of the function, they are equivalent.

I think both formats could be accepted for static const map values, therefore I support the third variant.

Apart form that I made another small fix in FunctorToMap. (The test file failed when it was compiled without optimization.)

What is your opinion? I think accepting this patch we needn't modify the documentation, since both syntax versions works. Is it okay?

The standard library stores the functors as value and not as reference. So Peter's solution is more convenient and conform to the c++ standard library, so I support to reverse this change.

[Peter Kovacs]

Maybe it wasn't clear, but there are two independent modifications in the patch.

• A new variant of constMap() without parameter (that was the second variant on my list) to support constMap<A,B>() calls. E.g. it makes possible to call constMap<A,Const<int,10> >(), which was not supported before.
• A bug fix in FunctorToMap: store the functor as value instead of reference. That bug was made by me during the general clean-up (the test file passed with optimization, so I didn't notice the problem).

So Peter's solution is more convenient and conform to the c++ standard library, so I support to reverse this change.

The patch just restores the solution used in the svn repository.

I think this bug fix is not a question. The only question is about the usage of constMap(). Should we also support constMap<A,B>() or just constMap<A,B>(b) and constMap<A,B,b>()? (The first one makes possible to call constMap<A,Const<B,b> >().)

[Balazs Dezso]

I think the patch solves the problem, so I think it should be merged into the main repository, and this thread could be closed.

[Alpar Juttner]

This patch​ in now in the main branch, see [8899d1891a3c]