Skip to content

Fix static const variable handling in opir#158

Open
gokr wants to merge 2 commits intoPMunch:masterfrom
gokr:master
Open

Fix static const variable handling in opir#158
gokr wants to merge 2 commits intoPMunch:masterfrom
gokr:master

Conversation

@gokr
Copy link

@gokr gokr commented Nov 10, 2025

I used Claude Code to fix the "static const" problem (Futhark just generating var type declarations without values). Claude created a fair bit of code so it could read the header source and try to generate a Nim initializer based on it, can probably be done simpler - but at least you can use this PR as inspiration! And probably better to just WARN and not generate var declarations.

The rest below is from Claude.

Previously, C static const variables were incorrectly generated as Nim var declarations without values, making them unusable. This commit adds proper detection and conversion of static const variables to Nim const declarations.

Changes:

  • Added getInitializerText() to extract initializer expressions from C code
  • Added convertInitializerToNim() to translate C expressions to Nim format
  • Modified genVarDecl() to detect static const variables (NoLinkage or Internal linkage with const-qualified type)
  • Successfully converted initializers generate "kind": "const" JSON with values
  • Failed conversions log clear warnings and skip generation entirely (return nil)

Supported initializer types:

  • Numeric literals: integers (decimal, hex, octal, binary), floats
  • String and character literals
  • Type suffixes: u, l, ll, f, etc.

Unsupported (warns and skips):

  • Macro function calls: orx2F(3.14f), orx2D(1.0)
  • Type cast expressions: (uint32_t)(-1)
  • Complex expressions requiring evaluation

Example warnings:
Warning: Cannot translate initializer for static const 'orxFLOAT_MAX': orx2F(3.402823466e+38f) - skipping Warning: Cannot translate initializer for static const 'orxU32_UNDEFINED': (orxU32)(-1) - skipping

This ensures static const variables are either properly converted with values or clearly flagged as unsupported, rather than generating broken var declarations.

🤖 Generated with Claude Code

Previously, C static const variables were incorrectly generated as Nim var
declarations without values. This commit adds proper detection and conversion
of static const variables to Nim const declarations, with a robust hint
system that persists across cached builds.

Changes to opir.nim:
- Added getInitializerText() to extract initializer expressions from C code
- Added convertInitializerToNim() to translate C expressions to Nim format
- Modified genVarDecl() to detect static const variables (NoLinkage or
  Internal linkage with const-qualified type)
- Successfully converted initializers generate "kind": "const" JSON with values
- Failed conversions now generate "kind": "warning" JSON entries (displayed as hints)

Changes to futhark.nim:
- Added warning handler in symbol gathering loop to display opir warnings
- Warnings from JSON are displayed as hints using Nim's hint() function
- Hints persist in cached JSON, so they appear on every build
- Can be suppressed with --hint[User]:off if desired

Supported initializer types:
- Numeric literals: integers (decimal, hex, octal, binary), floats
- String and character literals
- Type suffixes: u, l, ll, f, etc.

Unsupported (generates hint):
- Macro function calls: orx2F(3.14f), orx2D(1.0)
- Type cast expressions: (uint32_t)(-1)
- Complex expressions requiring evaluation

Example hints during compilation:
  Hint: Cannot translate initializer for static const 'orxFLOAT_MAX': orx2F(3.402823466e+38f) - skipping [User]
  Hint: Cannot translate initializer for static const 'orxU32_UNDEFINED': (orxU32)(-1) - skipping [User]

Benefits:
- Hints appear in Nim compilation output (not buried in opir stderr)
- Hints persist across cached builds (stored in JSON)
- Consistent with futhark's existing hint system (like renaming hints)
- Less alarming than warnings (informational rather than concerning)
- Can be suppressed with --hint[User]:off if already handled manually
- Static const variables are either properly converted or clearly flagged

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@heysokam
Copy link
Contributor

heysokam commented Nov 10, 2025

Mandatory disclaimer: I'm not a Futhark dev.

Lets please not add AI nonsense into a compiler tool.
Futhark code is hard enough to follow as it is.
This code is so overkill, only to fix something that already worked in the past.

As far as I'm aware, PMunch has been busy and hasn't been able to invest much time into futhark,
hence why the bug is still there.
But I'll let him speak for himself.

@gokr
Copy link
Author

gokr commented Nov 10, 2025

No need to be so hostile! :) And no, it can never have "worked in the past" (at least not for my case) since expression initializers were the culprits for my static const to end up as var type declarations. Simply because Futhark can not translate those. As I wrote, use the PR as inspiration and PMunch can fix it whichever way he likes.

One reasonable approach, as I mentioned, is to just warn and not generate anything at all. Which is exactly what my fork of Futhark now does (since my initializers are untranslatable because they use C macros).

Removed some commented out debug code
@gokr
Copy link
Author

gokr commented Nov 11, 2025

To make it more clear, if I understand "static const" correctly - without it being defined as "extern" then the idea is that each program including this header would get its own copy of it. So it would need to be able to fully evaluate the expression for the value. The logic can probably be improved to cover the "extern" case but for my case where I have weird expressions using custom macros - I simply needed to reimplement those in my Nim high level wrapper. So I just wanted Futhark to not generate anything (to avoid clashes).

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