Node creation bug where implicit conversion happens between type std::string and double

[badumbatish]

Describe the bug
The bug happens where user writes

CXXGraph::Node<std::string> node = CXXGraph::Node<std::string>("id", 0);

The library accepts this as valid and it will pass compile time. But then when it times to run the actual code, this happens:

[2]    43987 segmentation fault  ./build/standalone/anon

I suggest adding the explicit keyword to avoid conversion from double to std::string

This happens when user's IDE might not warn them that there is an implicit conversion and since they thought that templated type std::string is for the first parameter.

[badumbatish]

@ZigRazor i hope the PRs can pass through quickly. I would love to work on these new issues as well the travelling salesman problem as well seeing that i'm using it for my company

[ZigRazor]

I give the max priority to this bug, if you want to correct it you are welcome!

[sbaldu]

Hi. explicit wouldn't work in this case, because it assures that the class type can't be implicitly converted, not that the input parameters can't be implicitly converted.

[sbaldu]

Regarding the issue, the problem is that when passing "0" it interprets it as a nullptr, so it creates an empty string, which of course seg faults.
This was expected behaviour but they fixed it in C++23 by deleting that std::string constructor https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p2166r1.html (in fact if you try to compile with 23 it doesn't compile).
For now we could maybe add a check for the input not being NULL, and when well upgrade the standard we can remove it.

[sbaldu]

error: use of deleted function ‘std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::basic_string(std::nullptr_t) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>; std::nullptr_t = std::nullptr_t]’
    6 |         CXXGraph::Node<std::string> node = CXXGraph::Node<std::string>("id", 0);                                                                         

This is what you get compiling with C++23.

[ZigRazor]

ok, we can add this check?

[sbaldu]

Yep, should be a quick one.

[yapa-ymtl]

Hi, Can I work on this issue if not resolved yet

[sbaldu]

Yes thank you :)

[yapa-ymtl]

Hi @sbaldu What should I do after checking the null?

[JustCallMeRay]

... add a check for the input not being NULL

A check for NULL doesn't work on the templated method since NULL is 0 and we then break int templates. Another option is to delete a nullptr_t specialisation, I don't like this since it breaks on pointer type specialisation. We could override the the template specialisation for std::strings then do a check for nullptr.

I have marked it as explicit (only) in PR #484 .