COIN-OR::LEMON - Graph Library

Opened 10 years ago

Closed 10 years ago

#360 closed defect (fixed)

VS compilation error

Reported by: Peter Kovacs Owned by: Peter Kovacs
Priority: blocker Milestone: LEMON 1.2 release
Component: core Version: hg main
Keywords: Cc:
Revision id:

Description

bellman_ford_test.cc fails to compile with Visual Studio 2005 and 2008:

..\..\test\bellman_ford_test.cc(108) : error C3083: 'SetOperationTraits<lemon::BellmanFordToleranceOperationTraits<int,0> > >': the symbol to the left of a '::' must be a type

Attachments (1)

360-34380851fc86.patch (4.0 KB) - added by Peter Kovacs 10 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 10 years ago by Peter Kovacs

Status: newassigned

I found two problems here.

  1. It seems that VS somehow does not compile two consecutive ::SetOperationTraits<...> in a template named parameter list. However, this problem can be simply avoid.
  1. The other problem is with the BellmanFordToleranceTraits class, which has the following template parameter list:
      template <
        typename V,
        V eps = Tolerance<V>::def_epsilon>
    

comment:2 Changed 10 years ago by Peter Kovacs

The situation is even worse. The BellmanFordToleranceTraits solution that I proposed in #51 is illegal for non-integer types.

E.g. using this struct:

  template <typename T, T x>
  struct A {};

this code

  A<double, 0.0> a;

cannot be compiled. GCC says:

  error: ‘double’ is not a valid type for a template constant parameter

However, this one works:

  A<int, 0> a;

I must admit that I did not check this solution for floating point types (for which it was actually intended). I'm sorry. What to do then? Revert [a6eb9698c321]?

comment:3 in reply to:  2 ; Changed 10 years ago by Balazs Dezso

Replying to kpeter:

The situation is even worse. The BellmanFordToleranceTraits solution that I proposed in #51 is illegal for non-integer types.

E.g. using this struct:

  template <typename T, T x>
  struct A {};

this code

  A<double, 0.0> a;

cannot be compiled. GCC says:

  error: ‘double’ is not a valid type for a template constant parameter

However, this one works:

  A<int, 0> a;

I must admit that I did not check this solution for floating point types (for which it was actually intended). I'm sorry. What to do then? Revert [a6eb9698c321]?

It can be a fast solution, because this operation traits class is not used from other places.

In long time, I suggest to use non static operation traits classes, i.e. an instance can be constructed in the algorithm, which can hold the epsilon or a tolerance instance. This can be done in next release because it can be implemented with backward compatibility (it should be done in Dijkstra, as well).

comment:4 in reply to:  3 Changed 10 years ago by Peter Kovacs

Replying to deba:

It can be a fast solution, because this operation traits class is not used from other places.

Yes, it can be safely reverted, though it is not so nice to introduce a new feature and completely remove it soon after. :)

In long time, I suggest to use non static operation traits classes, i.e. an instance can be constructed in the algorithm, which can hold the epsilon or a tolerance instance. This can be done in next release because it can be implemented with backward compatibility (it should be done in Dijkstra, as well).

That was my first suggestion, too. It also requires set/get functions for the introduced op. traits object of the algorithm.

comment:5 Changed 10 years ago by Peter Kovacs

The attached patch [34380851fc86] undos the changes of [a6eb9698c321]. I'm sorry again for not testing the proposal correctly.

Using this patch, the library compiles using both GCC 4.1 and VS2008.

Changed 10 years ago by Peter Kovacs

Attachment: 360-34380851fc86.patch added

comment:6 in reply to:  3 Changed 10 years ago by Peter Kovacs

Replying to deba:

In long time, I suggest to use non static operation traits classes, i.e. an instance can be constructed in the algorithm, which can hold the epsilon or a tolerance instance. This can be done in next release because it can be implemented with backward compatibility (it should be done in Dijkstra, as well).

I created a follow-up ticket for this proposal, see #361.

comment:7 in reply to:  5 Changed 10 years ago by Alpar Juttner

Resolution: fixed
Status: assignedclosed

Replying to kpeter:

The attached patch [34380851fc86] undos the changes of [a6eb9698c321].

A more standard way of doing this kind of undo is unsing the hg backout command. See [d6052a9c4e8d].

It is already in the main branch and will also e merges to 1.2 together with the other relevant updates.

Note: See TracTickets for help on using tickets.