C++ Logo

std-proposals

Advanced search

Re: [std-proposals] Require diagnostic for array to bool conversion

From: Edward Catmur <ecatmur_at_[hidden]>
Date: Sun, 23 Jul 2023 07:42:36 +0100
On Sat, 22 Jul 2023 at 16:31, Arthur O'Dwyer via Std-Proposals <
std-proposals_at_[hidden]> wrote:

> On Sat, Jul 22, 2023 at 4:40 AM sasho648 via Std-Proposals <
> std-proposals_at_[hidden]> wrote:
>
>>
>> int arr[2];
>> bool test = arr;
>> bool teststr = "string";
>>
>>
>> Which I came across while having a function like this:
>>
>> void set_property(const char *name, const char *default_value);
>> void set_property_bool(const char*name, bool default_value);
>>
>> Where I thought the bool one was expecting a string as second argument as
>> well and so I wrote:
>> set_property_bool("test", "true");
>>
>> Which compiled fine and even worked until I decided to change it to:
>> set_property_bool("test", "false");
>>
>
> TBF, that's not a terribly convincing failure mode: obviously `"false"` is
> not the same thing as `false` (in C++).
> However, I have actually run into this implicit conversion before:
>
> https://quuxplusone.github.io/blog/2020/10/11/overloading-considered-harmful/#here-s-another-example-of-grief
>
> void f(int x, bool b);
> void f(int x, std::string s);
>
> ... f(42, "some string"); ...
> calls f(int, bool), not f(int, string).
>
> However, the original sin in that case was "too much overloading," and the
> fix was to give the two different `f`s two different names, just as OP did
> with `set_property` and `set_property_bool`.
>
> I think it would be quite reasonable to make pointers (and thus arrays)
> only *contextually* convertible to bool.
> Contextual conversion is *generally* not done by accident. People have
> already pointed out the `assert(x && "hi")` idiom. Another place I have
> used it is in macro-expansion, like
> #define CHECK(input, out1, out2) do { auto [x,y] = f(input);
> ASSERT_STREQ(x, out1 ? out1 : input); ASSERT_STREQ(y, out2 ? out2 : ""); }
> while (0)
> #define NONE (const char*)nullptr
> CHECK("input1", "xyz", NONE);
> CHECK("input2", NONE, "xyz");
> [...]
>
> However, I personally wouldn't try to change the language here. I'd just
> contact your friendly neighborhood compiler vendor and ask them to add a
> diagnostic for "non-contextual implicit conversion of pointer to bool"...
> if they haven't already done so.
>
> Clang has `-Wstring-conversion` for the `f` example above, where the
> pointer comes specifically from a *string literal*.
> Clang lacks any warning for *general* non-contextual pointer-to-bool
> conversion.
>
> In general, everyone's first instinct (when you find a bug in your code
> that a machine could have detected) should be to *add a compiler warning*
> that can be enabled or disabled for your specific codebase, not to *change
> the semantics of the language* for everyone else in the world.
>

Adding compiler warnings is not exactly easy - it's months of work at a
minimum. Changing the standard might honestly be easier. The old behavior
would be available as a compatibility option for some time if not
indefinitely.

When every compiler warns, we might as well standardize it? Per
https://godbolt.org/z/dqc7TP3aG :

MSVC warns for implicit conversion with C4800, which is under /Wall but no
lower level.
Clang warns for implicit conversion from array with
-Wpointer-bool-conversion, though that also catches contextual conversion
and explicit conversion, and for implicit conversion from string literal
with -Wstring-conversion, although that also catches explicit conversion.
-Wpointer-bool-conversion is under -Wall, but -Wstring-conversion is only
under -Weverything.
gcc warns for implicit conversion from array with -Waddress, which is under
-Wall; it also catches contextual conversion and contextual conversion from
a decayed pointer. It does not AFAIA have a warning for string literals.

So I don't think we're at a place where we can require a diagnostic, let
alone make it ill-formed. We could deprecate implicit conversion, but
compilers would be free to ignore that or put it behind a warning flag not
enabled by default - see e.g. -Wdeprecated-copy.

Received on 2023-07-23 06:42:46