search

mayah / tinytoml

A header only C++11 library for parsing TOML
BSD 2-Clause "Simplified" License
167 stars 31 forks source link

std::round() support #13

Closed jtbr closed 8 years ago

jtbr commented 8 years ago

First off, thanks for your work here. Great library.

I was having trouble with line toml.h:831

tp += std::chrono::microseconds(static_cast<int>(std::round(ss * 1000000)));

It seems std::round (a C++11 feature) has a variety of support issues. It appears visual studio hadn't implemented it as of 2012. And the issue I ran into: building with GCC 4.9.3 for cygwin, std::round is undefined (a bug), but round (no namespace) is, and works (it also builds without std:: using GCC 5.1.0 for mingw). It may thus be preferable to use this formulation, but it would be worth testing if this works under linux and visual studio. Alternatively, considering that computer clock precision is rarely in the 1 microsecond range, the round() call could be skipped altogether.

The other question is whether ss*1000000 could overflow if int is 32 bits (as is usually the case). It looks like not given how it's used, but on my platform anyway, microseconds appears to be using int64_t, so would it make sense to cast to that anyway?

mayah commented 8 years ago

Let's forget Visual Studio 2012 or 2013. Any Windows developer should use VS2015 Update 2 now. It has every C++ Standard Library feature that's been voted into C++11. Hehe...

Here, ss should be 0 ~ 1. So, it should be safe. Anyway, I'm OK with using int64_t here to reduce an implicit cast.

mayah commented 8 years ago

I've pushed the workaround version. Closing. If you still have a trouble, feel free to reopen the bug.