syoyo/tinygltf

[Defect] tinygltf::Value(char const*) invokes tinygltf::Value(bool) unexpectedly

agnat opened this issue · 2 comments

agnat commented

Hi @syoyo,

I'm currently playing with your loader. So first, thanks for making this!

I came across a little issue with tinygltf::Value, in particular with the massively overloaded constructor...

Describe the issue

Constructing values manually using a char * as argument should result in a tinygltf::Value of type string. Instead it invokes the bool constructor, because of the automatic conversion of any pointer type to bool.

To Reproduce

  • OS: Any
  • Compiler: Any
#include <iostream>
#include <tiny_gltf.h>

int
main(int argc, char * argv[]) {
  tinygltf::Value str_val("foo");
  std::cout << "str_val type: " << (int)str_val.Type() << std::endl;
  assert(str_val.Type() == tinygltf::STRING_TYPE);
  return 0;
}

As a workaround tinygltf::Value str_val(std::string("foo")); works as expected. Interestingly, tinygltf::Value str_val {"foo"}; fails at compile-time, because of the stricter type narrowing rules in C++11.

Expected behaviour

The code above should produce a value of type string (or fail at compile-time). Adding yet another constructor overload for char const* should do the trick... I think.

Let me know what you think...

Edit: typos

syoyo commented

@agnat Thanks! Confirmed the issue.

tinygltf::Value str_val("foo");

It is interesting behavior that Value constructor selects bool overload for const char * regardless of explicit qualifier in each constructor(I was expected it results to compile-time error as did in tinygltf::Value str_val {"foo"}).

Adding yet another constructor overload for char const* should do the trick... I think.

Yes, adding const char * constructor should be the solution.

  explicit Value(const char *s) : type_(STRING_TYPE) {
    string_value_ = s;
  }

PR is much appreciated!

agnat commented

PR is much appreciated!

Sure, give me a second.