Anyway to prevent this code from compiling?

Since switching over to clang C++11 on OS X, we had this weird C++ oddity surface while writing some new code. The problem is that ‘mutex’ is no longer a variable, it is a class type that can be interpreted as a function argument. That is, the following line of code can be interpreted as a function declaration now:

OELock lock(mutex);

Instead of a scoped lock acquisition as has been convention for some time now. The full code to recreate the subtle bug is as follows:

#include

using namespace std;

struct OEMutex
{
void Acquire() {}
void Release() {}
};

static OEMutex _mutex;

class OELock
{
OEMutex &_mutex;
OELock();
OELock(const OELock&);
OELock& operator=(const OELock&);

public:
OELock(OEMutex &mutex) : _mutex(mutex) { _mutex.Acquire(); }
~OELock() { _mutex.Release(); }
};

int main()
{
OELock lock(mutex);
}

Ideally, we would like the compilation to fail and tell the user the ‘mutex’ variable can not be found. Any clever C++ trick to do that? We’ve tried declaring the move constructors of OELock to be private, but it still compiles (maybe that’s SFINAE?).

Thanks,
Brian

Try using initialization list syntax. That way the parser won’t think you are declaring a function.

OELock lock{mutex};

Was hoping for something that would be C++03 compatible as well since we still have C++03 compilers to target as well.

you could try adding an extra set of parentheses, but you won’t get as good an error message.

OELock lock((mutex));

Or use initialization list with assignment:

OELock lock = {mutex};

Anything that can be added to the OELock class to make it uncompilable in that context? Getting users to change how they initialize a scoped lock won’t be easy.

A “just as easy” solution is to remove the ‘using namespace std’. I guess this is why Google banned ‘using namespace foo;’ in their style guide: https://google.github.io/styleguide/cppguide.html#Namespaces

Yes, get ride of using namespace std.

The problem is the parsing. Since mutex is a type, OELock lock(mutex) is function declaration, i.e., returns an OELock and takes a mutex as a parameter.

Could you use factory function instead?

lass OELock
{
OEMutex &_mutex;
OELock();
//OELock(const OELock&); // is this a problem for you???
OELock& operator=(const OELock&);
OELock(OEMutex &_mutex) : _mutex(_mutex) { _mutex.Acquire(); }
public:
static OELock makeLock(OEMutex &mutex) { return OELock(mutex);}

~OELock() { _mutex.Release(); }
};

OELock lock(mutex);

int main()
{
OELock lock = OELock::makeLock(mutex);
}

If you're #including <mutex>, haven't you locked yourself into C++11 (or better) anyway? In that case, you should use curly braces for your initializer (or at least stop saying `using namespace std;`).

For existing code where `mutex` is a global variable, you'll be fine. If you add

   #include <mutex>
   using namespace std;

to existing code, you'll get a compiler error because the name `mutex` is now ambiguous.

If you wanted to warn about this in *all* cases, you'd need an additional warning to detect function declarations in local scope, since that's what C++ would treat this as. Clang doesn't have such a warning, but perhaps you could convince someone to add it.

- Jim

That’s a good point. Since mutex is a new c++11 feature, why is it injected into the std namespace when not compiling with c++11. I looked at the mutex header, and it doesn’t test current value of __cplusplus.

Should it?

Just looked online and stdlibc++ has this at the top the mutex header:

https://gcc.gnu.org/onlinedocs/gcc-4.9.3/libstdc++/api/a01070_source.html

#if __cplusplus < 201103L

include <bits/c++0x_warning.h>

#else

Shouldn’t we perform the same sort of check?