logging-log4cxx-user mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Scott McCaskill" <scott.mccask...@gmail.com>
Subject Re: ObjectPtrT::operator-> const problem
Date Thu, 08 Nov 2007 19:40:06 GMT
On Nov 7, 2007 5:10 PM, Curt Arnold <carnold@apache.org> wrote:

>
> On Nov 7, 2007, at 3:39 PM, Scott McCaskill wrote:
>
> > I've noticed the const version of this method is returning a
> > pointer to const.  The reason this is a problem is that it makes it
> > impossible to log from const methods when LoggerPtr is a member
> > variable:
> >
> > class C
> > {
> >     public:
> >         C() : logger(log4cxx::Logger::getLogger("my logger")) { }
> >
> >         void constMethod() const
> >         {
> >             // Error: can't do this because the const version of
> > ObjectPtrT::operator->()
> >             // must be used, and it returns a pointer to const T.
> >             LOG4CXX_DEBUG(logger, "debug message");
> >         }
> >
> >     private:
> >         log4cxx::LoggerPtr logger;
> > };
> >
> > In both versions of operator->, the ObjectPtrT itself cannot be
> > modified, so it's not clear why there is a need for a non-const
> > version, or for a version that returns a pointer to const.  It
> > seems that in this case, the notion of pointer constness is being
> > conflated with the notion of pointee constness.
> >
> > Surely the above example is not such an unreasonable a thing to
> > expect to be able to do? :)
> >
> > BTW, I wasn't sure whether this was more appropriate for the user
> > list or the dev list (I'm subscribed to both).
> >
> > Scott McCaskill
> >
>
> log4cxx-dev is likely the more appropriate since things will quickly
> delve into minutiae that most users won't care to follow, but I think
> most people who could contribute subscribe to both.
>
> I was around that code about a week ago and had the same reaction.  I
> did a naive quick fix on my local copy and everything fell apart, so
> I quickly reverted that change and moved on to other things.  I
> believe that const logging methods has been discussed before either
> on the list or in a bug report and might be something to research to
> see if any earlier discussion is helpful.
>

> My quick gut reaction is:
>
> a) your analysis is likely right
>
> b) fixing it would likely require creating an alternative template
> that takes a pointer parameter, something like:
>
> typedef log4cxx::helpers::ObjectPtrT2<const Logger*> ConstLoggerPtr;
> typedef log4cxx::helpers::ObjectPtrT2<Logger*> LoggerPtr;
>
> I wouldn't call it ObjectPtrT2, but I thought that was clearer for
> this discussion.
>
> Changing the parameterization parameter for the existing template
> would have too much affect on client code.
>
> c) Copying between pointer classes (for example, LoggerPtr,
> ConstLoggerPtr and ObjectPtrT<Logger>) would need to be supported as
> appropriate.
>
> d) Methods that don't (obviously) change the state (logger->info()
> etc) would benefit from being const'd.
>
> If anyone wants to play with it feel free and report back to the list.
>

Actually, I think there is a simpler solution.  Here's what I tried:

===================================================================
--- src/main/include/log4cxx/helpers/objectptr.h        (revision 593214)
+++ src/main/include/log4cxx/helpers/objectptr.h        (working copy)
@@ -137,8 +137,7 @@
             bool operator!=(const ObjectPtrT& p1) const { return (this->p
!= p1.p); }
             bool operator==(const T* p1) const { return (this->p == p1); }
             bool operator!=(const T* p1) const { return (this->p != p1); }
-            T* operator->() {return (T*) p; }
-            const T* operator->() const {return (const T*) p; }
+            T* operator->() const {return (T*) p; }
             T& operator*() const {return (T&) *p; }
             operator T*() const {return (T*) p; }


Note that the single version of operator-> that I added can be used anywhere
that either of the previous versions could be used, as well as in the test
case I originally described.

I tried this with msvc8.  The trunk (593214) builds fine, as do the unit
tests.  The unit tests run as well as they did before.  There is a crash
midway through, but it crashes in the same place without any local changes.
Naturally, all of my code is also fine with this change.

Mime
View raw message