Skip to content

Add c++ name mangling to avoid keyword collision#50

Merged
tlambert03 merged 5 commits into
mainfrom
windows-name-collision-fix
Jun 9, 2026
Merged

Add c++ name mangling to avoid keyword collision#50
tlambert03 merged 5 commits into
mainfrom
windows-name-collision-fix

Conversation

@cmalinmayor

Copy link
Copy Markdown
Contributor

https://github.com/live-image-tracking-tools/geff/actions/runs/26179372787/job/77017880290

28 compilation failures in our GEFF windows tests, where apparently we call an attribute uint16 and windows c++ breaks on keyword collision - Claude suggested adding this attribute name mangling to the c++ side only, avoiding keyword collision without affecting the public Python API

@codecov-commenter

codecov-commenter commented Jun 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.11%. Comparing base (bd4c275) to head (968f976).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #50      +/-   ##
==========================================
+ Coverage   95.03%   95.11%   +0.07%     
==========================================
  Files          12       12              
  Lines         383      389       +6     
==========================================
+ Hits          364      370       +6     
  Misses         19       19              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tlambert03

Copy link
Copy Markdown
Member

thanks @cmalinmayor! I'll have a look at this today (if the test failures are unrelated, let me just touch up the repo again)

@cmalinmayor

cmalinmayor commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

Claude says witty needs typing_extensions in it's dependencies, hence the failures that are on main. Seems like the fix doesn't actually work on the Windows CI though (I couldnt' test locally), so I'll need to iterate

@tlambert03

Copy link
Copy Markdown
Member

ok, I'll do the updates in parallel

@cmalinmayor

Copy link
Copy Markdown
Contributor Author

Alright the fix seems to work now, and the general approach of mangling c++ keywords that are passed from user-specified attributes into the c++ template seems like a good idea to avoid other unforeseen issues

@codspeed-hq

codspeed-hq Bot commented Jun 9, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 6 untouched benchmarks


Comparing windows-name-collision-fix (968f976) with main (bd4c275)

Open in CodSpeed

@tlambert03

Copy link
Copy Markdown
Member

thanks @cmalinmayor, good fix 👍

@tlambert03 tlambert03 merged commit 9c30091 into main Jun 9, 2026
21 checks passed
@tlambert03 tlambert03 deleted the windows-name-collision-fix branch June 9, 2026 16:52
@tlambert03

Copy link
Copy Markdown
Member

just pushed tag v0.0.6 ... so this should be released shortly

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.

3 participants