"Mutable Constants" binary size check incorrectly applied to intentionally-mutable Rust symbols

[Łukasz Anforowicz]

Hello @binary-size folks,

Rust style guidelines ask to name `static` variables using `SHOUTY_CASE`. (Note that something like `static ONCE: Once = Once::new()` is seen as sharable by the borrow checker, but this doesn't necessarily mean it is immutable or constant).  This clashes with the "Mutable Constants" binary size check which complains if `SHOUT_CASE` symbols are not in a read-only section of a binary.  This clash is tracked in https://crbug.com/485785258.

Questions:

• MAIN QUESTION: Would it be okay if I just delete the SHOUTY_CASE part of the regex here?
• Could you add reproduction steps that someone like me could use to reproduce the bot's findings locally?  Maybe I missed them, but I didn't see the repro steps in `//docs/speed/binary_size/android_binary_size_trybot.md`
• If we need to complaining about SHOUTY_CASE symbols from C++, then can you please help me figure out how to skip Rust symbols?  I see that `tools/binary_size/libsupersize/models.py` defines filtering helpers like `WhereNameMatches`, but I am not sure how to check the raw symbol - maybe I can use `WhereFullNameMatches`?  or maybe I should introduce `WhereIgnoringRustSymbols`?  (I would experiment with this myself or ask Gemini CLI for help but this is a bit challenging without a local repro)

Thanks,

Lukasz

[Andrew Grieve]

Here's how to repro locally: 

https://source.chromium.org/chromium/chromium/src/+/main:tools/binary_size/trybot_commit_size_checker.py;l=8

It's probably easier to just do some slow attempts using the bot though.

There's already a carve out for "abi:logically_const": https://source.chromium.org/chromium/chromium/src/+/main:tools/binary_size/trybot_commit_size_checker.py;drc=159c5930410c092d794dc73669e242621e7fe2d5;l=133

If there's some thing in the full_name that we can use to exclude them, that'd be fine, but we don't store the mangled name, so there might not be anything in the unmangled name to identify them.

If we are not in the habit of using precompiled rust files, then we could just exclude those from .rs sources:

  symbols = (
      symbols.WhereInSection('d').WhereNameMatches(r'\bk[A-Z]|\b[A-Z_]+$').
      WhereFullNameMatches('abi:logically_const').Inverted()

      WhereSourcePathMatches(r'\.rs$').Inverted())

)

Maybe let's start with that, and then if we still hit issues, we can remove the ALL_CAPS part?

--
You received this message because you are subscribed to the Google Groups "binary-size" group.
To unsubscribe from this group and stop receiving emails from it, send an email to binary-size...@chromium.org.
To view this discussion visit https://groups.google.com/a/chromium.org/d/msgid/binary-size/CAA_NCUFTFBmxoHZmsX%3DhSV3QjvJnAPsDWTBXBRtWs%3D6TY5xqvA%40mail.gmail.com.

[Łukasz Anforowicz]

/cc @Daniel Cheng 

Thanks!  Excluding `.rs` sources seems like a good way to proceed here - let me try adding that to https://crrev.com/c/7695683.