6

Following code compiles fine without any warnings (with default options to g++). Is there a flag that we can use to ask g++ to emit warnings in such cases?

void foo(bool v) {
}

void bar() {
  foo("test");
}
TemplateRex
  • 69,038
  • 19
  • 164
  • 304
user3612009
  • 655
  • 1
  • 6
  • 18
  • It's not only for character literals and pointer to characters that are affected by this, but *all* pointers are implicitly convertible to `bool`. – Some programmer dude Sep 22 '15 at 10:51
  • 3
    if *a raw string literal as an argument* is the only problem you could add a `void foo(const char*) = delete;` overload – Piotr Skotnicki Sep 22 '15 at 10:52
  • @PiotrSkotnicki That's interesting. If you delete the `foo(const void *)` overload, would it be a better match than the bool overload and hence prevent all pointer conversions? – Peter - Reinstate Monica Sep 22 '15 at 10:55
  • Not *all* pointer conversions, only for the specific type of pointer. If you want to make it for *all* pointer types, then use templates. – Some programmer dude Sep 22 '15 at 10:56
  • 2
    @PaoloM "warning: command line option '-Wimplicit' is valid for C/ObjC but not for C++" – molbdnilo Sep 22 '15 at 10:57
  • 1
    @JoachimPileborg Actually, g++ (4.9.33, cygwin) seems to consider the deleted `foo(const void *)` before the overload taking a bool. I'd have to look it up to be sure but I guess that's the order of overload resolution, and the deleted function takes part in it. – Peter - Reinstate Monica Sep 22 '15 at 11:02
  • @JoachimPileborg At least with c++11 enabled, g++ can delete template functions, nice one. There are some subtle differences to deleting `foo(const void *)`: 1. `foo(0)` is not ambiguous; 2. `foo(nullptr)` is permitted. – Peter - Reinstate Monica Sep 22 '15 at 11:11
  • 1
    In turn `template void foo(T) = delete;` would catch anything that’s not `bool cv {,&,&&}`. – Luc Danton Sep 22 '15 at 11:20
  • 1
    given that `-Wall` doesn't warn about it, I assume the answer must be "no". – davmac Sep 22 '15 at 11:21
  • 4
    @davmac Despite the cute name, `-Wall` doesn't turn on everything. E.g. there’s `-Wextra`, and then there’s more. – Luc Danton Sep 22 '15 at 11:22
  • @LucDanton even `-pedentic` doesn't catch it – TemplateRex Sep 22 '15 at 11:24
  • @LucDanton I see you are correct. It seems odd that that there is no option to actually turn on all warnings. – davmac Sep 22 '15 at 11:28
  • 1
    @TemplateRex, of course not. `-Wpedantic` / `-pedantic` is for things that are technically not allowed by the standard, and this is perfectly legal according to the standard. A warning for this code would be for purely stylistic reasons, not pedantic standard conformance reasons, so would not belong in `-Wpedantic` – Jonathan Wakely Sep 22 '15 at 11:29
  • @davmac, http://stackoverflow.com/q/11714827/981959 – Jonathan Wakely Sep 22 '15 at 11:30
  • 1
    @JonathanWakely thanks. but I still yearn for -Weverything on gcc – TemplateRex Sep 22 '15 at 11:34
  • The discussion makes me want a "-Wunintuitive" option -- stuff that's perfectly legal but often unintended, like `sqrt('A')`. Some pitfalls from the "legal but probably wrong" department are warned about routinlely, like assignments in conditions. – Peter - Reinstate Monica Sep 22 '15 at 11:35
  • 1
    @davmac Literally turning on all warnings would make [this sort of code](http://coliru.stacked-crooked.com/a/1afffbfed83fec2a) warn. Some warning options are also not binary on/off deals and take a threshold, e.g. `-Wlarger-than=42` for which there is no reasonable default. All in all, there are many specialty warnings only useful in special circumstances and not for everybody. You could suggest that there are special groupings of useful warnings… which is precisely what `-Wall` and `-Wextra` are about. (It’s fine if you want/need more though.) – Luc Danton Sep 22 '15 at 11:47
  • @PeterSchneider, but testing a pointer in a boolean context is used widely e.g. `if (Foo* foo = getFoo()) ...` so it's not always unintended. I've considered proposing a change to the language to allow pointers to be _contextually converted to bool_ so the `if` case works, but the OP's example wouldn't work without a cast. – Jonathan Wakely Sep 22 '15 at 12:18
  • @LucDanton, great -Waggregate-return example, thanks. I've added it to http://stackoverflow.com/q/11714827/981959 – Jonathan Wakely Sep 22 '15 at 12:19
  • @LucDanton Sure, but you could turn on all errors (excluding any requiring a value with no obvious default value) and then turn off any that you didn't want. – davmac Sep 22 '15 at 13:18
  • @LucDanton Code returning an aggregate used to be an anti-pattern before RVO was common, which probably was when the warning was introduced. – Peter - Reinstate Monica Sep 22 '15 at 13:30
  • (anyway, it seems reasonable to expect that `-Wall` or at least `-Wall -Wextra` turns on all warnings that have to do with possibly unintended use of language constructs, in which case it should cover the issue in OP's question). – davmac Sep 22 '15 at 13:30
  • @JonathanWakely With a declaration there is little reason to assume that the single `=` is unintentional. Hell, it's not even an assignment. I still think it's useful to warn about non-bracketed assignments in conditionals. Testing a pointer is, of course, extremely idiomatic, and would as such not trigger my imagined "-Wunintentional" heuristics. Oh, was it "-Wunintuitive"? – Peter - Reinstate Monica Sep 22 '15 at 14:04
  • @PeterSchneider, I'm not talking about warning for assignment vs equality (as you say that already produces a warning) I'm talking about treating a pointer as a boolean, and you agree that doing so is idiomatic and reasonable .. so I'm not sure what your request for `-Wunintuitive` boils down to. Doesn't `-Wall` already cover precisely that? _"This enables all the warnings about constructions that some users consider questionable"_ – Jonathan Wakely Sep 22 '15 at 14:16
  • 1
    @JonathanWakely The examples given. clang seems to do perform better heuristics here: `void foo(bool); foo("hello");` is not wrong, is actually nominally idiomatic, but is (to me) surprising and probably not intended (think more complicated overload scenarios). The same goes for `sqrt('A')`. Sure, it's the second-oldest conversion in the history of C, and it's clearly widening so that no information is lost, but again it can be surprising in a more complex context, and nobody in their right mind would on purpose write that. Perhaps there are more? Apple's double goto in the SSL code? – Peter - Reinstate Monica Sep 22 '15 at 14:27

2 Answers2

3

I like to try clang -Weverything and pick the warnings that pop out:

void foo(bool) {}

void bar() {
  foo("test");
}

void baz() {
    foo(nullptr);    
}

int main() {}

main.cpp:5:7: warning: implicit conversion turns string literal into bool: 'const char [5]' to 'bool' [-Wstring-conversion] foo("test");

main.cpp:8:9: warning: implicit conversion of nullptr constant to 'bool' [-Wnull-conversion] foo(nullptr);

Unfortunately, neither -Wstring-conversion nor -Wnull-conversion is supported by g++. You could try and submit feature request / bug report to gcc.

TemplateRex
  • 69,038
  • 19
  • 164
  • 304
2

If I really wanted to prevent such a function being passed a pointer, I would do this in C++11;

 void foo(bool);
 template<class T> void foo(T *) = delete;

 void bar()
 {
     foo("Hello");
 }

which will trigger a compiler error.

Before C++11 (not everyone can update for various reasons) a technique is;

 void foo(bool);
 template<class T> void foo(T *);   // note no definition

 void bar()
 {
     foo("Hello");
 }

and then have the definition of foo(bool) somewhere (in one and only one compilation unit within your build). For most toolchains that use a traditional compiler and linker (which is most toolchains in practice, including most installations of g++) the linker error is caused by foo<char const>(char const*) not being defined. The precise wording of the error is linker dependent.

Note that the errors can be deliberately circumvented by a developer. But such techniques will stop accidental usage.

If you want to allow any pointer except a char const * being passed, simply declare void foo(const char *) as above and don't declare the template.

Peter
  • 35,646
  • 4
  • 32
  • 74