> Looks good. I like the compile-time checks, why do you have them disabled?
type_traits is c++11…

> Why the static_cast?
GSLists store their data in void-pointers. dynamic_cast refuses to cast them.

> Why is there a namespace "Tools" and another "Tool"? That does not look too good.
I’ll change that later J .

> Perhaps make a templated cast function that works for all types?

I’d rather leave it in “Tool”. For me it’s important that code is easily readable.

Tool::is_a<Pen> is easier to read than

Inkscape::is_a<Tool, Pen>.

Besides that, you’d have the problem “Tool is_a Pen? Or Pen is_a Tool?”.

These casts are inlined anyway J .

> Let's add a const-version of ::to aswell.

Yeah, I forgot that. This won’t work without it.

> Perhaps I can motivate you to spend your energy now on fixing bugs, so we can get the release out the door? ;-)
Well, currently I’ve got 4 in-progress bugs. I’m just not appearing in Bryce’s list as I have to set them to “fix released” instead of “committed” as they are caused by my refactoring L .

Regards,

Markus

Von: Johan Engelen [mailto:jbc.engelen@...2592...]
Gesendet: Donnerstag, 27. Februar 2014 00:20
An: Markus Engel; 'Inkscape Devel List'
Betreff: Re: Casting-macro replacement

On 26-2-2014 20:41, Markus Engel wrote:

Hi there,

in r13061 I added two template functions in order to replace the tools’ casting macros.

You can see them in action in widgets/pencil-toolbar.cpp:110.

I also added some compile-time checking there which can be uncommented later.

Looks good. I like the compile-time checks, why do you have them disabled?

Why the static_cast?
Why is there a namespace "Tools" and another "Tool"? That does not look too good.

Perhaps make a templated cast function that works for all types?

template<typename Derived, typename T>

bool Inkscape::is_a(const T* t) {

     static_assert(std::is_base_of<T, Derived>(),...
     return dynamic_cast<const Derived*>(t) != NULL;
}

Let's add a const-version of ::to aswell.

As I just stumbled across this… In order to build clang from its svn source, you need a c++11-enabled compiler:

“This is expected to be the last release of LLVM which compiles using a C++98 toolchain. We expect to start using some C++11 features in LLVM and other sub-projects starting after this release. That said, we are committed to supporting a reasonable set of modern C++ toolchains as the host compiler on all of the platforms. This will at least include Visual Studio 2012 on Windows, and Clang 3.1 or GCC 4.7.x on Mac and Linux. The final set of compilers (and the C++11 features they support) is not set in stone, but we wanted users of LLVM to have a heads up that the next release will involve a substantial change in the host toolchain requirements.”

Should I proceed replacing the casting-macros like this? Any suggestions?

Perhaps I can motivate you to spend your energy now on fixing bugs, so we can get the release out the door? ;-)
Then after release, we can bug everybody about getting C++11 build environments set up, flip the switch, and go wild refactoring. :P  (this is absolutely my plan after release)

Cheers,
  Johan