Re: [Boost-users] [BGL] Bundled properties and property maps
On Tue, Jan 13, 2009 at 9:56 AM, Andrew Sutton <andrew.n.sutton@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.n.sutton@gmail.com
Andrew Sutton wrote:
On Tue, Jan 13, 2009 at 9:56 AM, Andrew Sutton <andrew.n.sutton@gmail.com <mailto:andrew.n.sutton@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.n.sutton@gmail.com
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*. Thanks, Geoff
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.n.sutton@gmail.com
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.n.sutton@gmail.com
Thanks, Geoff
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.n.sutton@gmail.com
Andrew Sutton wrote:
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.n.sutton@gmail.com
Can this be fixed for 1.38? Thanks, Geoff
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.n.sutton@gmail.com
Oh and about the compiler optimizations in your previous message; good. :) Geoff
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.n.sutton@gmail.com
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.n.sutton@gmail.com
Good enough! I hope it does. :) Thanks, Geoff
participants (1)
-
Andrew Sutton -
Geoff Hilton