#239 Non-explicit constructors cause unintended conversions

[Vidar Hasfjord]

Many constructors for the classes in the OWLNext framework takes one argument, either explicitly, or due to default arguments for the subsequent parameters. Most of these constructors are currently not marked explicit, hence may cause unintended conversions that may make seemingly simple code do surprising things. For example, consider this code:

  TEdit& edit = ...;
  ostringstream s;
  s << edit.GetText();

This code will work fine in ANSI build mode. It also compiles fine in Unicode build mode, but now it creates a TModule and an exception! What? A TModule? Yes, really!

The reason is that there is no operator << for tstring (returned by TEdit::GetText) in Unicode build mode, so the compiler searches and finds that it can use the operator << for TModule after applying the TModule (const tstring&, ...) converting constructor. This of course causes an exception at run-time ("Unable to load module ...").

Proposed resolution

All converting constructors should be marked explicit except for classes which are interchangeable with the argument type, which should be very few cases, if any.

[Vidar Hasfjord]

A partial fix for this issue, which addresses constructors in TWindow, has been submitted in [r2002].

Two of the TWindow constructors have default arguments for the second parameter making them converting constructors, leading to possible unintended conversions (from TWindow* to TWindow, and HWND to TWindow) causing surprising behaviour and silent bugs.

An example, of which I experienced recently:

void Foo(TWindow* w, TMDIClient* workspace)
{
  // Calculate window rectangle in TMDIClient workspace coordinates.
  //
  TRect r = w->MapWindowRect(workspace); // BUG: Silent conversion!
  ...
}

Here the code unintentionally creates a new TWindow, a child of workspace, in the call to MapWindowRect, leading to garbage in the returned rectangle. After applying the fix, making the constructor explicit, the code produces a compilation error. MapWindowRect expects a TWindow reference, not a pointer.

[Vidar Hasfjord]

Another partial fix in [r2003] addresses the converting TModule constructor mentioned in the ticket description.

[Vidar Hasfjord]

Unintentional converting constructors were made explicit (i.e. non-converting) in Owlet [r2391]. Also see [r2395] (TFileTime), [r2870] (validator overhaul) and [r4795] (TRegKey).

Some of these changes have been applied on the trunk as well, e.g. [r3514] (THdrItem), [r4629] (TDib), [r5161] (TMenu).