From acf26257b74ca5446fd5a0390a29723dc25ef2a3 Mon Sep 17 00:00:00 2001 From: Tim Foley Date: Tue, 19 Dec 2017 11:49:36 -0800 Subject: Fix floating-point literal emit to be locale-independent. (#315) There was a bug that arose in the context of Falcor, because: - Slang uses `sprintf` to format floating-point values when outputting HLSL/GLSL source - Falcor (or a library it uses) does the equivalent of `setlocale(LC_ALL, "")` to set the global locale for the process based on the user's environment variables This led to a floating-point literal of `0.5` getting printed as `0,5f`, with a comma for the decimal point. This then gets consumed by `glslang` which (luckily for us) complains that `5f` is not a valid floating-point literal in their language (since it has neither decimal point nor exponent). The most expedient fix in this case was to switch from using C `sprintf` for formatting floating-point numbers over to using the C++ `` implementation, which allows the locale to be set per-stream so that we don't have to rely on (or potentially disrupt) the global locale set by an application using Slang. Longer term if we have the time/resources to do the Right Thing, it would be best to implement our own printing logic for floating-point numbers, since we would eventually need/want to support arbitrary-precision numbers for literals. --- source/slang/emit.cpp | 40 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 36 insertions(+), 4 deletions(-) (limited to 'source') diff --git a/source/slang/emit.cpp b/source/slang/emit.cpp index ce17c8d03..e40350df8 100644 --- a/source/slang/emit.cpp +++ b/source/slang/emit.cpp @@ -13,6 +13,14 @@ #include +// Note: using C++ stdio just to get a locale-independent +// way to format floating-point values. +// +// TODO: Go ahead and implement teh Dragon4 algorithm so +// that we can print floating-point values to arbitrary +// precision as needed. +#include + #ifdef _WIN32 #include #pragma warning(disable:4996) @@ -546,10 +554,34 @@ struct EmitVisitor void Emit(double value) { - // TODO(tfoley): need to print things in a way that can round-trip - char buffer[128]; - sprintf(buffer, "%.20ff", value); - Emit(buffer); + // There are a few different requirements here that we need to deal with: + // + // 1) We need to print somethign that is valid syntax in the target language + // (this means that hex floats are off the table for now) + // + // 2) We need our printing to be independent of the current global locale in C, + // so that we don't depend on the application leaving it as the default, + // and we also don't revert any changes they make. + // (this means that `sprintf` and friends are off the table) + // + // 3) We need to be sure that floating-point literals specified by the user will + // "round-trip" and turn into the same value when parsed back in. This means + // that we need to print a reasonable number of digits of precision. + // + // For right now, the easiest option that can balance these is to use + // the C++ standard library `iostream`s, because they support an explicit locale, + // and can (hopefully) print floating-point numbers accurately. + // + // Eventually, the right move here would be to implement proper floating-point + // number formatting ourselves, but that would require extensive testing to + // make sure we get it right. + + std::ostringstream stream; + stream.imbue(std::locale::classic()); + stream.precision(20); + stream << value; + + Emit(stream.str().c_str()); } -- cgit v1.2.3