How to detect wrong Constructor usage... Object A(arguments) vs Object(arguments)

[Jan Riewenherm]

Hi,

i´ve got an issue where i do not really find a proper solution.

We have locking objects that take a pointer as argument and protect simultaneous access to the same object from different threads. These locking objects have a singleton in the backed but thats not the point of interest here.

What i would like to prevent is that the locking object is used in the wrong way.

Code Snippped of the locking helper object.
class LockingObject
{
LockingObject(void* pObjectToLock)
{
Singleton_Lock(pObjectToLock);
}
~LockingObject()
{
Singleton_Unlock(m_pObjectToLock);
}
private:
void* m_pObjectToLock
}

How to use the locking object:
function test()
{
LockingObject object(this);
do_something_on_this();
}

As soon as the locking object is created the simultaneous access is protected.
As soon as i run out of the scope of my function the locking objects is freeed.

The problem i have:
function test()
{
LockingObject(this);
do_something_on_this();
}

How to prevent calling the constructor where the created object is not used at all? c++17 [[nodiscard]] does not work for Constructors.

The locking object is a very generic approach to lock access from different threads and leads to a very low ressource consumption as i do not need to have dedicated locking objects for members or functions i want to protect. So please do not take a closer look at how other locking mechanisms might work. I would like to focus on the problem of having constructors which do not actually create an object.

Any advice appreciated.

Jan

[Öö Tiib]

On Wednesday, 21 November 2018 16:06:08 UTC+2, Jan Riewenherm wrote:
> Hi,
>
> i´ve got an issue where i do not really find a proper solution.
>
> We have locking objects that take a pointer as argument and protect simultaneous access to the same object from different threads. These locking objects have a singleton in the backed but thats not the point of interest here.
>
> What i would like to prevent is that the locking object is used in the wrong way.
>
> Code Snippped of the locking helper object.
> class LockingObject
> {
> LockingObject(void* pObjectToLock)
> {
> Singleton_Lock(pObjectToLock);
> }

That constructor leaves private member m_pObjectToLock
uninitialized.

> ~LockingObject()
> {
> Singleton_Unlock(m_pObjectToLock);
> }

That destructor passes uninitialized member.

> private:
> void* m_pObjectToLock
> }
>
> How to use the locking object:
> function test()
> {
> LockingObject object(this);
> do_something_on_this();
> }
>
> As soon as the locking object is created the simultaneous access is protected.
> As soon as i run out of the scope of my function the locking objects is freeed.
>
> The problem i have:
> function test()
> {
> LockingObject(this);

Note that the temporary constructed above must be destroyed by now.

> do_something_on_this();
> }
>
> How to prevent calling the constructor where the created object is not used at all? c++17 [[nodiscard]] does not work for Constructors.

Technically it is possible to replace that particular public
constructor with a public factory method and private
constructor. Inlining and RVO might achieve that there
are no performance differences.

The compilers do not warn there because it might be
code as its designer desired:

std::ofstream("filename"); // creates ./filename if it doesn't exist

Philosophically all useful languages have to allow endless
ways to express lies and nonsenses for to achieve that
at least some truths can be expressed in those as well.
Blocking all lies and nonsense will be achieved only when
nothing useful can be expressed anymore.

>
> The locking object is a very generic approach to lock access from different threads and leads to a very low ressource consumption as i do not need to have dedicated locking objects for members or functions i want to protect. So please do not take a closer look at how other locking mechanisms might work. I would like to focus on the problem of having constructors which do not actually create an object.
>
> Any advice appreciated.

Always try to post code that compiles, runs and demonstrates
the issue that you have. Otherwise it is hard to realize
what sort of defect or lack of expertise is its cause.

[Paavo Helde]

On 21.11.2018 16:05, Jan Riewenherm wrote:
> How to use the locking object:
> function test()
> {
> LockingObject object(this);
> do_something_on_this();
> }
>
> As soon as the locking object is created the simultaneous access is protected.
> As soon as i run out of the scope of my function the locking objects is freeed.
>
> The problem i have:
> function test()
> {
> LockingObject(this);
> do_something_on_this();
> }
>
> How to prevent calling the constructor where the created object is not used at all? c++17 [[nodiscard]] does not work for Constructors.

Define a macro

#define SCOPED_LOCK(x) LockingObject object(x);

and make a rule to use only this macro throughout the code base:

void test() {
SCOPED_LOCK(this);
// ...
}

I use this all the time, works fine for avoiding temporary objects
(although TBH the initial motivation for using a macro was to have
correct __FILE__ and __LINE__ for debugging purposes).

[peter koch]

onsdag den 21. november 2018 kl. 15.06.08 UTC+1 skrev Jan Riewenherm:
> Hi,
>
> i´ve got an issue where i do not really find a proper solution.
>
> We have locking objects that take a pointer as argument and protect simultaneous access to the same object from different threads. These locking objects have a singleton in the backed but thats not the point of interest here.
>
> What i would like to prevent is that the locking object is used in the wrong way.
>
> Code Snippped of the locking helper object.
> class LockingObject
> {

...

> }
>
> How to use the locking object:
> function test()
> {
> LockingObject object(this);
> do_something_on_this();
> }
>
> As soon as the locking object is created the simultaneous access is protected.
> As soon as i run out of the scope of my function the locking objects is freeed.
>
> The problem i have:
> function test()
> {
> LockingObject(this);
> do_something_on_this();
> }
>
> How to prevent calling the constructor where the created object is not used at all? c++17 [[nodiscard]] does not work for Constructors.
>
>

> Any advice appreciated.

You can use nodiscard on the class:

class [[nodiscard]] LockingObject
>
> Jan

[Paavo Helde]

[[nodiscard]] only applies if the object is returned from a function,
which does not happen here. MSVC2017 and g++ 7.3 agree and compile the
following without warnings or errors:

class [[nodiscard]] LockingObject {
public:
LockingObject(int x) {}
};

int main() {
LockingObject(1);
}

[peter koch]

Ah - you are right. I got the warning in my test - using -Wall, but it is not related to the nodiscard attribute.
So that is probably the solution: use -Wall. This option IMO actually should always be included.
That said, it is weird that the attribute does not work in the situation above.