Using a macro to simplify implementing non-const methods in terms of const overload
Martin T.
Implementing a non-const function in terms of it's const overload is an
accepted idea:
class foo {
public:
std::string & bar() {
return const_cast< std::string& >( static_cast<foo
const*>(this)->bar() );
}
const std::string & bar() const {
return bar_;
}
void mooh() {
static_cast<foo const*>(this)->mooh();
}
void mooh() const {
// mooh!
}
private:
std::string bar_;
};
However, I think the syntax is so horrible that I would rather have some
wrapper code for this.
I am currently considering the following:
(adapted from:
http://groups.google.com/group/comp.lang.c++.moderated/msg/d14308749babc833
)
...
namespace my {
template < typename T >
inline T const* toconst( T* t ) {
return t;
}
}
#define MY_CALL_CONST(RET_NAME, MEMBER_NAME) \
const_cast< RET_NAME >( my::toconst(this)-> MEMBER_NAME ) \
/**/
...
used like so:
...
std::string & bar() {
return MY_CALL_CONST( std::string&, bar() );
}
...
void mooh() {
my::toconst(this)->mooh();
}
...
I like this because:
* No repetition of class type name
* Says what it does, instead of cluttering the line with static_cast and
parentheses
* const_cast is hidden behind a hard to misuse macro, so if we search
for const_cast in the code base there are less false positives
So ...
* Having written some code that needed above idiom multiple times (e.g.
27 times in 2 files), did YOU use some wrapper approach, or did you use
the "plain" version. -- or didn't you use the idiom at all?
* Would YOU consider putting such a macro in your private toolset?
* Would you wrap my::toconst in a second macro for consistency?
cheers,
Martin
--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
Goran
> Hi all.
>
> Implementing a non-const function in terms of it's const overload is an
> accepted idea:
>
> class foo {
> public:
> std::string & bar() {
> return const_cast< std::string& >( static_cast<foo
> const*>(this)->bar() );
> }
> const std::string & bar() const {
> return bar_;
> }
> void mooh() {
> static_cast<foo const*>(this)->mooh();
> }
> void mooh() const {
> // mooh!
> }
> private:
> std::string bar_;
>
> };
>
> However, I think the syntax is so horrible that I would rather have some
> wrapper code for this.
>
> I am currently considering the following:
> (adapted
Frankly, I'd ask myself what I am doing wrong. Why is it that so many
const and non-const functions are the same?
For example, your bar() function allows clients to modify internal
string when they have a non-const foo&. That seems to be at odds with
"tell, don't ask" idea. Also, even if needed, that might be better
handled using single responsibility principle (http://
www.objectmentor.com/resources/articles/srp.pdf).
Or take mooh(). If we presume that one function does one thing
(reasonable, I'd say), how come it can be both const and non-const? It
either modifies object state, in which case it's non-const anyhow and
should not be called for a const object, or it doesn't, in which case
it's const and can be called for both?
And finally, did you consider "mutable" for certain members? (I've
found this to be a good solution).
HTH,
Goran.
Chris Morley
"Martin T." <0xCDC...@gmx.at> wrote in message
news:h70eg3$45i$1...@news.eternal-september.org...
> Hi all.
>
> Implementing a non-const function in terms of it's const overload is an
> accepted idea:
>
> class foo {
> public:
> std::string & bar() {
> return const_cast< std::string& >( static_cast<foo
> const*>(this)->bar() );
> }
> const std::string & bar() const {
> return bar_;
> }
> void mooh() {
> static_cast<foo const*>(this)->mooh();
> }
> void mooh() const {
> // mooh!
> }
> private:
> std::string bar_;
> };
>
> However, I think the syntax is so horrible that I would rather have some
> wrapper code for this.
Indeed. Why don't you put the implementation in the non-const member
function & then simply return that for the const one?
e.g.
class foo{
public:
std::string& bar() {return bar_;}
const std::string& bar() const {return bar();}
private:
std::string bar_;
};
Chris
Nick Hounsome
> Hi all.
>
> Implementing a non-const function in terms of it's const overload is an
> accepted idea:
>
Occasionally (maybe in your real class?) but your examples and most
others that I have come across are actually just bad design.
> class foo {
> public:
> std::string & bar() {
This is a bad example because it exposes the implementation of foo
(foo MUST contain a std::string).
Even worse it allows a client to maintain a reference to the guts of a
foo after the object has been destroyed:
foo* fp = new foo;
std::string& s( fp->bar() );
delete fp;
s.length(); // ooops! Undefined behaviour.
To keep the implementation hidden you must instead have something like
"void setbar(const std::string&)" and your use of the const bar()
would nolonger be relevant. Contrariwise, if foo were really intended
to be just a glorified struct then bar_ should be public and you would
have no need for either method.
> return const_cast< std::string& >( static_cast<foo
> const*>(this)->bar() );
> }
> const std::string & bar() const {
> return bar_;
> }
> void mooh() {
This method is NEVER necessary - Just delete it and nobody will ever
know the difference.
(Okay I can think of one reason to have it but it's pretty obscure and
I've never seen it needed in practice).
> static_cast<foo const*>(this)->mooh();
> }
> void mooh() const {
> // mooh!
> }
> private:
> std::string bar_;
>
> };
Mathias Gaunard
> Hi all.
>
> Implementing a non-const function in terms of it's const overload is an
> accepted idea
Really? I thought no one enforced the use of casting out const except
for compatibility with legacy systems.
Martin T.
Great big no-no. I made the same made here:
http://groups.google.com/group/comp.lang.c++.moderated/msg/f50ed3daf6a4726e
1.) You must not call a non-const member function from a const one.
2.) your code above results in endless recursion as bar()-const will
call the const version of bar().
cheers,
Martin
Martin T.
> On 25 Aug, 12:41, "Martin T." <0xCDCDC...@gmx.at> wrote:
>> Hi all.
>>
>> Implementing a non-const function in terms of it's const overload is an
>> accepted idea:
>>
>
> Occasionally (maybe in your real class?) but your examples and most
> others that I have come across are actually just bad design.
>
Hmmm ... you mean to say that it is a sign of bad design if a member
function needs a cost and non-const version ??
>> class foo {
>> public:
>> std::string & bar() {
>
> This is a bad example because it exposes the implementation of foo
> (foo MUST contain a std::string).
> Even worse it allows a client to maintain a reference to the guts of a
> foo after the object has been destroyed:
>
> foo* fp = new foo;
> std::string& s( fp->bar() );
> delete fp;
> s.length(); // ooops! Undefined behaviour.
>
> To keep the implementation hidden you must instead have something like
> "void setbar(const std::string&)" and your use of the const bar()
> would nolonger be relevant. Contrariwise, if foo were really intended
> to be just a glorified struct then bar_ should be public and you would
> have no need for either method.
>
While I don't agree that the reference returning is such a bad thing,
you do raise a valid point. Namely whether it's necessary to have such
accessors in the first place.
There are valid cases where we want to return a pointer/reference/handle
like thing from a mem.fun. and for these cases I think it makes sense to
provide a const + non-const version.
One example are the std library containers -- Consider this snippet from
std::vector from the STL supplied with Visual Studio 2008 Express:
// vector, line 758:
const_reference operator[](size_type _Pos) const
{ // subscript nonmutable sequence
#if _HAS_ITERATOR_DEBUGGING
if (size() <= _Pos)
{
_DEBUG_ERROR("vector subscript out of range");
_SCL_SECURE_OUT_OF_RANGE;
}
#endif /* _HAS_ITERATOR_DEBUGGING */
_SCL_SECURE_VALIDATE_RANGE(_Pos < size());
return (*(_Myfirst + _Pos));
}
reference operator[](size_type _Pos)
{ // subscript mutable sequence
#if _HAS_ITERATOR_DEBUGGING
if (size() <= _Pos)
{
_DEBUG_ERROR("vector subscript out of range");
_SCL_SECURE_OUT_OF_RANGE;
}
#endif /* _HAS_ITERATOR_DEBUGGING */
_SCL_SECURE_VALIDATE_RANGE(_Pos < size());
return (*(_Myfirst + _Pos));
}
I think it would kind of make sense to use the solution with the
const_cast instead of 1:1 duplicating 9 lines of code.
Microsoft (or whoever their STL comes from) didn't, maybe they had a
reason. My initial posting was trying to find out. :-)
>> return const_cast< std::string& >( static_cast<foo
>> const*>(this)->bar() );
>> }
>> const std::string & bar() const {
>> return bar_;
>> }
>> void mooh() {
>
> This method is NEVER necessary - Just delete it and nobody will ever
> know the difference.
Agreed. I was just over eager to supply a completeish example.
cheers,
Martin
Pete Becker
> "Martin T." <0xCDC...@gmx.at> wrote in message
> news:h70eg3$45i$1...@news.eternal-september.org...
>> Hi all.
>>
>> Implementing a non-const function in terms of it's const overload is an
>> accepted idea:
>>
>>
>> However, I think the syntax is so horrible that I would rather have some
>> wrapper code for this.
>
> Indeed. Why don't you put the implementation in the non-const member
> function & then simply return that for the const one?
>
> e.g.
>
> class foo{
> public:
> std::string& bar() {return bar_;}
> const std::string& bar() const {return bar();}
> private:
> std::string bar_;
> };
>
Um, because it doesn't work? <g> As written, the const version of bar()
recurses until the program crashes. And if you use a cast to call the
non-const version, you end up with the possibility of a const member
function modifying data (through the call to the non-const bar()).
--
Pete
Roundhouse Consulting, Ltd. (www.versatilecoding.com) Author of
"The Standard C++ Library Extensions: a Tutorial and Reference"
(www.petebecker.com/tr1book)
Martin T.
> On 25 ao�t, 13:41, "Martin T." <0xCDCDC...@gmx.at> wrote:
>> Hi all.
>>
>> Implementing a non-const function in terms of it's const overload is an
>> accepted idea
>
> Really? I thought no one enforced the use of casting out const except
> for compatibility with legacy systems.
>
Well. It certainly makes sense to me to avoid code duplication.
Scott Meyers seems to think the same:
Effective C++, 3rd ed - Item 3 (use const):
[q p23] Avoid duplication in const and non-const members f. [/q]
[q p26] When const and non-const m.f. have [the same] implementations,
code duplication can be avoided by having the the non-const version call
the const version. [/q]
br,
Martin
SG
>
> [...] Why don't you put the implementation in the non-const member
> function & then simply return that for the const one?
>
> e.g.
>
> class foo
> {
> public:
> std::string& bar() {return bar_;}
> const std::string& bar() const {return bar();}
> private:
> std::string bar_;
> };
It compiles. But it doesn't do what you think. It's an infinite
recursion. You get the behaviour you want by writing
class foo
{
public:
std::string& bar() {return bar_;}
const std::string& bar() const
{
return const_cast<foo*>(this)->bar();
}
private:
std::string bar_;
};
But that's really a very bad idea because
(*) it's easier to accidentally violate const correctness
(*) if you accidentally modify something in the non-const version
of bar() you will invoke undefined behaviour in cases like
const foo f; // f is really const!
f.bar(); // Ooops!
Actually, I wrote a KD-tree data structure with a multimap-like
interface a couple of weeks ago. I wanted to have a nearest-neighbour-
search in both versions: const and non-const. It looks like this:
class kdtree_node
{
...
value_type const* nnsearch(point_type const& p,
Real & maxssd) const;
value_type * nnsearch(point_type const& p,
Real & maxssd)
{
kdtree_node const* const me = this;
value_type const* const ret = me->nnsearch(p,maxssd);
return const_cast<value_type*>(ret);
}
...
};
It's not pretty but IMHO reasonably safe. I wonder how std::map::find
is implemented...
Cheers!
SG
Balog Pal
"Nick Hounsome"
> Occasionally (maybe in your real class?) but your examples and most
> others that I have come across are actually just bad design.
> This is a bad example because it exposes the implementation of foo
> (foo MUST contain a std::string).
> Even worse it allows a client to maintain a reference to the guts of a
> foo after the object has been destroyed:
Yeah, there are guidelines to avoid this when possible -- but for the rest
of the cases it is good and natural. For classes that are just
holders/collections/aggregations.
Is std::vector 'bad design' for having const and non-const accessors?
Sure, pointers and refs come with lifetime issues attached, but it shall be
pretty common for any C/C++ user, and whoever can't cope with the thing (in
general) is just not fit as a programmer.
>
> foo* fp = new foo;
> std::string& s( fp->bar() );
> delete fp;
> s.length(); // ooops! Undefined behaviour.
>
> To keep the implementation hidden you must instead have something like
> "void setbar(const std::string&)"
You mean
void setbar(std::string)
that actually prevents aliasing and a possible storing of the address or
binding to a ref... As long as we're addicted to 'efficiency' so
sacrificing the separation, the point is kinda moot.
The set() interface looks good as long as the argument is simple enough, but
what if it is a map<string, vector< > > and we'd just want to change a
single item in it?
Exposing the internal type is a problem only if it will actually change --
and the cure of abstraction and writing forwarders may be deals worse than
the disease. :)
Nick Hounsome
> Nick Hounsome wrote:
> > On 25 Aug, 12:41, "Martin T." <0xCDCDC...@gmx.at> wrote:
> >> Hi all.
>
> >> Implementing a non-const function in terms of it's const overload is an
> >> accepted idea:
>
> > Occasionally (maybe in your real class?) but your examples and most
> > others that I have come across are actually just bad design.
>
> Hmmm ... you mean to say that it is a sign of bad design if a member
> function needs a cost and non-const version ??
I didn't say always - but usually.
The only time I can remember doing it myself (my memory isn't great)
is for tree navigation. (eg. node* parent()/const node* parent()
const)).
Note that the motivation here is not to have simultaneous const and
non-const access to a node but to maintain the logical constness of
the containing entity (stupid people will write just "node* parent()
const" and it will be fine with the compiler but logically wrong).
> >> class foo {
> >> public:
> >> std::string & bar() {
>
> > This is a bad example because it exposes the implementation of foo
> > (foo MUST contain a std::string).
> > Even worse it allows a client to maintain a reference to the guts of a
> > foo after the object has been destroyed:
>
> > foo* fp = new foo;
> > std::string& s( fp->bar() );
> > delete fp;
> > s.length(); // ooops! Undefined behaviour.
>
> > To keep the implementation hidden you must instead have something like
> > "void setbar(const std::string&)" and your use of the const bar()
> > would nolonger be relevant. Contrariwise, if foo were really intended
> > to be just a glorified struct then bar_ should be public and you would
> > have no need for either method.
>
> While I don't agree that the reference returning is such a bad thing,
> you do raise a valid point.
Why not? You haven't addressed my reasons.
Do you think it is OK to constrain the internals of a class by the
interface used?
Do you not agree that my example of undefined behaviour is
undesirable?
The other advantage of setter methods over private variable access is
that you can track and/or validate changes.
If you return a reference to your bar_ and at some point you discover
that it is wrong then how do you track down where it changed? With
setbar(const std::string&) you can just set a breakpoint. With your
implementation it is much harder.
Or suppose that a new requirement is introduced that says that bar_
must always be upper case. With a setter you can either validate or
force it. Without a setter you have nothing.
> Namely whether it's necessary to have such
> accessors in the first place.
>
> There are valid cases where we want to return a pointer/reference/handle
> like thing from a mem.fun. and for these cases I think it makes sense to
> provide a const + non-const version.
> One example are the std library containers -- Consider this snippet from
> std::vector from the STL supplied with Visual Studio 2008 Express:
> // vector, line 758:
But this is completely different from your example.
A vector<T> MUST logically contains some T so returning a reference
doesn't expose anything that we could implement in any other way.
What we are hiding in vector is not the T itself but the details of
how more than one of them are held (plus giving some checking).
Your foo example holds a logical string which (in the absence of other
design info) need not necessarily be implemented as a std::string but
because bar() returns a reference it is impossible alter foo to use
char* instead (or CString if you're into Windows) without changing the
declaration of bar().
> const_reference operator[](size_type _Pos) const
> { // subscript nonmutable sequence
>
> #if _HAS_ITERATOR_DEBUGGING
> if (size() <= _Pos)
> {
> _DEBUG_ERROR("vector subscript out of range");
> _SCL_SECURE_OUT_OF_RANGE;
> }
> #endif /* _HAS_ITERATOR_DEBUGGING */
> _SCL_SECURE_VALIDATE_RANGE(_Pos < size());
>
> return (*(_Myfirst + _Pos));
> }
>
> reference operator[](size_type _Pos)
> { // subscript mutable sequence
>
> #if _HAS_ITERATOR_DEBUGGING
> if (size() <= _Pos)
> {
> _DEBUG_ERROR("vector subscript out of range");
> _SCL_SECURE_OUT_OF_RANGE;
> }
> #endif /* _HAS_ITERATOR_DEBUGGING */
> _SCL_SECURE_VALIDATE_RANGE(_Pos < size());
>
> return (*(_Myfirst + _Pos));
> }
>
> I think it would kind of make sense to use the solution with the
> const_cast instead of 1:1 duplicating 9 lines of code.
>
In this case you could.
If you did then in general you would have to implement the const
method in terms of the non-const method rather than the other way
around - This is because, for example, for general iterators rather
than just T*, it is not possible to convert from const_iterator to
iterator - You then run into the problem of potentially undefined
behaviour again.
Martin T.
> On 25 Aug, 21:39, "Martin T." <0xCDCDC...@gmx.at> wrote:
>> Nick Hounsome wrote:
>>> On 25 Aug, 12:41, "Martin T." <0xCDCDC...@gmx.at> wrote:
>>>> Hi all.
>>>> Implementing a non-const function in terms of it's const overload is an
>>>> accepted idea:
>>> Occasionally (maybe in your real class?) but your examples and most
>>> others that I have come across are actually just bad design.
>>>> class foo {
>>>> public:
>>>> std::string & bar() {
>>> This is a bad example because it exposes the implementation of foo
>>> (foo MUST contain a std::string).
>>> Even worse it allows a client to maintain a reference to the guts of a
>>> foo after the object has been destroyed:
>>> foo* fp = new foo;
>>> std::string& s( fp->bar() );
>>> delete fp;
>>> s.length(); // ooops! Undefined behaviour.
>>> To keep the implementation hidden you must instead have something like
>>> "void setbar(const std::string&)" and your use of the const bar()
>>> would nolonger be relevant. Contrariwise, if foo were really intended
>>> to be just a glorified struct then bar_ should be public and you would
>>> have no need for either method.
>> While I don't agree that the reference returning is such a bad thing,
>> you do raise a valid point.
>
> Why not? You haven't addressed my reasons.
> Do you think it is OK to constrain the internals of a class by the
> interface used?
> Do you not agree that my example of undefined behaviour is
> undesirable?
>
I haven't addressed these points because I think they are not relevant
to the question of my original post. (Which was about avoiding code
duplication *if* there is some duplicated code.)
> The other advantage of setter methods over private variable access is
> that you can track and/or validate changes.
> If you return a reference to your bar_ and at some point you discover
> that it is wrong then how do you track down where it changed? With
> setbar(const std::string&) you can just set a breakpoint. With your
> implementation it is much harder.
> Or suppose that a new requirement is introduced that says that bar_
> must always be upper case. With a setter you can either validate or
> force it. Without a setter you have nothing.
>
I agree that with my foo-bar example your setter approach makes more sense.
>> Namely whether it's necessary to have such
>> accessors in the first place.
>>
>> There are valid cases where we want to return a pointer/reference/handle
>> like thing from a mem.fun. and for these cases I think it makes sense to
>> provide a const + non-const version.
>> One example are the std library containers -- Consider this snippet from
>> std::vector from the STL supplied with Visual Studio 2008 Express:
>> // vector, line 758:
>
> But this is completely different from your example.
> A vector<T> MUST logically contains some T so returning a reference
> ...
I agree that my example makes more sense with a get+set approach, but it
was just a simple example.
Let's say I rephrase the question: If you were implementing such
collections and you had to copy&paste your operator[] code in every
class, would you then consider the const_cast approach ?
>
>> ...
>>
>> I think it would kind of make sense to use the solution with the
>> const_cast instead of 1:1 duplicating 9 lines of code.
>>
>
> In this case you could.
>
> If you did then in general you would have to implement the const
> method in terms of the non-const method rather than the other way
> around - This is because, for example, for general iterators rather
> than just T*, it is not possible to convert from const_iterator to
> iterator - You then run into the problem of potentially undefined
> behaviour again.
>
Well if const_iterator and iterator were two unrelated types, the
const_cast would fail anyway, so we wouldn't hit undef.behaviour because
it wouldn't compile.
Bart van Ingen Schenau
>
> "Nick Hounsome"
>> Occasionally (maybe in your real class?) but your examples and most
>> others that I have come across are actually just bad design.
> ...
>> This is a bad example because it exposes the implementation of foo
>> (foo MUST contain a std::string).
>> Even worse it allows a client to maintain a reference to the guts of
>> a foo after the object has been destroyed:
> ...
>
> Yeah, there are guidelines to avoid this when possible -- but for the
> rest
> of the cases it is good and natural. For classes that are just
> holders/collections/aggregations.
>
> Is std::vector 'bad design' for having const and non-const accessors?
No, because std::vector (and other containers) is an exception to the
rule of not returning references to your innards.
The rule, as I would formulate it, is that member functions of a class
shall not return a (non-const) reference to a member, unless the
semantics of the class dictate that such a member must be present.
To take your example of std::map<std::string, std::vector<T>>, that
class can have member functions returning a std::string& or a
std::vector<T>&, but NOT a std::pair<std::string, std::vector<T>>&,
because the use of std::pair is an implementation detail that is not
mandated by the semantics of std::map.
Bart v Ingen Schenau
--
a.c.l.l.c-c++ FAQ: http://www.comeaucomputing.com/learn/faq
c.l.c FAQ: http://c-faq.com/
c.l.c++ FAQ: http://www.parashift.com/c++-faq-lite/
Nick Hounsome
> Nick Hounsome wrote:
[lots removed to focus on one issue]
>
> > But this is completely different from your example.
> > A vector<T> MUST logically contains some T so returning a reference
> > ...
>
> I agree that my example makes more sense with a get+set approach, but it
> was just a simple example.
> Let's say I rephrase the question: If you were implementing such
> collections and you had to copy&paste your operator[] code in every
> class, would you then consider the const_cast approach ?
In this particular case I would consider it (I think I said so
earlier)....but what I'd actually do is implement BOTH in terms of a
private method "private: reference getref(size_t) const" - No explicit
casting is required and both operator[] methods are one liners.
Note that this castless approach is possible here but not your foo
example because the thing that you will return a reference to is not
actually a member of vector and so no const casting is needed. getref
violates logical const correctness but that's okay because it's a
private implementation detail.
...
>
> >> I think it would kind of make sense to use the solution with the
> >> const_cast instead of 1:1 duplicating 9 lines of code.
>
> > In this case you could.
See - I agreed with you :-)
>
> > If you did then in general you would have to implement the const
> > method in terms of the non-const method rather than the other way
> > around - This is because, for example, for general iterators rather
> > than just T*, it is not possible to convert from const_iterator to
> > iterator - You then run into the problem of potentially undefined
> > behaviour again.
>
> Well if const_iterator and iterator were two unrelated types, the
> const_cast would fail anyway, so we wouldn't hit undef.behaviour because
> it wouldn't compile.
>
I was just making a more general point - If you applied your approach
to methods returning iterators then you would have to reverse the
implementation dependency which would be confusing.
Chris Morley
> > {
> > public:
> > std::string& bar() {return bar_;}
> > const std::string& bar() const {return bar();}
> > private:
> > std::string bar_;
> > };
>
> It compiles. But it doesn't do what you think. It's an infinite
> recursion. You get the behaviour you want by writing
Yeah sorry, FAIL! VS2008 would have given me a compile time warning had I
thought before typing.
> class foo
> {
> public:
> std::string& bar() {return bar_;}
>
> const std::string& bar() const
> {
> return const_cast<foo*>(this)->bar();
> }
> private:
> std::string bar_;
> };
^ This _is_ what I intended.
> But that's really a very bad idea because
> (*) it's easier to accidentally violate const correctness
> (*) if you accidentally modify something in the non-const version
> of bar() you will invoke undefined behaviour in cases like
>
> const foo f; // f is really const!
> f.bar(); // Ooops!
I'm not sure it is all that bad because you'd only want such a construct in
a situation where you are doing something trivial like returning a
reference/pointer to a structure/class. Perhaps you have a usage counter but
otherwise the non-const version would be as trivial as "return x;". More
complexity with different requirements for const & non-const function
version should hint that they need different function names.
Here is a slightly less trivial but still similar complete program.
#include "stdio.h"
#include <string>
#include <vector>
struct Thing {
Thing() : k(0), s("init") {}
int k;
std::string s;
};
class foo {
public:
foo() : _Count(0) {_V.resize(3);}
Thing& bar(int n) {++_Count; return _V[n];}
const Thing& bar(int n) const {return const_cast<foo*>(this)->bar(n);}
void Display() const
{
printf("count=%d\n",_Count);
for (std::vector<Thing>::size_type i=0;i<_V.size();i++) printf("%d: k=%d,
s=%s\n",i,_V[i].k,_V[i].s.c_str());
}
private:
std::vector<Thing> _V;
int _Count;
};
int main(int argc, char* argv[])
{
const foo t;
t.Display();
const Thing& thing1=t.bar(1);
t.Display();
// lets force a cast just because we can
Thing& thing2 = const_cast<foo*>(&t)->bar(2);
thing2.s="oops?";
t.Display();
}
I don't see the problem or the UB - seriously please tell me if there is a
problem here. It behaves as intended with VS2008 & GCC even the forced
removal of the const. Can you give me a realistic example of the "oops" with
casting through to the non-const bar()?
Regards,
Chris
SG
> SG wrote:
> > class foo
> > {
> > public:
> > std::string& bar() {return bar_;}
>
> > const std::string& bar() const
> > {
> > return const_cast<foo*>(this)->bar();
> > }
> > private:
> > std::string bar_;
> > };
>
> ^ This _is_ what I intended.
>
> > But that's really a very bad idea because
> > (*) it's easier to accidentally violate const correctness
> > (*) if you accidentally modify something in the non-const version
> > of bar() you will invoke undefined behaviour in cases like
>
> > const foo f; // f is really const!
> > f.bar(); // Ooops!
>
> I'm not sure it is all that bad because you'd only want such a construct
It IS very very bad for the reasons I mentioned above.
> in a situation where you are doing something trivial like returning a
> reference/pointer to a structure/class. Perhaps you have a usage counter but
> otherwise the non-const version would be as trivial as "return x;". More
> complexity with different requirements for const & non-const function
> version should hint that they need different function names.
I don't understand why you prefer to risk const correctness violations
and/or UB. You don't have the problems I mentioned above if you write
the non-const version in terms of the const version.
> Here is a slightly less trivial but still similar complete program.
>
> #include "stdio.h"
> #include <string>
> #include <vector>
>
> struct Thing {
> Thing() : k(0), s("init") {}
> int k;
> std::string s;
> };
>
> class foo {
> public:
> foo() : _Count(0) {_V.resize(3);}
>
> Thing& bar(int n) {++_Count; return _V[n];}
> const Thing& bar(int n) const {return const_cast<foo*>(this)->bar(n);}
Did I already say that this is a very VERY bad idea? (hint hint)
You convert a foo const* to a foo*. This allows you to accidentally
violate const correctness and leads to UB if you modify the object's
state in case it REALLY is a const object.
> private:
> std::vector<Thing> _V;
> int _Count;
> };
>
> int main(int argc, char* argv[])
> {
> const foo t;
> const Thing& thing1=t.bar(1);
Undefined behaviour! You're trying to modify t._Count but t IS CONST!
Alternativly you could have written:
foo s;
const foo& t = s;
const Thing& thing1=t.bar(1);
This is NOT UB because t ist just a const ref to s but s is non-const.
Still, it's a very VERY bad idea for reasones that have been mentioned
a couple of times by now.
Another example just for you:
int x = 23;
int const y = 42;
int const& z = x;
const_cast<int&>(y) = 99; // UB!
const_cast<int&>(z) = 99; // Ugly but fine
Cheers!
SG
Felipe Magno de Almeida
wrote:
> Balog Pal wrote:
[snip]
> To take your example of std::map<std::string, std::vector<T>>, that
> class can have member functions returning a std::string& or a
> std::vector<T>&, but NOT a std::pair<std::string, std::vector<T>>&,
> because the use of std::pair is an implementation detail that is not
> mandated by the semantics of std::map.
Isn't it what *begin() returns in std::map?
Though I think it shouldn't be so for the same reasons.
> Bart v Ingen Schenau
> --
--
Bart van Ingen Schenau
>> class foo
>> {
>> public:
>> std::string& bar() {return bar_;}
>>
>> const std::string& bar() const
>> {
>> return const_cast<foo*>(this)->bar();
>> }
>> private:
>> std::string bar_;
>> };
>
> ^ This _is_ what I intended.
>
>> But that's really a very bad idea because
>> (*) it's easier to accidentally violate const correctness
>> (*) if you accidentally modify something in the non-const version
>> of bar() you will invoke undefined behaviour in cases like
>>
>> const foo f; // f is really const!
>> f.bar(); // Ooops!
>
> I'm not sure it is all that bad because you'd only want such a
> construct in a situation where you are doing something trivial like
> returning a reference/pointer to a structure/class.
No matter how trivial the code, applying a const_cast<Foo*> to an object
defined as 'const Foo bar;' already lands you in the land of UB.
After that point, whatever the compiler does to your code, it is correct
by definition.
The compiler is fully within its right to mark the memory page that
contains the object t as being read-only. If it chooses to do so, any
usage of the function foo:bar() will most likely result in a crash.
And even if it does not choose to make t physically read-only, the
compiler can do whatever it wants when you cast away the constness.
>
> t.Display();
>
> const Thing& thing1=t.bar(1);
>
> t.Display();
>
> // lets force a cast just because we can
> Thing& thing2 = const_cast<foo*>(&t)->bar(2);
> thing2.s="oops?";
>
> t.Display();
> }
>
> I don't see the problem or the UB - seriously please tell me if there
> is a problem here. It behaves as intended with VS2008 & GCC even the
> forced removal of the const.
That is the biggest problem with most instances of UB: The compilers do
not diagnose it and in small test programs they seem to give sensible
results.
> Can you give me a realistic example of
> the "oops" with casting through to the non-const bar()?
When used with a compiler that targets really memory-constrained
devices, it is not unreasonable to expect that t will be allocated in
ROM, which, depending on the hardware layout, may cause very bizarre
effects when calling foo::bar.
>
> Regards,
> Chris
>
Bart v Ingen Schenau
--
a.c.l.l.c-c++ FAQ: http://www.comeaucomputing.com/learn/faq
c.l.c FAQ: http://c-faq.com/
c.l.c++ FAQ: http://www.parashift.com/c++-faq-lite/
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
Chris Morley
> No matter how trivial the code, applying a const_cast<Foo*> to an object
> defined as 'const Foo bar;' already lands you in the land of UB.
> After that point, whatever the compiler does to your code, it is correct
> by definition.
Fair enough. The standard does say UB esp. 7.1.5.1 & const_cast bit.
> The compiler is fully within its right to mark the memory page that
> contains the object t as being read-only. If it chooses to do so, any
> usage of the function foo:bar() will most likely result in a crash.
> And even if it does not choose to make t physically read-only, the
> compiler can do whatever it wants when you cast away the constness.
In real life this isn't going to happen though is it? The class has a
non-trivial constuctor and the instance must be a temporary. It is OK for
the constructor to modify the members - requiring proper writable storage.
12.1
4 A constructor shall not be virtual (10.3) or static (9.4). A constructor
can be invoked for a const, volatile or
const volatile object. A constructor shall not be declared const, volatile,
or const volatile (9.3.2). const
and volatile semantics (7.1.5.1) are not applied on an object under
construction. They come into effect when the
constructor for the most derived object (1.8) ends.
so..
class foo {
public:
foo():_Count(0) {++_Count;}
int count() const {return _Count;}
int _Count; // doesn't even need a mutable
};
int main()
{
const foo f;
int k=f.count();
}
Is well formed isn't it?
Ok, to do the const_cast is UB according to the spec. but it simply must be
a proper object with real writable storage. So while it might be strictly
"UB" the behaviour is determined. It may be ill advised but it will work.
Who would do such a thing with a member which isn't trivial anyway - that is
asking for trouble.
Perhaps the extreme trivial example is bad anyway because it is less typing
to reimplement the functions...
(make _Count mutable)
Thing& bar(int n) {++_Count; return _V[n];}
const Thing& bar(int n) const {++_Count; return _V[n];}
is shorter than casting anyway. Even more so if you don't have a count.
It does have it's uses though. I've used it in a "lazy load" wrapper for
accessing a pointer to a runtime polymorphic classes instantiated from DB
data (amongst other function). The DB load is the slow part so that bit only
runs if the pointer to the constructed class (based on db data) is actually
used. The wrapper is compile time constant size and has no heap storage - a
trait I want. I know all decls are temporary (non-const) or in e.g. vector.
Some code gets passed a const& to the wrapper. It was simplest to const_cast
just the const member but with hind sight I suppose I could have consted
everything and mutabled all the member variables - more typing :(
I know this const_cast is safe but mutabling everything needed means it is
easier to "oops" in another fuction which _shouldn't_ be allowed to modify
them. Lesser of two weivels.
Chris
--
Bart van Ingen Schenau
> On Aug 26, 10:17 pm, Bart van Ingen Schenau <b...@ingen.ddns.info>
> wrote:
>> Balog Pal wrote:
>
> [snip]
>
>> To take your example of std::map<std::string, std::vector<T>>, that
>> class can have member functions returning a std::string& or a
>> std::vector<T>&, but NOT a std::pair<std::string, std::vector<T>>&,
>> because the use of std::pair is an implementation detail that is not
>> mandated by the semantics of std::map.
>
> Isn't it what *begin() returns in std::map?
<nitpick-mode=on>
No, std::map<std::string, std::vector<T>>::iterator::operator*() returns
a std::map<std::string, std::vector<T>>::value_type&, which is a typedef
for std::pair<const std::string, std::vector<T>>&.
<nitpick-mode=off>
> Though I think it shouldn't be so for the same reasons.
On the one hand, it is an over-specification of std::map.
On the other hand, it was probably added to make std::map fit in the
same mould as the other containers (which store only single values, not
pairs of them).
>
>> Bart v Ingen Schenau
>> --
>
>
Bart v Ingen Schenau
--
a.c.l.l.c-c++ FAQ: http://www.comeaucomputing.com/learn/faq
c.l.c FAQ: http://c-faq.com/
c.l.c++ FAQ: http://www.parashift.com/c++-faq-lite/
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
Bart van Ingen Schenau
> Hello Bart,
Hello Chris,
>> The compiler is fully within its right to mark the memory page that
>> contains the object t as being read-only. If it chooses to do so, any
>> usage of the function foo:bar() will most likely result in a crash.
>> And even if it does not choose to make t physically read-only, the
>> compiler can do whatever it wants when you cast away the constness.
>
> In real life this isn't going to happen though is it? The class has a
> non-trivial constuctor and the instance must be a temporary. It is OK
> for the constructor to modify the members - requiring proper writable
> storage. 12.1
Who know what the next compiler is going to do.
Also note that on modern processors, a memory page can be marked as
read-only at any time. So the compiler can give that instruction right
after the constructor finished.
And perhaps a next generation of processors allows a much finer
granularity in what is marked as read-only, read-write and/or
executable.
<snip>
> so..
> class foo {
> public:
> foo():_Count(0) {++_Count;}
> int count() const {return _Count;}
> int _Count; // doesn't even need a mutable
> };
> int main()
> {
> const foo f;
> int k=f.count();
> }
>
> Is well formed isn't it?
Yes, that is well-formed code that must initialise k with the value 1.
>
> Ok, to do the const_cast is UB according to the spec. but it simply
> must be a proper object with real writable storage.
No, that is not true. Under the "as-if" rule, if a conforming program
(without UB!) can't tell the difference, the compiler is free to do what
it likes.
So, in this example, the compiler can transform the code to:
int main()
{
int k=1;
}
And even if the address of f was taken (so the object can't be
eliminated entirely), f could have been allocated in read-only storage
(by transforming the constructor that _Count is initialised to 1).
> So while it might
> be strictly "UB" the behaviour is determined. It may be ill advised
> but it will work. Who would do such a thing with a member which isn't
> trivial anyway - that is asking for trouble.
Well, rule number 1 for programmers should be: "Don't try to outsmart
your compiler. It _will_ get its revenge on you."
I consider this a prime example of trying to outsmart your compiler.
<snip>
> It does have it's uses though.
Sure. Otherwise it would not have been part of the language. :-)
> I've used it in a "lazy load" wrapper
> for accessing a pointer to a runtime polymorphic classes instantiated
> from DB data (amongst other function). The DB load is the slow part so
> that bit only runs if the pointer to the constructed class (based on
> db data) is actually used. The wrapper is compile time constant size
> and has no heap storage - a trait I want. I know all decls are
> temporary (non-const) or in e.g. vector. Some code gets passed a
> const& to the wrapper. It was simplest to const_cast just the const
> member but with hind sight I suppose I could have consted everything
> and mutabled all the member variables - more typing :( I know this
> const_cast is safe but mutabling everything needed means it is easier
> to "oops" in another fuction which _shouldn't_ be allowed to modify
> them. Lesser of two weivels.
As you know that in that case all objects are physically non-const, the
const_cast is safe.
And as the load_from_db function will modify just about every member of
the class and it is the only function that should do so when used on a
const-qualified reference, using a cons_cast there is entirely
appropriate IMO.
>
> Chris
>
Bart v Ingen Schenau
--
a.c.l.l.c-c++ FAQ: http://www.comeaucomputing.com/learn/faq
c.l.c FAQ: http://c-faq.com/
c.l.c++ FAQ: http://www.parashift.com/c++-faq-lite/
[ See http://www.gotw.ca/resources/clcm.htm for info about ]