1

It is an implementation of class String in The C++ Programming Language. This is my code and I just show a part of them to eliminate the unrelated stuff.

#include <iostream>
class String {

public:
    explicit String(const char *x);
    explicit String(const String &x);

    friend String operator+(const String &, const char *) {
        char *str;

        String ret(str);

        return ret;
    }
};

The problem was found in function String operator+(const String &x, const char *y);

Error message said:

error: no matching constructor for initialization of 'String'
    return ret;
           ^~~
note: explicit constructor is not a candidate
    explicit String(const char *x) : rep(new Srep(strlen(x), x)) {}
             ^

But the constructor explicit String(const char *x); had been implemented. I got confused. And I tried to eliminate the explicit in explicit String(const char *x);, but it gave me another error:

error: no matching constructor for initialization of 'String'
    return ret;
           ^~~
note: candidate constructor not viable: no known conversion from 'String' to 'const char *' for 1st argument
     String(const char *x) : rep(new Srep(strlen(x), x)) {}// x = "abc"
     ^

When I reduce some code, I found that if there is no function explicit String(const String &x); or this function is not explicit, error won't occur. Are there any relation between explicit String(const String &x) : rep(x.rep); and my ret in String operator+(const String &x, const char *y);? I think it just creates a String and return it by value but maybe it is wrong?

count here stands for reference count, if it use copy constructor to return, it would be wrong for count, and if I use String(str) to return directly, it seems to cause memory leak.

MrZ
  • 166
  • 1
  • 1
  • 11
  • Shorten it down please. It's easily analyzed, but if you want help, cut it down a bit, (I'm voting to close it until that happens) – Ted Lyngmo May 15 '21 at 02:35
  • @TedLyngmo I have cut off the code, does it work for you? – MrZ May 15 '21 at 02:45
  • Better. Can you make it into a [mre]? You don't need "fiiles". (unless that a part of the problem) – Ted Lyngmo May 15 '21 at 02:49
  • 1
    @TedLyngmo done! Hope it can clarify my question. – MrZ May 15 '21 at 03:14
  • try reducing your code... if something that should work does not, remove (comment or #if 0) all else. if it starts to work then start adding stuff back until it doesn't. What does it say when you just shove a const char * into the constructor? – Abel May 15 '21 at 03:16
  • A [mre] is a snippet anyone can copy and test. Yours isn't yet – Ted Lyngmo May 15 '21 at 03:17
  • important stuff includes the Srep constructor that is called Srep(strlen(x), x). important stuff does not include the stuff that is not called... – Abel May 15 '21 at 03:19
  • 2
    explicit copy ctor, hmm. It is strange, but does not look like a good idea. – Slava May 15 '21 at 03:27
  • 1
    Note, identifiers with double underscore are reserved in C++ – Slava May 15 '21 at 03:32
  • @Slava yes. It is just an exercise for implementing String just like std::string, so I try to do it with double underscore. It can be fixed quickly. – MrZ May 15 '21 at 03:40
  • I think now all part of code is necessary. If reduce some part of it, it may not be able to reproduce the problem I try to use explicit in copy constructor because copy here will influence `count`. – MrZ May 15 '21 at 03:41
  • 1
    When creating a [MRE] you need to ask yourself what part of the code contributes to the problem and what not. It's ok to not be sure, because what you need to do is to remove code you suspect and see if the error is still the same. In your case you get an error about not being able to call a method and for this the implementation is completely irrelevant; all you need to keep is the call that causes this with only the data types involved are relevant. I went ahead and created a proper MRE for you, please try and learn from it: https://godbolt.org/z/891es8EWz – bolov May 15 '21 at 04:40
  • On a side note, your `char *str;` local variable is a mistake and needs to be removed. The input `char*` parameter needs to be named instead, eg: `friend String operator+(const String &, const char *str)`. Also, you are completely ignoring the `const String&` parameter. You are not actually concatenating the two parameters in the returned `String` – Remy Lebeau May 15 '21 at 15:36
  • @RemyLebeau This is a short example that highly simplified. In the real code, there have full logic of concatenation. : ) – MrZ May 16 '21 at 07:55

1 Answers1

3

You declare your copy constructor explicit, this means ... well... you need to be explicit when doing a copy:

    friend String operator+(const String &, const char *) {
        // ...
        return ret;
        //     ^~~~
        //     this is a copy, it's an error because it's implicit
    }
    friend String operator+(const String &, const char *) {
        // ...
        return String{ret};
       //      ^~~~~~
       //      now we are explicit, as your copy constructor requires
    }
bolov
  • 72,283
  • 15
  • 145
  • 224
  • Thanks so well! Here I want to clarify two things as well, first one is why we need copy here and how to avoid it. I finished this String following the author's opinion that if use copy constructor, it would not build a new String but make pointer to point the same char*. Therefore, copy constructor must be avoid here. But, if I don't `delete[] str`, memory seems to leak, and this is the second question. I found `std::move` help actually, but the book was finished before C++11, which means there should be another way to do it. Thanks again! – MrZ May 15 '21 at 09:13
  • `ret` is of type `String`, the function returns type `String` (by value), you return `ret`, hence a copy. Yes, `std::move` avoid a copy and makes a move in this case. I strongly suggest you use C++11. Here is a list of good C++ books if you need: https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list – bolov May 15 '21 at 10:11
  • Thanks a lot! *The C++ Programming Language* which is my textbook, do have C++11 version, but two version's String is totally different therefore I have to use this version's String because my lecture is based on it. But I will keep learning Modern C++ as well! – MrZ May 15 '21 at 11:32