[Boost-users] [BGL] Bundled properties and property maps

[Alex Brooks]

Hi,

I've used BGL before for representing graphs, but this is my first time with
an algorithm. I've been stuck for many hours trying to define a property map
with bundled properties, I'd really appreciate some help. I'm using gcc4.3,
boost 1.34. An example program and the compiler error are:

-------------------------------------------------------------
#include <boost/graph/graph_traits.hpp>
#include <boost/graph/adjacency_list.hpp>
#include <boost/graph/astar_search.hpp>

using namespace std;

class Point {
public:
double p_[2];
};

struct EdgeProp {
double cost;
};

typedef boost::adjacency_list<boost::vecS,
boost::vecS,
boost::undirectedS,
Point*,
EdgeProp> Graph;

typedef boost::graph_traits<Graph>::vertex_descriptor VertexRef;

class AStarHeuristic : public boost::astar_heuristic<Graph, double>
{
public:
double operator()( VertexRef vertex ) const {return 0.0;}
};

// exception for termination
struct FoundGoal {};

//
// A* Visitor that terminates when we reach the goal
//
class AStarVisitor : public boost::default_astar_visitor
{
public:
AStarVisitor( const VertexRef &goal )
: goal_(goal) {}

template <class Graph>
void examine_vertex( VertexRef vertex, Graph &g )
{
if( vertex == goal_ )
throw FoundGoal();
}

private:
VertexRef goal_;
};

bool
searchThroughGraph( const Graph &graph,
VertexRef start,
VertexRef goal,
std::vector<VertexRef> &shortestPath )
{
AStarHeuristic aStarHeuristic;
AStarVisitor aStarVisitor( goal );

typedef boost::property_map<Graph, double EdgeProp::*>::type
EdgeWeightMap;
EdgeWeightMap edgeWeightMap = get(&EdgeProp::cost, graph);

std::vector<VertexRef> predecessors(num_vertices(graph));
try {
astar_search( graph,
start,
aStarHeuristic,
boost::weight_map(edgeWeightMap).
predecessor_map(&predecessors[0]).
visitor(aStarVisitor) );
}
catch ( const FoundGoal& )
{
shortestPath.clear();
for ( VertexRef vertex = goal;; vertex = predecessors[vertex] )
{
shortestPath.push_back( vertex );
if ( predecessors[vertex]==vertex ) break;
}
std::reverse( shortestPath.begin(), shortestPath.end() );
return true;
}

// If we get to here, we couldn't reach the goal
return false;
}

int main()
{
Graph graph;
std::vector<VertexRef> path;
searchThroughGraph(graph,0,1,path);
return 0;
}

-------------------------------------------------------------

/home/a.brooks/shh/src/libs/prm/test/stupid.cpp: In function ‘bool
searchThroughGraph(const Graph&, VertexRef, VertexRef, std::vector<unsigned
int, std::allocator<unsigned int> >&)’:
/home/a.brooks/shh/src/libs/prm/test/stupid.cpp:64: error: conversion
from ‘boost::bundle_property_map<const boost::adjacency_list<boost::vecS,
boost::vecS, boost::undirectedS, Point*, EdgeProp, boost::no_property,
boost::listS>, boost::detail::edge_desc_impl<boost::undirected_tag, unsigned
int>, EdgeProp, const double>’ to non-scalar type ‘searchThroughGraph(const
Graph&, VertexRef, VertexRef, std::vector<unsigned int,
std::allocator<unsigned int> >&)::EdgeWeightMap’ requested

-------------------------------------------------------------

Does anyone know what might be wrong?

More generally, I'm a bit mystefied by the "<Graph, double EdgeProp::*>::type"
syntax -- can anyone give me any pointers about what this syntax means, or
where I can look it up? (googling for colon-colon-star doesn't help much).
I also tried pattern-matching from the documentation page about bundled
properties (the weight_map(get(&Highway::miles, map)) example) but couldn't
get this to compile either. Is this example correct?

Thanks very much in advance,

Alex
_______________________________________________
Boost-users mailing list
Boost...@lists.boost.org
http://lists.boost.org/mailman/listinfo.cgi/boost-users

[Alex Brooks]

Sorry, I neglected to point out the line that doesn't compile. It's marked
below:

typedef boost::property_map<Graph, double EdgeProp::*>::type EdgeWeightMap;

// The following line fails to compile

EdgeWeightMap edgeWeightMap = get(&EdgeProp::cost, graph);

Thanks again,

[Andrew Sutton]

Does anyone know what might be wrong?

Yes... The type of the property map generated by the get() function is built for a *const* graph (since you're passing graph as const&), but your're declaring EdgeWeightMap over a non-const graph (as in property_map<Graph, ...>). I think the most appropriate fix is to change the definition of EdgeWeightMap to
 
   typedef boost::property_map<const Graph, double const EdgeProp::*>::type EdgeWeightMap;

Another solution might be to always pass by non-const reference, but that's not really good style. I would have thought that property_map was appropriately specialized for const/non-const versions. I'll have to look into it.

More generally, I'm a bit mystefied by the "<Graph, double EdgeProp::*>::type"
syntax -- can anyone give me any pointers about what this syntax means, or
where I can look it up? (googling for colon-colon-star doesn't help much).
I also tried pattern-matching from the documentation page about bundled
properties (the weight_map(get(&Highway::miles, map)) example) but couldn't
get this to compile either.  Is this example correct?

It's pointer-to-member syntax. Read from right to left You're saying that the type is a "pointer to a member of EdgeProp that is of type double." Or in this case, a pointer to a const EdgeProp that's a double.

Andrew Sutton
andrew....@gmail.com

[Alex Brooks]

Hi Andrew,

Thanks very much for the help, I've got it working now. I had to use a
non-const reference though. Here are the options I tried:

// Option A:
Graph &nonConstGraph = const_cast<Graph&>(graph);

typedef boost::property_map<Graph, double EdgeProp::*>::type
EdgeWeightMap;

EdgeWeightMap edgeWeightMap = get(&EdgeProp::cost, nonConstGraph);

// Option B:
// typedef boost::property_map<Graph, double const EdgeProp::*>::type
EdgeWeightMap;
// EdgeWeightMap edgeWeightMap = get(&EdgeProp::cost, graph);

Option A works, but is a bit ugly.
Option B is what you suggested, but it gives the following error:

/home/a.brooks/shh/src/libs/prm/test/stupid.cpp:70: error: conversion

from 'boost::bundle_property_map<const boost::adjacency_list<boost::vecS,
boost::vecS, boost::undirectedS, Point*, EdgeProp, boost::no_property,
boost::listS>, boost::detail::edge_desc_impl<boost::undirected_tag, unsigned
int>, EdgeProp, const double>' to non-scalar type 'searchThroughGraph(const
Graph&, VertexRef, VertexRef, std::vector<unsigned int,
std::allocator<unsigned int> >&)::EdgeWeightMap' requested

Do I also need to modify the 'get()' line?

> It's pointer-to-member syntax. Read from right to left You're saying that
> the type is a "pointer to a member of EdgeProp that is of type double." Or
> in this case, a pointer to a const EdgeProp that's a double.

Awesome, thanks for the pointer.

Cheers,

[Andrew Sutton]

//     typedef boost::property_map<Graph, double const EdgeProp::*>::type
EdgeWeightMap;
//     EdgeWeightMap edgeWeightMap = get(&EdgeProp::cost, graph);

Option A works, but is a bit ugly.
Option B is what you suggested, but it gives the following error:

Gotta a const Graph also:

property_map<const Graph, double const EdgeProp::*>

Andrew Sutton
andrew....@gmail.com

[Alex Brooks]

> Gotta a const Graph also:
>
> property_map<const Graph, double const EdgeProp::*>

This line compiles now, but I get a new compile error from the astar_search
line. The important bits are:

typedef boost::property_map<const Graph, double const EdgeProp::*>::type
EdgeWeightMap;

EdgeWeightMap edgeWeightMap = get(&EdgeProp::cost, graph);

std::vector<VertexRef> predecessors(num_vertices(graph));

astar_search( graph,
start,
aStarHeuristic,
boost::weight_map(edgeWeightMap).
predecessor_map(&predecessors[0]).
visitor(aStarVisitor) );

The error is:

/usr/include/c++/4.3/bits/allocator.h:84: instantiated
from 'std::allocator<const double>'
/usr/include/c++/4.3/bits/stl_vector.h:75: instantiated
from 'std::_Vector_base<const double, std::allocator<const double> >'
/usr/include/c++/4.3/bits/stl_vector.h:176: instantiated
from 'std::vector<const double, std::allocator<const double> >'
/usr/include/boost/graph/astar_search.hpp:360: instantiated from 'void
boost::detail::astar_dispatch1(VertexListGraph&, typename
boost::graph_traits<G>::vertex_descriptor, AStarHeuristic, CostMap,
DistanceMap, WeightMap, IndexMap, ColorMap, const Params&) [with
VertexListGraph = const boost::adjacency_list<boost::vecS, boost::vecS,

boost::undirectedS, Point*, EdgeProp, boost::no_property, boost::listS>,

AStarHeuristic = AStarHeuristic, CostMap =
boost::detail::error_property_not_found, DistanceMap =
boost::detail::error_property_not_found, WeightMap =

boost::bundle_property_map<const boost::adjacency_list<boost::vecS,
boost::vecS, boost::undirectedS, Point*, EdgeProp, boost::no_property,
boost::listS>, boost::detail::edge_desc_impl<boost::undirected_tag, unsigned

int>, EdgeProp, const double>, IndexMap =
boost::vec_adj_list_vertex_id_map<boost::property<boost::vertex_bundle_t,
Point*, boost::no_property>, unsigned int>, ColorMap =
boost::detail::error_property_not_found, Params =
boost::bgl_named_params<AStarVisitor, boost::graph_visitor_t,
boost::bgl_named_params<unsigned int*, boost::vertex_predecessor_t,
boost::bgl_named_params<boost::bundle_property_map<const

boost::adjacency_list<boost::vecS, boost::vecS, boost::undirectedS, Point*,
EdgeProp, boost::no_property, boost::listS>,
boost::detail::edge_desc_impl<boost::undirected_tag, unsigned int>, EdgeProp,

const double>, boost::edge_weight_t, boost::no_property> > >]'
/usr/include/boost/graph/astar_search.hpp:394: instantiated from 'void
boost::astar_search(VertexListGraph&, typename
boost::graph_traits<G>::vertex_descriptor, AStarHeuristic, const
boost::bgl_named_params<P, T, R>&) [with VertexListGraph = const

boost::adjacency_list<boost::vecS, boost::vecS, boost::undirectedS, Point*,

EdgeProp, boost::no_property, boost::listS>, AStarHeuristic = AStarHeuristic,
P = AStarVisitor, T = boost::graph_visitor_t, R =
boost::bgl_named_params<unsigned int*, boost::vertex_predecessor_t,
boost::bgl_named_params<boost::bundle_property_map<const

boost::adjacency_list<boost::vecS, boost::vecS, boost::undirectedS, Point*,
EdgeProp, boost::no_property, boost::listS>,
boost::detail::edge_desc_impl<boost::undirected_tag, unsigned int>, EdgeProp,

const double>, boost::edge_weight_t, boost::no_property> >]'
/home/a.brooks/shh/src/libs/prm/test/stupid.cpp:83: instantiated from here
/usr/include/c++/4.3/ext/new_allocator.h:82: error: 'const _Tp*
__gnu_cxx::new_allocator<_Tp>::address(const _Tp&) const [with _Tp = const
double]' cannot be overloaded
/usr/include/c++/4.3/ext/new_allocator.h:79: error: with '_Tp*
__gnu_cxx::new_allocator<_Tp>::address(_Tp&) const [with _Tp = const double]'

Any other ideas?

Thanks again,

[Dmitry Bufistov]

Hi Alex,

I was able to compile your original code by changing the line
typedef boost::property_map<Graph, double EdgeProp::*>::type EdgeWeightMap;
to
typedef boost::property_map<Graph, double EdgeProp::*>::const_type
EdgeWeightMap;

hope this will help,
Dmitry

Alex Brooks escribió:

>> Gotta a const Graph also:
>>
>> property_map<const Graph, double const EdgeProp::*>
>

>

[Andrew Sutton]

I was able to compile your original code by changing the line
typedef boost::property_map<Graph, double EdgeProp::*>::type  EdgeWeightMap;
to
typedef boost::property_map<Graph, double EdgeProp::*>::const_type EdgeWeightMap;

That's right! I had completely forgotten about the const_type member... Good catch Dmitry.
 

Andrew Sutton
andrew....@gmail.com

[Andrew Sutton]

/usr/include/c++/4.3/ext/new_allocator.h:82: error: 'const _Tp*
__gnu_cxx::new_allocator<_Tp>::address(const _Tp&) const [with _Tp = const
double]' cannot be overloaded
/usr/include/c++/4.3/ext/new_allocator.h:79: error: with '_Tp*
__gnu_cxx::new_allocator<_Tp>::address(_Tp&) const [with _Tp = const double]'

Not good... astar passes its graph by non-const reference so when it declares:

      typedef typename property_traits<WeightMap>::value_type D;
      typename std::vector<D>::size_type

D is apparently const double, and std::vector<const double> doesn't seem to work too well - not to mention the fact that you probably want to assign values to elemetns of that vector :) I guess your only solution is to pass the graph by non-const reference.

This may be a bug, but I'm not entirely sure.

Andrew Sutton
andrew....@gmail.com

[Alex Brooks]

Hi Andrew/Dmitry,

Thanks again for the help. I guess I'll stick with the const-cast.

Cheers,

[Geoff Hilton]

I'm having the same problem though mine (with MSVC9) says:

error C2678: binary '=': no operator found which takes a left-hand
operand of type 'const Weight' (or there is no acceptable conversion)
c:\program files\boost\boost_1_37_0\boost\property_map.hpp 161

A follow-up would be greatly appreciated.

Thanks,
Geoff

[Dmitry Bufistov]

Geoff Hilton wrote:
>
>
> I'm having the same problem though mine (with MSVC9) says:
>
> error C2678: binary '=': no operator found which takes a left-hand
> operand of type 'const Weight' (or there is no acceptable conversion)
> c:\program files\boost\boost_1_37_0\boost\property_map.hpp 161
>
>
> A follow-up would be greatly appreciated.
>
> Thanks,
> Geoff

You can try to use
boost::remove_const<Weight>::type

Regards,
Dmitry

[Geoff Hilton]

Dmitry Bufistov wrote:
> Geoff Hilton wrote:
>>
>>
>> I'm having the same problem though mine (with MSVC9) says:
>>
>> error C2678: binary '=': no operator found which takes a left-hand
>> operand of type 'const Weight' (or there is no acceptable conversion)
>> c:\program files\boost\boost_1_37_0\boost\property_map.hpp 161
>>
>>
>> A follow-up would be greatly appreciated.
>>
>> Thanks,
>> Geoff
>
> You can try to use
> boost::remove_const<Weight>::type
>
> Regards,
> Dmitry

I appreciate your suggestion, but the reason I'm getting the error is
because I'm trying to make the code const-correct, until yesterday code
that raised this issue would have the graph parameter be rendered
non-const as a temporary "fix", but that's not good style, I'm looking
to remedy this by making the code const-correct now. :)

If there's no other solution then I'll revert the const-correctedness
changes until there is one, but I have a hard time believing this to be
the case.

Thanks,
Geoff

[Dmitry Bufistov]

Geoff Hilton wrote:
>
>
>
> I appreciate your suggestion, but the reason I'm getting the error is
> because I'm trying to make the code const-correct,

Just a quick glance at your code would be greatly appreciated,

Regards,
Dmitry

[Geoff Hilton]

Dmitry Bufistov wrote:
> Geoff Hilton wrote:
>>
>>
>> I'm having the same problem though mine (with MSVC9) says:
>>
>> error C2678: binary '=': no operator found which takes a left-hand
>> operand of type 'const Weight' (or there is no acceptable conversion)
>> c:\program files\boost\boost_1_37_0\boost\property_map.hpp 161
>>
>>
>> A follow-up would be greatly appreciated.
>>
>> Thanks,
>> Geoff
>
> You can try to use
> boost::remove_const<Weight>::type
>
> Regards,
> Dmitry

To help things along, here's the code in question:

Note: graph variable is of type const Graph&.

const boost::property_map<Graph,
Weight EdgeProp::*>::const_type eprop =
boost::get(&EdgeProp::weight, graph);

const boost::property_map<Graph,
boost::vertex_index_t>::const_type vindex =
boost::get(boost::vertex_index, graph);
const vertex_descriptor firstVertex = *GetGraphBeginVertexIterator(graph);

PredecessorMap preds = PredecessorMap(numVertices);
DistanceMap dists = DistanceMap(numVertices);
boost::bellman_ford_shortest_paths(graph, numVertices,
weight_map(eprop)
.predecessor_map(boost::make_iterator_property_map(preds.begin(),
vindex))
.distance_map(boost::make_iterator_property_map(dists.begin(), vindex))
.root_vertex(firstVertex)
.distance_combine(Combiner())
.distance_compare(Comparer(currentOffset))
);

Also, my Weight type has an explicit constructor which takes a POD
convertible to 0, a requirement imposed it seems by line 155 of
bellman_ford_shortest_paths.hpp. Why doesn't the algorithm take a weight
of value zero as a parameter and use it instead?

Anyway, because of this explicit constructor I've added a (typical)
assignment operator (not sure if this is relevant, but I'm mentioning it
just in case).

I should also note that out of curiosity I commented out line 103 of
bellman_ford_shortest_paths.hpp which is:

function_requires<ReadablePropertyMapConcept<WeightMap, Edge> >();

Commenting out the above line allows my code as written above to
compile, otherwise it fails with the error written in my original
response (quoted above). Whether the compiled code (with line 103
commented out) functions as expected I don't know, I haven't tested it.

Thanks,
Geoff

[Dmitry Bufistov]

Geoff Hilton escribió:

>
> Note: graph variable is of type const Graph&.
>

[skip]

>
> Also, my Weight type has an explicit constructor which takes a POD
> convertible to 0, a requirement imposed it seems by line 155 of
> bellman_ford_shortest_paths.hpp. Why doesn't the algorithm take a weight
> of value zero as a parameter and use it instead?
>
> Anyway, because of this explicit constructor I've added a (typical)
> assignment operator (not sure if this is relevant, but I'm mentioning it
> just in case).
>
> I should also note that out of curiosity I commented out line 103 of
> bellman_ford_shortest_paths.hpp which is:
>
> function_requires<ReadablePropertyMapConcept<WeightMap, Edge> >();
>
> Commenting out the above line allows my code as written above to
> compile, otherwise it fails with the error written in my original
> response (quoted above). Whether the compiled code (with line 103
> commented out) functions as expected I don't know, I haven't tested it.
>
> Thanks,
> Geoff

It looks like the problem is with
template <class PMap, class Key> struct ReadablePropertyMapConcept;

The following code fails to compile:

#include <boost/graph/graph_concepts.hpp>
#include <boost/graph/adjacency_list.hpp>

using namespace boost;
struct EdgeProp
{
double weight;
};

typedef boost::adjacency_list<vecS, vecS, directedS, no_property,
EdgeProp > graph_t;
int main()
{
typedef boost::property_map<graph_t, double
EdgeProp::*>::const_type WeightMap;
typedef boost::graph_traits<graph_t>::edge_descriptor Edge;

function_requires<ReadablePropertyMapConcept<WeightMap, Edge> >();
}

You may try to change the boost/property_map.hpp as follows:
(in the version 1.37 the line number is 165)
typename typename property_traits<PMap>::value_type val;
to the
typename remove_const<typename property_traits<PMap>::value_type>::type val;

Does it work for you?

Regards,
Dmitry

[Geoff Hilton]

Thanks Dmitry, it does work! I do wonder though, isn't there a better
fix? What about specializing ReadablePropertyMapConcept specifically for
const properties, or does that not make sense with concept classes? I'm
okay with modifying BGL code, but the remove_const makes me feel dirty.

This sounds like something that should be brought up with BGL maintainers...

Geoff

[Dmitry Bufistov]

Geoff Hilton escribió:

>
>> (in the version 1.37 the line number is 165)
>> typename typename property_traits<PMap>::value_type val;
>> to the
>> typename remove_const<typename
>> property_traits<PMap>::value_type>::type val;
>>
>> Does it work for you?
>>
>> Regards,
>> Dmitry
>
> Thanks Dmitry, it does work! I do wonder though, isn't there a better
> fix? What about specializing ReadablePropertyMapConcept specifically for
> const properties, or does that not make sense with concept classes? I'm
> okay with modifying BGL code, but the remove_const makes me feel dirty.
>
> This sounds like something that should be brought up with BGL
> maintainers...
>
> Geoff

I believe that something is bad with the const version of
bundled_properties. Andrew, what do you think?

Dmitry

[Andrew Sutton]

Thanks Dmitry, it does work! I do wonder though, isn't there a better fix? What about specializing ReadablePropertyMapConcept specifically for const properties, or does that not make sense with concept classes? I'm okay with modifying BGL code, but the remove_const makes me feel dirty.

This sounds like something that should be brought up with BGL maintainers...

Geoff

I believe that something is bad with the const version of bundled_properties. Andrew, what do you think?

This is the only report - to my knowledge - of this kind of problem. I'm not entirely that the best solution is to change the BGL. I'll have to take a good look tomorrow morning.
 

Andrew Sutton
andrew....@gmail.com

[Geoff Hilton]

Merry Christmas, Happy New Year and all that!
Any progress?

Thanks,
Geoff

[Andrew Sutton]

On Tue, Jan 13, 2009 at 9:56 AM, Andrew Sutton <andrew....@gmail.com> wrote:

Hi Andrew, apologies for writing directly to you, I just thought I'd try to get your attention this way since you hadn't yet replied to my last post on the above topic in the mailing list.

 

Sorry about that, I've been swamped since new years. I'm going to bounce this back to the list

I would avoid declaring property maps as const. Somewhere in your posted code you have:

const property_map<...>::const_type map; // or something similar.

For all property maps, map = get(...) is a valid expression, regardless of whether your map is a ::type or ::const_type. By declaring it const, and then instantiating a template with the const pmap, you're going to run into problems - probably the problem you reported earlier.

Andrew Sutton
andrew....@gmail.com

[Geoff Hilton]

Okay, so if I understand correctly I shouldn't use const prop maps with
const_type at all?

It seems to me like a line such as the above should still theoretically
compile. At least from the perspective of a concept check for
Readability; this is by definition what const is meant to restrict
objects to so in theory in should be allowable no? If I'm right it would
be a compilation error caused by the underlying implementation. Either
that or a debatable foible that should be documented? *shrug*.

[Andrew Sutton]

   const property_map<...>::const_type map; // or something similar.

   For all property maps, map = get(...) is a valid expression,
   regardless of whether your map is a ::type or ::const_type. By
   declaring it const, and then instantiating a template with the const
   pmap, you're going to run into problems - probably the problem you
   reported earlier.

 

Okay, so if I understand correctly I shouldn't use const prop maps with const_type at all?

It seems to me like a line such as the above should still theoretically compile. At least from the perspective of a concept check for Readability; this is by definition what const is meant to restrict objects to so in theory in should be allowable no? If I'm right it would be a compilation error caused by the underlying implementation. Either that or a debatable foible that should be documented? *shrug*.

I don't think you should be using const property maps at all - with type or const_type. For example:

/* 1 */ property_map<...>::const_type p;  // Good
/* 2 */ const property_map<...>::const_type p; // Bad

The const_type in 1 forces the p to operate on its underlying reference in a const way. Returing const references, no put() operation, etc. The leading const in 2 means that you can't write:

p = q; // Assuming q is type property_map<...>::type

It's a compiler error since p is not modifiable.

If you look at the concept definition in boost/property_map.hpp for ReadablePropertyMapConcept, you'll find, in the constraints() member, this line:

val = get(pmap, k)

So apparently, the concept definition actually requires that property maps are never const - which is admittedly a little weird.

Andrew Sutton
andrew....@gmail.com

[Geoff Hilton]

Andrew Sutton wrote:
> const property_map<...>::const_type map; // or something similar.
>
> For all property maps, map = get(...) is a valid expression,
> regardless of whether your map is a ::type or ::const_type. By
> declaring it const, and then instantiating a template with
> the const
> pmap, you're going to run into problems - probably the
> problem you
> reported earlier.
>
>
>
>
> Okay, so if I understand correctly I shouldn't use const prop maps
> with const_type at all?
>
> It seems to me like a line such as the above should still
> theoretically compile. At least from the perspective of a concept
> check for Readability; this is by definition what const is meant to
> restrict objects to so in theory in should be allowable no? If I'm
> right it would be a compilation error caused by the underlying
> implementation. Either that or a debatable foible that should be
> documented? *shrug*.
>
>
> I don't think you should be using const property maps at all - with type
> or const_type. For example:

Okay.

>
> /* 1 */ property_map<...>::const_type p; // Good
> /* 2 */ const property_map<...>::const_type p; // Bad
>
> The const_type in 1 forces the p to operate on its underlying reference
> in a const way. Returing const references, no put() operation, etc. The
> leading const in 2 means that you can't write:
>
> p = q; // Assuming q is type property_map<...>::type
>
> It's a compiler error since p is not modifiable.

I agree, both situations are perfectly reasonable.

>
> If you look at the concept definition in boost/property_map.hpp for
> ReadablePropertyMapConcept, you'll find, in the constraints() member,
> this line:
>
> val = get(pmap, k)
>
> So apparently, the concept definition actually requires that property
> maps are never const - which is admittedly a little weird.

It sounds like we're now on the same page! The above essentially means
that it becomes literally impossible to write perfectly const-correct
code (from a user standpoint). While const-incorrect code is bad enough,
it also means that I may be missing out on potential compiler
optimizations, that bugs me quite a bit more.

What may be done to remedy this?
>
> Andrew Sutton
> andrew....@gmail.com

[Andrew Sutton]

val = get(pmap, k)

So apparently, the concept definition actually requires that property maps are never const - which is admittedly a little weird.

It sounds like we're now on the same page! The above essentially means that it becomes literally impossible to write perfectly const-correct code (from a user standpoint). While const-incorrect code is bad enough, it also means that I may be missing out on potential compiler optimizations, that bugs me quite a bit more.

What may be done to remedy this?

I will admit that writing perfectly const-correct code can be very, very difficult in many situations :) However, I'll defer to more experienced Boosters with regards that its impossible. I will also say that const-incorrect code isn't necessarily "bad". There are times that ignoring const-ness is quite useful.

In this particular instance, I will cautiously say that the concept definition is implemented incorrectly. If the constraints() funciton did this:

property_map<Key, Graph>::type pm = get(...);

instead of:

pm = get(),

It would preserve the const'ness of the pmap object, and the compiler error might not exist.

I also wouldn't worry about compiler optimizations in this case. The compiler is going to elide all or most of the pmap copies and inline all off the function calls to the underlying data structures. Generally, you'll never see the statement (pm = get(...)) in Boost.Graph algorithms.

Andrew Sutton
andrew....@gmail.com

[Geoff Hilton]

Can this be fixed for 1.38?

[Andrew Sutton]

Can this be fixed for 1.38?

Too late for 1.38, and I'm not entirely sure it should be fixed.
 

Andrew Sutton
andrew....@gmail.com

[Geoff Hilton]

Andrew Sutton wrote:
>
> Can this be fixed for 1.38?
>
>
> Too late for 1.38,

Darn.

> and I'm not entirely sure it should be fixed.

Really? It sounds like the fix is very reasonable.
>
> Andrew Sutton
> andrew....@gmail.com

Oh and about the compiler optimizations in your previous message; good. :)

[Andrew Sutton]

Really? It sounds like the fix is very reasonable.

The problem with changing concept definitions is that you can't reliably predict the impact on other people's code. Maybe the constraints implementation is actually correct, and that property maps should always be copy assignable.

At some point this semester, I hope to find time to integrate my old SoC work from 2007. This includes a complete rewrite of the concept checking classes, so it may fix the problem.
 

Andrew Sutton
andrew....@gmail.com

[Geoff Hilton]

Andrew Sutton wrote:
> Really? It sounds like the fix is very reasonable.
>
>
> The problem with changing concept definitions is that you can't reliably

> predict the impact on other people's code.. Maybe the constraints

> implementation is actually correct, and that property maps should always
> be copy assignable.
>
> At some point this semester, I hope to find time to integrate my old SoC
> work from 2007. This includes a complete rewrite of the concept checking
> classes, so it may fix the problem.
>
> Andrew Sutton
> andrew....@gmail.com

Good enough! I hope it does. :)

Thanks,