Thursday, January 7, 2016

You can(not) have a void namespace in C++...

Update: So this was discussed on IRC, and it turns out I've got this all entirely wrong. It's actually much simpler - it turns out you don't need a space before the scope operator thus you can legally have:

void::TheClass::TheFunction(int n1, int n2) {}

This is the same as:

void ::TheClass::TheFunction(int n1, int n2) {}

and:

void     ::TheClass      ::TheFunction(int n1, int n2) {}

--------

I ran doxygen over the vcl module in LibreOffice and I got a lot of warnings and errors. I am now making a concerted effort to fix these warnings because I actually find doxygen generates good info on the classes, and I rather like the collaboration and inheritance diagrams it produces.

However, a strange error that it produced was:

/home/chris/repos/libreoffice/vcl/source/control/combobox.cxx:1322 warning: no uniquely matching class member found for void::ComboBox::Impl::ImplUserDrawHandler(UserDrawEvent *pEvent)

"A void namespace in the LibreOffice codebase?" I thought. How could this be? How is this even possible?

Sure enough, the definition was void::ComboBox::Impl:ImplUserDrawHandler(UserDrawEvent*)!

In an effort to work out if this was what was intended (which I was very doubtful of) I tracked down the commit, which converted the old style IMPL_LINK macro to a boost signal. It was definitely not intended!

I then tracked down where the C++ standard is held and reviewed the C++14 draft (I can't afford to spend US$216 on the official ISO standard). Section 7.3 deals with namespaces:



Amazingly (to my mind) there is no restriction on the namespace identifier, thus it is entirely possible to accidentally create a void, int, float or any other reserved C++ keyword as a namespace!

I wonder if it might be worthwhile for compilers to spit out a warning if they see this? Perhaps we need to write a clang plugin to detect this sort of thing in LibreOffice. The only reason this hasn't had an impact is that it doesn't appear that anything is currently using user-defined draw signals on comboboxes.

I pushed commit 86338fbb94324ed to fix this.

1 comment:

  1. I think you mistake a bug in Doxygen for a bug in the C++ standard... If any compiler actually parsed the line this way, it would issue an error. Actually, 2 errors: one for definition of an undeclared symbol (because no namespace void::ComboBox::Impl has been declared previously) and second for a function definition without a return type.

    The line is simply parsed as void ::ComboBox::Impl::ImplDrawUserHandler( UserDrawEvent *pEvent); That's because "identifier" cannot be a keyword, so the leading "void" must be parsed separately. If you have any doubts, just try to compile a source containing "namespace void {}"...

    ReplyDelete