ignore_unused_variable_warning

[chrisatlemon]

I noticed that both in #include <boost/graph/graph_concepts.hpp> and in #include <lemon/graph_concepts.hpp>

the helper template function "ignore_unused_variable_warning" is defined! Trying to write an BGL adaption of a ListGraph?, for me gcc complains e.g. about

"call of overloaded 'ignore_unused_variable_warning(lemon::ListGraphBase::Node&)' is ambiguous"

I attached a minimal code example which reproduces the error

[Alpar Juttner]

The two instances of ignore_unused_variable_warning() are in different namespaces, thus it should not cause conflicts.

Are you sure that this is the real root of the problems. For example I can see other errors preceding the one you refer to, see below.

g++ testgraph.cpp $(pkg-config --cflags --libs /home/alpar/projects/LEMON/local/lib/pkgconfig/lemon.pc)
/usr/include/boost/graph/graph_concepts.hpp: In destructor 'boost::concepts::VertexListGraph<G>::~VertexListGraph() [with G = lemon::ListGraph]':
/usr/include/boost/graph/graph_concepts.hpp:164:   instantiated from 'static void boost::concept::requirement<Model>::failed() [with Model = boost::concepts::VertexListGraphConcept<lemon::ListGraph>]'
/usr/include/boost/concept_check.hpp:43:   instantiated from 'void boost::function_requires(Model*) [with Model = boost::concepts::VertexListGraphConcept<lemon::ListGraph>]'
testgraph.cpp:74:   instantiated from here
/usr/include/boost/graph/graph_concepts.hpp:185: error: no matching function for call to 'vertices(lemon::ListGraph&)'
/usr/include/boost/graph/graph_concepts.hpp: In member function 'void boost::concepts::VertexListGraph<G>::const_constraints(const G&) [with G = lemon::ListGraph]':
/usr/include/boost/graph/graph_concepts.hpp:187:   instantiated from 'boost::concepts::VertexListGraph<G>::~VertexListGraph() [with G = lemon::ListGraph]'
/usr/include/boost/graph/graph_concepts.hpp:164:   instantiated from 'static void boost::concept::requirement<Model>::failed() [with Model = boost::concepts::VertexListGraphConcept<lemon::ListGraph>]'
/usr/include/boost/concept_check.hpp:43:   instantiated from 'void boost::function_requires(Model*) [with Model = boost::concepts::VertexListGraphConcept<lemon::ListGraph>]'
testgraph.cpp:74:   instantiated from here
/usr/include/boost/graph/graph_concepts.hpp:199: error: no matching function for call to 'vertices(const lemon::ListGraph&)'

[Balazs Dezso]

shorter example

[Balazs Dezso]

If the ignore_unused_variable_warning() function is called in the boost namespace and the parameter is in the lemon namespace, then the Koenig lookup will find both functions.

I have uploaded a shorter example, which shows only this problem.

I think, it is better to rename this function.

[Alpar Juttner]

Replying to alpar:

For the record, I copy here Chris' answer from the lemon-devel list.

Hello,

The two instances of ignore_unused_variable_warning() are in different namespaces, thus it should not cause conflicts.

I think this can indeed cause problems because of the "argument dependent name lookup" (see e.g. "​http://en.wikipedia.org/wiki/Argument_dependent_name_lookup") which makes the lemon version avaiable to the boost namespace as the given object also comes from the lemon namespace.

For the other errors I habe to apologize, they seem to be purely boost related (I'm currently investigating this)

greetings, chris

[Alpar Juttner]

Replying to deba:

I think, it is better to rename this function.

I'd rather think the problem on the Boost's side. If they want to use their own ignore_unused_variable_warning() for third party classes, they should give the boost namespace explicitly, like

boost::ignore_unused_variable_warning(x);

or even like

::boost::ignore_unused_variable_warning(x);

Cris, could you confirm (or confute) this?

[chrisatlemon]

Replying to alpar:

Cris, could you confirm (or confute) this?

This would be the cleanest solution to the problem but renaming the easiest, I think, as this would not require to convince the developers on boost's side.

[Balazs Dezso]

Replying to alpar:

Replying to deba:

I think, it is better to rename this function.

I'd rather think the problem on the Boost's side. If they want to use their own ignore_unused_variable_warning() for third party classes, they should give the boost namespace explicitly, like

boost::ignore_unused_variable_warning(x);

or even like

::boost::ignore_unused_variable_warning(x);

Cris, could you confirm (or confute) this?

In my opinion, this is a quite deep problem in C++. Let us imagine, that the boost Dijkstra algorithm is called dijkstra(). (Fortunately, it is dijkstra_shortest_path()). If similarly the dijkstra were called from the boost namespace on a LEMON graph, then we would get the same problem. Because, you cannot know which names will be reused in other libraries in different namespaces, therefore you should explicitly specify the namespace of each function, which takes template parameters. In addition, following your advice about specifying the namespace of ignore_unused_variable_warning() in boost, it should be done for each template function in boost, lemon and all template libraries.

This is a design problem in C++, the effect of Koenig lookup were not checked against templates.

[Alpar Juttner]

Replying to deba:

In my opinion, this is a quite deep problem in C++.

I agree, that this is a superb stupidity in C++, but your example is probably wrong. The name collision causes problems when you the function from within the same namespace, and its parameter (type) comes from a third party namespace. I don't think this could be the case with dijsktra().

Anyway, if there is a change that it may happen, you must use it the explicitly given namespace. This is what boost should to do with ignore_unused_variable_warning.

[Alpar Juttner]

Replying to chrisatlemon:

This would be the cleanest solution to the problem but renaming the easiest, I think, as this would not require to convince the developers on boost's side.

Well, AFAIK, Boost guy's are big C++ advocates. So, their attitude should be very positive towards the correct use of the language. At least it worth trying to get in touch with them. Any volunteer for this task?

Btw. I have some fear that we will run into the same problem with enable_if. If this is true, then it gonna be a major headache.

[Balazs Dezso]

Replying to alpar:

Replying to chrisatlemon:

This would be the cleanest solution to the problem but renaming the easiest, I think, as this would not require to convince the developers on boost's side.

Well, AFAIK, Boost guy's are big C++ advocates. So, their attitude should be very positive towards the correct use of the language. At least it worth trying to get in touch with them. Any volunteer for this task?

Btw. I have some fear that we will run into the same problem with enable_if. If this is true, then it gonna be a major headache.

The enable_if is not affected by this problem, because that is a class. My proposal is to change the ignore_unused_variable_warning to a static template member function, because for them the Koenig lookup is not used.

[Alpar Juttner]

Replying to deba:

Replying to alpar:

Replying to chrisatlemon:

This would be the cleanest solution to the problem but renaming the easiest, I think, as this would not require to convince the developers on boost's side.

Well, AFAIK, Boost guy's are big C++ advocates. So, their attitude should be very positive towards the correct use of the language. At least it worth trying to get in touch with them. Any volunteer for this task?

Btw. I have some fear that we will run into the same problem with enable_if. If this is true, then it gonna be a major headache.

The enable_if is not affected by this problem, because that is a class. My proposal is to change the ignore_unused_variable_warning to a static template member function, because for them the Koenig lookup is not used.

You mean you would like to put a copy of this function into each class it is used? I don't like this idea very much.

Instead, I would still prefer persuading the Boost people somehow to use their ignore_unused_variable_warning properly. Has anyone made an attempt in this direction? (Sending an e-mail to an appropriate list, issuing a bug report etc.)

[Balazs Dezso]

Replying to alpar:

Replying to deba:

Replying to alpar:

Replying to chrisatlemon:

This would be the cleanest solution to the problem but renaming the easiest, I think, as this would not require to convince the developers on boost's side.

Well, AFAIK, Boost guy's are big C++ advocates. So, their attitude should be very positive towards the correct use of the language. At least it worth trying to get in touch with them. Any volunteer for this task?

Btw. I have some fear that we will run into the same problem with enable_if. If this is true, then it gonna be a major headache.

The enable_if is not affected by this problem, because that is a class. My proposal is to change the ignore_unused_variable_warning to a static template member function, because for them the Koenig lookup is not used.

You mean you would like to put a copy of this function into each class it is used? I don't like this idea very much.

Instead, I would still prefer persuading the Boost people somehow to use their ignore_unused_variable_warning properly. Has anyone made an attempt in this direction? (Sending an e-mail to an appropriate list, issuing a bug report etc.)

What does it mean that using it properly? In my point of view, if I define a function in a namespace (for example in lemon namespace), then I can call it without specifying its namespace from lemon.

Anyway, if the boost code is wrong, then our code should be wrong as well. So we have to fix our code. There are a few solutions.

• We have to specify lemon namespace for our function:

    ::lemon::ignore_unused_variable_warning(t);
  

If we choose this variant then boost have to solve the problem similarly. In addition if a function is defined in lemon namespace and it is called from the same namespace and it is unconstrained, then it has to be called with specifying its namespace.

• The second option to change the name of the function:

    lemon_ignore_unused_variable_warning(t);
  

I thought that we can forget the prefixes with the introduction of namespaces in c++, but the lemon prefix can help to avoid the name collision.

• We can wait to the C++ committee while they drop the Koenig lookup rule for unconstrained template functions.

• Using class based implementation:

    struct IgnoreUnusedVariableWarning {
      template <typename T>
      void ignore(const T &t) { } 
    };
    ...
    IgnoreUnuseVariableWarning::ignore(t);
  

• The previous solution with defines:

    #define LEMON_IGNORE_UNUSED_VARIABLE_WARNING(t) IgnoreUnuseVariableWarning::ignore(t)
  

[Alpar Juttner]

Replying to deba:

What does it mean that using it properly? In my point of view, if I define a function in a namespace (for example in lemon namespace), then I can call it without specifying its namespace from lemon.

It would be nice, but alas it is not true, exactly because of the Koenig lookup.

Anyway, if the boost code is wrong, then our code should be wrong as well.

I don't think so. I have checked the usage of ignore_unused_variable_warning() at random places and found no problem with the usage. Recall that it is necessary to declare the namespace only if there is a chance that it will be used with a type coming from another namespace. I could not find any example for this kind of usage (though I haven't checked all). Could you?

So we have to fix our code.

Did you indeed find any problematic code?

There are a few solutions.

• We have to specify lemon namespace for our function:

    ::lemon::ignore_unused_variable_warning(t);
  

Yes, I prefer this.

If we choose this variant then boost have to solve the problem similarly.

Yes, they have.

In addition if a function is defined in lemon namespace and it is called from the same namespace and it is unconstrained, then it has to be called with specifying its namespace.

I can't understand this.

[Balazs Dezso]

Replying to alpar:

Replying to deba:

What does it mean that using it properly? In my point of view, if I define a function in a namespace (for example in lemon namespace), then I can call it without specifying its namespace from lemon.

It would be nice, but alas it is not true, exactly because of the Koenig lookup.

Anyway, if the boost code is wrong, then our code should be wrong as well.

I don't think so. I have checked the usage of ignore_unused_variable_warning() at random places and found no problem with the usage. Recall that it is necessary to declare the namespace only if there is a chance that it will be used with a type coming from another namespace. I could not find any example for this kind of usage (though I haven't checked all). Could you?

You miss one point, lets see the following example, let's take that boost have some bignum class or some other ordered number type (boost::BigNum?). The dijkstra should check that the heap is conform to the heap concept. If somebody want to call the dijkstra with BigNum? value type.

  lemon::Dijkstra<SmartDigraph, SmartDigraph::ArcMap<boost::BigNum> >
    ::SetHeap<FibHeap<boost::BigNum, SmartDigraph::ArcMap<int> >
    ::Create dijkstra(digraph, length);

Because the heap concept check uses the ignore_unused_variable_warning() for the value type of the heap, and it can be defined in any namespaces (in this example in the boost namespace), therefore our ignore_unused_variable_warning() is wrong.

So we have to fix our code.

Did you indeed find any problematic code?

There are a few solutions.

• We have to specify lemon namespace for our function:

    ::lemon::ignore_unused_variable_warning(t);
  

Yes, I prefer this.

If we choose this variant then boost have to solve the problem similarly.

Yes, they have.

In addition if a function is defined in lemon namespace and it is called from the same namespace and it is unconstrained, then it has to be called with specifying its namespace.

I can't understand this.

We do not restrict which classes are lemon graphs, we define only the interface of the graph in the graph concepts. If somebody defined a graph in the boost namespace which is conform to our graph concept, and the boost defined a countNodes() function, then in our algorithms the countNodes() function calls would become ambiguous. Therefore, most of the LEMON functions can be affected with this problem (at least which are unconstrained and called from the library itself).

[Peter Kovacs]

I like the class based solution. However, we could write to Boost developers about this problem.

[Alpar Juttner]

Replying to kpeter:

I like the class based solution. However, we could write to Boost developers about this problem.

What is the advantage of the class based solution over simply adding the full explicit namespace?

[Alpar Juttner]

What shall we do with this ticket?

[Alpar Juttner]

What shall we do with this ticket? Any feedback from the Boost guys?

[Balazs Dezso]

I have added a changeset to add explicitly namespace to the ignore_unused_variable_warning() calls.

[Alpar Juttner]

Thanks. I split the modification to more changesets so that it can be merged to all active branches, see [3e711ee55d31], [a337a0dd3f75] and [dd1443e4a34c].