Prevent function taking const std::string& from accepting 0

Question

Worth a thousand words:

#include<string>
#include<iostream>

class SayWhat {
    public:
    SayWhat& operator[](const std::string& s) {
        std::cout << s << "\n";
        return *this;
    }
};

int main() {
    SayWhat ohNo;
    // ohNo[1]; // Does not compile. Logic prevails.
    ohNo[0]; // you didn't! this compiles.
    return 0;
}

The compiler is not complaining when passing the number 0 to the bracket operator accepting a string. Instead, this compiles and fails before entry to the method with:

terminate called after throwing an instance of 'std::logic_error'
  what():  basic_string::_S_construct null not valid

For reference:

> g++ -std=c++17 -O3 -Wall -Werror -pedantic test.cpp -o test && ./test
> g++ --version
gcc version 7.3.1 20180303 (Red Hat 7.3.1-5) (GCC)

My guess

The compiler is implicitly using the std::string(0) constructor to enter the method, which yields the same problem (google the above error) for no good reason.

Question

Is there anyway to fix this on the class side, so the API user does not feel this and the error is detected at compile time?

That is, adding an overload

void operator[](size_t t) {
    throw std::runtime_error("don't");
}

is not a good solution.

Vines

Compiled code, throwing exception in Visual studio at ohNo[0] with exception "0xC0000005: Access violation reading location 0x00000000" – Planimeter

@pmp Yup, that's the point. – Vines

Declare a private overload of operator[]() that accepts an int argument, and don't define it. – Jaban

@Jaban Another good solution I did not think of! Please post it as answer. – Vines

@Jaban Though do note it's a linker error, which is still better than what I had. – Vines

@Vines In above scenario, it will be a compiler error, because the operator is private! Linker error only if called within class... – Weitman

@Jaban That's especially interesting in scenarios where no C++11 is available – and these do exist even today (actually I'm in a project having to deal with, and I'm missing pretty much some of the new features...). – Weitman

@Weitman You're right, I completely missed that part of the comment, thanks. Sill worth mentioning in case somebody does use it in a class (or when dealing with a function). Peter you should definitely post it for some upvotes, especially with for pre-C++11 readers. – Vines

This has a rough duplicate (looking for broad rejection of NULL -> std::string conversion) over on the Software Engineering Stack Exchange: Compile-time checking for NULL initialized std::string – Esmeralda

Answer 1

The reason std::string(0) is valid, is due to 0 being a null pointer constant. So 0 matches the string constructor taking a pointer. Then the code runs afoul of the precondition that one may not pass a null pointer to std::string.

Only literal 0 would be interpreted as a null pointer constant, if it was a run time value in an int you wouldn't have this problem (because then overload resolution would be looking for an int conversion instead). Nor is literal 1 a problem, because 1 is not a null pointer constant.

Since it's a compile time problem (literal invalid values) you can catch it at compile time. Add an overload of this form:

void operator[](std::nullptr_t) = delete;

std::nullptr_t is the type of nullptr. And it will match any null pointer constant, be it 0, 0ULL, or nullptr. And since the function is deleted, it will cause a compile time error during overload resolution.

Antigone

This by far is the best solution, completely forgot I can overload a NULL pointer. – Vines

in Visual Studio, even "ohNo[0]" throwing a null value exception. Does it mean a implementation specific of std::string class? – Planimeter

@pmp What is thrown (if anything) is implementation specific, but the point is the string is a NULL pointer in all of them. With this solution you will not get to the exception part, it will be detected at compile time. – Vines

@pmp - Passing a null pointer to std::string's constructor is not allowed by the C++ standard. It's undefined behavior, so MSVC can do whatever it likes (like throw an exception). – Antigone

Answer 2

One option is to declare a private overload of operator[]() that accepts an integral argument, and don't define it.

This option will work with all C++ standards (1998 on), unlike options like void operator[](std::nullptr_t) = delete which are valid from C++11.

Making the operator[]() a private member will cause a diagnosable error on your example ohNo[0], unless that expression is used by a member function or friend of the class.

If that expression is used from a member function or friend of the class, the code will compile but - since the function is not defined - generally the build will fail (e.g. a linker error due to an undefined function).

Jaban

Answer 3

Using string_view helps (somewhat)

As of C++17, we have the std::string_view class. It is intended exactly for this use-case, of passing non-owning references-to-string-like-objects, to functions which only read a string. You should seriously consider using it for this kind of operators.

Now, std:: string_view has its own set issues (See: enough string_view to hang ourselves with), but here it will give you a useful warning. If you replace:

    SayWhat& operator[](const std::string& s) {

with

    SayWhat& operator[](std::string_view s) {

and you compile with --std=c++17 -Wall, you get:

<source>: In function 'int main()':
<source>:16:11: warning: null argument where non-null required (argument 2) [-Wnonnull]
   16 |     ohNo[0]; // you didn't! this compiles.
      |           ^

Orthoptic

I think this would be the preferable solution in read only case, such as my mock example - maybe even any const string& function argument. I think most of the "dangling" issues would be moot in these cases (without some effort to break things on purpose). – Vines