The-OpenROAD-Project / The-OpenROAD-Project/OpenROAD

Feature request: POLA — UTF-8 throughout, and a locale-proof environment

Open
#11,293 5 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bazel
Dominant language
Verilog
Stars
3.1k
Forks
1k
Avg merge
2d 23h
Merged PRs (30d)
136

Description

Feature request

Principle of least astonishment: UTF-8 in, UTF-8 out, . as the decimal
separator, everywhere, on every host, with nothing to configure and nothing to
remember. That is mostly what OpenROAD does today — but it is nowhere written
down, and in a couple of places it holds by accident rather than by
configuration.

Three asks:

  • Say it. A short docs/contrib/SourceEncoding.md, linked from
    CONTRIBUTING.md: sources are UTF-8, here are the traps per language.
  • Pin it in the environment, not in the scripts. No sys.stdout.reconfigure
    sprinkled through .py files, no per-tool workarounds. The build environment
    should make encoding and LC_NUMERIC idempotent regardless of the host OS.
  • Close the gaps. The runtime locale pin covers exactly one entry point
    today.

Today the policy is implicit in .bazelrc (we set no
-finput-charset/-fexec-charset, so GCC and Clang use their UTF-8
defaults), implicit in <meta charset="utf-8"> in the web GUI, implicit in
encoding="utf-8" in etc/find_messages.py, and implicit in the ~220 source
files that contain non-ASCII characters. Going without saying is exactly the
problem: #11248 and #11283 both read the tea leaves the other way and
concluded we were aiming at ASCII or latin-1.

Scope caveat, up front: this is not an exhaustive study, and it wasn't a
couple of hours of work either — it is a quick AI sweep, steered by flashbacks
to previous encoding-and-locale projects and by knowing in advance that there
are a number of subtleties here to get right. Every section below ends where I
stopped looking, not where the topic ends. atof() in particular is a rabbit
hole with real technical debt at the bottom of it, and I have only counted
call sites, not audited them. The good news is that between AI and Bazel we
control everything: the sweeps and the archaeology are cheap now, and the
environment those tools run in is ours to pin, in one place, for every host.

Draft content below; the details are the point, so please shoot at them.


Part 1: the encoding policy

Source files are UTF-8, no BOM, LF. Same rule for .cpp/.cc/.h/.hh,
.py, .js and .tcl. Non-ASCII is allowed where it carries meaning and
discouraged where it is decoration.

Where it carries meaning, today, in src/:

Extension Files with non-ASCII
.cpp/.cc/.h/.hh 159
.js 38
.tcl 14
.py 10
// src/gui/src/dbDescriptors.cpp
convertUnits(layer->getCapacitance() * 1e-12) + "F/μm²"
convertUnits(via->getResistance()) + "Ω/sq"
// src/gui/src/mainWindow.cpp — μ is part of the parsing contract, not decoration
} else if (new_value.contains("μ")) {
  new_value = new_value.left(new_value.indexOf("μ"));

μ, Ω, ², →, ▶ and λ have no ASCII spelling a physical-design engineer wants to
read in a properties pane or a debug dump, and latin-1 has neither ² nor → nor
▶ either. "Make every character one byte" is therefore not a policy we can
adopt without making the GUI worse, and the doc should say so plainly so the
question stops being reopened.

Where it is decoration — prose em-dashes in comments and log messages —
ASCII is preferred and swapping for - is a welcome drive-by. That is a
style rule, not an encoding fix. Worth being precise about, because #11283's
stated rationale ("the std::string in the logger api doesn't handle unicode
naturally") isn't the mechanism: std::string holds UTF-8 bytes, fmt passes
them through, the logger copies them. If that were the mechanism, every μm
in the GUI would be broken too, and it isn't.


Part 2: why it is the way it is

Before proposing anything, the fence. src/Main.cc did not arrive at
{"en_US.UTF-8", "C.UTF-8", "C"} by taste; every element of it is a scar:

  • 39c2c4c7dd (2021-11-16), fixing #1229: setenv("LC_ALL", "C") +
    LANG=C. A Brazilian locale broke LEF parsing, because strtod read 0.5
    as 0. Matt's note: "Strangely this problem only occurs when the GUI is
    initialized... Qt locales must be involved."
    That is Qt calling
    setlocale(LC_ALL, "") for us.
  • cf2bdb9159 (2021-11-23), one week later: Cen_US.UTF-8, "for
    python"
    . Plain C fixed the decimal separator and immediately broke Python
    text I/O, which in the C locale falls back to ASCII. That is the same
    failure #11248 is reporting, in 2021, from the opposite direction.
  • a943f45107 (2023-04-10), from the #3137 discussion: en_US.UTF-8 isn't
    installed everywhere, so try it, then C.UTF-8, then C, via setlocale.
  • bac67788f7 (2023-05-22): put the setenv back alongside setlocale, so
    child processes and later Qt/Tcl initialization see the same choice.

The invariant those four commits converge on, and which nothing states
anywhere: the process runs in a UTF-8 locale whose LC_NUMERIC is ., and
we pick the first such locale that exists on the host.
Not C, because
Python needs UTF-8. Not the user's, because atof needs .. That is the
policy. It just isn't written down, and it is enforced in exactly one
main().

Anyone proposing LC_ALL=C in a .bazelrc — as I nearly did while drafting
this — is re-breaking 2021-11-23.


Part 3: what is actually missing

a) The build environment doesn't pin encoding, it merely omits the locale.

From bazel aquery on //src/web:messages_txt:

Environment: [PATH=/bin:/usr/bin:/usr/local/bin]
Command Line: ... external/rules_python++python+python_3_13_x86_64-unknown-linux-gnu/bin/python3 etc/find_messages.py ...

Two things to note. The interpreter is the hermetic rules_python 3.13, not the
host's — that part is already right, and --incompatible_strict_action_env
already strips the host env. But the locale is absent rather than pinned:
Python sees LC_CTYPE=C and PEP 540 quietly enables UTF-8 mode. It works by
fallback, which is why it works for @maliberty. Anything that puts a locale
back into the action environment — an --action_env=LANG=... in a
user.bazelrc, a remote-executor image, a CI wrapper, a non-Bazel build path
— reintroduces the bug, which is presumably how @hongted got latin-1.

Reproduced here against the hermetic 3.13 interpreter itself, with nothing but
a latin-1 locale added to the action environment — the read is explicitly
UTF-8, the write is not:

$ env -i PATH=/bin:/usr/bin:/usr/local/bin LC_ALL=en_US.ISO-8859-1 python3 repro.py
UnicodeEncodeError: 'latin-1' codec can't encode character '\u2014'
in position 27: ordinal not in range(256)

$ env -i PATH=/bin:/usr/bin:/usr/local/bin LC_ALL=en_US.ISO-8859-1 PYTHONUTF8=1 python3 repro.py
warn(WEB, 30, "No STA data — timing sections will be empty.");

Same error string as #11248, and sys.stdout.encoding goes iso8859-1
utf-8. Without the locale, sys.flags.utf8_mode is already 1 and it
passes — which is the accident we are currently relying on.

The fix belongs in .bazelrc, once, and it should be the OS-independent lever
rather than a locale name:

# Python text I/O must not depend on the host, on any OS.
build  --action_env=PYTHONUTF8=1  --host_action_env=PYTHONUTF8=1
test   --test_env=PYTHONUTF8=1
common --repo_env=PYTHONUTF8=1

PYTHONUTF8=1 forces UTF-8 for stdio and for open() defaults on Linux,
macOS and Windows alike, without betting on C.UTF-8 existing (it doesn't on
macOS, nor on glibc < 2.35) — the same portability problem a943f45107 hit.
Pinning a constant value keeps actions hermetic and cacheable: it is part of
the action key and identical on every host. And it is the environment doing
the work — no sys.stdout.reconfigure sprinkled through our scripts, no
per-tool workaround. Explicit encoding="utf-8" on open() stays good
practice as belt-and-braces, not as the mechanism.

b) The runtime locale pin covers exactly one entry point.

src/Main.cc is the only main() that runs the fallback dance. These do not:
the ODB LEF/DEF utilities (defrw, defwrite, differDef, lefrw,
lefwrite, differLef), the ODB swig mains,
ceff_training_data_generator, and every gtest binary — each of which writes
its own testing::InitGoogleTest main. A unit test that parses a .def under
nb_NO.UTF-8 fails, or worse passes with zeros.

The fix is to lift the four-commits-worth of knowledge in Main.cc into one
shared ord::initLocale() and call it from every entry point, rather than an
env var — because the whole point of a943f45107 is that the right locale name
varies by host, and an env var can't fall back.

c) Why this is not negotiable at the parser level.

The C and C++ standards put file-format parsing and user-facing formatting
behind the same switch. atof cannot tell "parse this Liberty float, whose
grammar mandates ." from "parse what the user typed into a dialog". Liberty,
LEF/DEF, SPEF, SDF and SDC all specify . and none of them care what the
operator's desktop is set to. OpenSTA's position — the standard is simply
wrong here — is the pragmatic one and it is not going to budge: atof() stays
in the SPEF and SDF lexers (src/sta/parasitics/SpefLex.ll,
src/sta/sdf/SdfLex.ll). std::from_chars is the locale-independent
replacement the standard eventually shipped, but rewriting every reader to use
it is on nobody's roadmap and would still leave the readers we don't own.

So it is a whole-program commitment, and src/ outside sta/ is in the same
boat: ~31 atof, ~13 strtod, ~18 std::stod, ~10 atoi. Demonstrated on
this machine, in Norway:

setlocale(LC_ALL, "");         // what Qt and Tcl do behind your back
atof("0.5");                   // 0    <-- silently truncated
atof("0,5");                   // 0.5
std::stod("0.5");              // 0    <-- same

No error, no warning, just a design with the wrong numbers in it. That is the
worst failure mode we have, and the only defense is pinning the locale before
anything else runs.


The traps, per language, for the doc

None of these are fixed by deleting characters, which is the argument for
documenting them instead of sweeping.

Python — implicit locale encoding. Every text-mode open() gets an explicit
encoding; the environment pins the rest.

open(path)                       # locale-dependent; breaks on non-UTF-8 hosts
open(path, encoding="utf-8")     # correct

subprocess.run(cmd, text=True)   # same trap on the decode side
subprocess.run(cmd, text=True, encoding="utf-8")

Binary in / ASCII out is equally correct and is what etc/file_to_string.py
already does (open(file_name, "rb") → base64), which is why it never hit
this.

C++ — bytes are not characters. The literal is fine; the arithmetic around
it is where it goes wrong.

std::string s = "Ω/sq";      // 5 bytes, 4 characters
s.size()                     // 5 — not a column count
fmt::format("{:>10}", s)     // pads to 10 *bytes* → table misaligns
s.substr(0, 3)               // may split Ω in half → invalid UTF-8 downstream
std::toupper(s[0])           // per-byte; corrupts the sequence
char c = 'μ';                // doesn't compile: multi-character literal

Truncating a name for display is the usual way to manufacture mojibake. If a
string can hold units, don't slice it at a byte offset. And never call
setlocale(LC_ALL, "") — see Part 3c.

JavaScript — length is UTF-16 code units; bytes exist only at the boundary.

'▶'.length        // 1
'🔍'.length       // 2 — surrogate pair
[...'🔍'].length  // 1 — iterate, don't index, when slicing for display

Anything computing a byte count for the wire (a Content-Length, a buffer
size) goes through new TextEncoder().encode(s).length, never s.length. And
the page must declare its encoding or the browser guesses; we do this in both
places today (src/web/src/index.html and the inline document in web.cpp)
and it needs to stay:

<meta charset="utf-8">

Proposed work
  1. .bazelrc: pin PYTHONUTF8=1 for actions, host actions, tests and repo
    rules. Environment-level, OS-independent, no .py edits. This is the fix
    for #11248.
  2. Lift Main.cc's locale fallback into a shared ord::initLocale() and call
    it from the other entry points — ODB utilities, swig mains, gtest mains.
    Keep the {"en_US.UTF-8", "C.UTF-8", "C"} order; #1229, #3137 and
    cf2bdb9159 are the reasons it looks like that.
  3. Add docs/contrib/SourceEncoding.md: sources are UTF-8, the process runs
    in a UTF-8 .-decimal locale, here are the per-language traps and the
    history that produced the rule. Link from CONTRIBUTING.md and
    docs/toc.yml.
  4. On that basis close #11248's broader ask: sources stay UTF-8, and
    decorative em-dash cleanup (#11283) continues as a style change.

If anyone is on a platform where the build environment genuinely cannot be
UTF-8, that is the interesting case and I'd like the specifics — the same
locale will mangle μm on the way out of the GUI, and 0.5 on the way into
the parsers, no matter what we do to the sources.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with .bazelrc, CONTRIBUTING.md, and the proposed docs/contrib/SourceEncoding.md, then read src/Main.cc and the listed ODB, SWIG, training-data, and gtest entry points. Use Bazel action inspection and locale-sensitive tests to map current behavior. Done means the policy is documented, Python actions are pinned, and every entry point shares the locale initialization without per-script workarounds.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, javascript, python
Domain
build-system, documentation, internationalization, testing
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.