Sort out locales for OSL #795
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I got quite the education this week, realizing just how much of C++'s I/O functionality implicitly takes the system's "locale" into consideration. Of particular concern is the conversion of text to floating point numeric values and vice versa, for which much C++ functionality will happily swap the role of '.' and, say, ',' (comma) for the decimal mark, depending on the user's country and whether they are savvy enough to set their locale to the classic "C" (essentially US-style) version. On the other hand, who can blame somebody for not blindly accepting US hegemony these days?
But dammit, OSL is a programming language, and so '.' has a specific role in numeric constants and ',' does something quite different (comma operator!), and if you start mixing up the two when parsing programs, it will all end in tears. All other major programming languages just adopt this convention as well.
Now, if you were writing a full app, you could set the locale to the classic one globally (to that process) upon startup, and then never worry about it. Which we can do for oslc itself. But much of OSL is actually a library that can be embedded into other libraries or apps, so globally setting the locale can mess up UI internationalization or the intentional localization of other app I/O. So we have to handle this deftly throughout.
The fix comes in three parts:
I've submitted a separate review to OIIO here: More principled handling of float<->string OpenImageIO#1785 which tries to straighten up that library, and in particular the various Strutil and ustring utilities that we liberally use throughout OSL. And patrol some of the places where we directly call the locale-sensitive C++ functions like atof and strtod, and replace them with the now-safe OIIO::Strutil equivalents. We'll be able to count on all this working for OIIO >= 1.8.6.
In a variety of places that don't rely on OIIO (for example, various outputs via std::ostringstream), we need to take care to explicitly set the stream's locale to produce the output we expect.
For people building OSL against a too-old version of OIIO to contain the fixes described above, when compiling a buffer of OSL source code, do the gross but necessary safeguard of saving the prior locale, globally changing to "C" locale while we compile the OSL, then restoring the locale. And HOPE that the surrounding application isn't simultaneously counting on the original locale-based internationalization still being in effect for something it is doing in another thread that's unaware that we've temporarily altered a global setting. That's inherently unsafe, but remember it only affects OSL builds against pre-1.8 OIIO (once that patch is applied on the OIIO side), and only will be symptomatic for systems using a non-. locale (which is, empirically speaking, rare enough that nobody noticed this issue until this week).