Skip to content

Use absl's locale-independent conversion functions in XPRESS interface#135

Merged
sgatto merged 1 commit intomainfrom
fix/remove-scoped-locale
Sep 20, 2024
Merged

Use absl's locale-independent conversion functions in XPRESS interface#135
sgatto merged 1 commit intomainfrom
fix/remove-scoped-locale

Conversation

@flomnes
Copy link
Copy Markdown
Collaborator

@flomnes flomnes commented Sep 20, 2024

Using such functions as std::stoi, std::stod, etc. caused us a lot of trouble because these functions are locale dependent, as illustrated below

#include <iostream>
#include <locale>
#include <string>

void convert(std::string locale)
{
    std::string value = "9.81";
    if (std::setlocale(LC_ALL, locale.c_str()))
    {
        double converted = std::stod(value);
        std::cout << "locale " << locale << " converted to " << converted << std::endl;
    }
    else
    {
        std::cout << "locale set failed for " << locale << std::endl;
    }
}
int main()
{
    convert("C");
    convert("fr_FR.utf8");
    // OUTPUT
    // locale C converted to 9.81
    // locale fr_FR.utf8 converted to 9
}

At first, we thought that using ScopedLocale would allow us to temporarily use the "C" locale, but this approach has multiple problems

  1. It's not thread-safe. For example calling std::stod when the locale is being changed through std::setlocale is undefined behavior source.
  2. C is hard, const char* pointers easily become dangling or invalid without the caller knowing
struct ScopedLocale {
  ScopedLocale() {
    oldLocale = std::setlocale(LC_NUMERIC, nullptr);
    auto newLocale = std::setlocale(LC_NUMERIC, "C");
    CHECK_EQ(std::string(newLocale), "C");
  }
  ~ScopedLocale() { std::setlocale(LC_NUMERIC, oldLocale); }

 private:
  const char* oldLocale;
};

We ran into an issue where the locale wasn't properly restored after using MPSolver::XpressInterface. So we decided to use absl's locale independent functions to perform that task.

@sgatto sgatto changed the base branch from google/stable to main September 20, 2024 14:19
@flomnes flomnes force-pushed the fix/remove-scoped-locale branch from ed94162 to 67b395e Compare September 20, 2024 14:23
@flomnes flomnes changed the title Use absl's locale independent conversion functions in XPRESS interface Use absl's locale-independent conversion functions in XPRESS interface Sep 20, 2024
@sgatto
Copy link
Copy Markdown
Member

sgatto commented Sep 20, 2024

Le code a l'air pas mal plus simple :)

@sgatto sgatto merged commit 54f2f90 into main Sep 20, 2024
@flomnes flomnes deleted the fix/remove-scoped-locale branch September 20, 2024 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants