Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove uses of std::string from exception throwing #21

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

swuerl
Copy link

@swuerl swuerl commented Dec 2, 2024

This uses the micropython-internal mp_obj_new_exception_msg_varg to format exception strings rather than std::string concatenation. Also changed the <class>_locals string to use micropython-native string concatenation.

@swuerl
Copy link
Author

swuerl commented Dec 2, 2024

I'm again very confused about the CI, the missing reference exists here, also in the windows-pyd branch: https://github.com/stinos/micropython/blob/windows-pyd/py/vstr.c#L176

@stinos
Copy link
Owner

stinos commented Dec 3, 2024

Dynamic linking with msvc or similar is different than for gcc: functions/data needs to be exported explicitly. Typical solution would be to add the missing function to micropython\ports\windows\msvc\exports.def which is annoying because it is in a separate repository.. Could be fixed with a pre-build event, but I've just added it to the windows-pyd branch so if built again this should now link fine.

@swuerl swuerl force-pushed the do-not-use-std-string branch from 06b2ab2 to 0cfb1e1 Compare December 3, 2024 14:08
@swuerl swuerl force-pushed the do-not-use-std-string branch from 0cfb1e1 to fa6a119 Compare December 3, 2024 14:15
@stinos
Copy link
Owner

stinos commented Dec 3, 2024

Appveyor seems to have just updated their toolchain so this is now failing on micropython/micropython#16352..

@swuerl
Copy link
Author

swuerl commented Dec 3, 2024

Ah makes sense. Thank you for looking into it, was just trying to figure out what's going on there.

I'll see if I can spin up a VM for windows testing; I'm currently relying on the CI here

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