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

djgpp compatibility #2820

Closed
wants to merge 3 commits into from
Closed

djgpp compatibility #2820

wants to merge 3 commits into from

Conversation

jwt27
Copy link

@jwt27 jwt27 commented Mar 18, 2022

These two patches make the library compile on djgpp (using gcc 10.3.0). Everything seems to work, although the test cases don't compile, due to excessive usage of std::wstring. They would probably work if you replaced all those, but I don't really see the purpose in doing that.

I've wanted to use fmtlib for a long time, but making it compatible with djgpp always seemed like a lot of work, so I kept putting it off. I thought everything related to wide character strings would have to be #ifdef'd out.
Turns out the only problem is std::wstring, which is only ever used on two lines. :)

@jwt27
Copy link
Author

jwt27 commented Mar 19, 2022

This fixes #1077.

Copy link
Contributor

@vitaut vitaut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR but missing wstring and wostream is non-conformant and we shouldn't be providing workarounds for this. You can define those aliases in your code before including {fmt}.

@jwt27
Copy link
Author

jwt27 commented Mar 20, 2022

Could those typedefs be provided by fmt (as is the case for wstring_view)? Technically adding anything in namespace std is undefined behavior, according to the standard.

edit: also it's not possible to build a static library if wstring/wostream are not available in the std headers.

This is the "correct" way to find the page size on DPMI.  In practice
though, this could be hardcoded to return 4096 and no one would ever
notice the difference.
COFF objects don't support visibility, so using this attribute
needlessly generates a long list of warnings.
This is for systems with no libc support for wide character strings
(specifically djgpp).  libstdc++ then doesn't define these aliases,
but the primary templates are still available.
@jwt27 jwt27 requested a review from vitaut March 20, 2022 18:22
@vitaut
Copy link
Contributor

vitaut commented Mar 21, 2022

Could those typedefs be provided by fmt (as is the case for wstring_view)?

This is essentially the same workaround that I don't think is worth doing considering a very niche nonconformant target. You might want reporting this to djgpp maintainers if there are any and/or patching {fmt} locally.

@vitaut vitaut closed this Mar 21, 2022
@jwt27
Copy link
Author

jwt27 commented Mar 21, 2022

It's been reported there before and nothing was ever done. I think it'll be significantly easier to keep fmt compatible with djgpp rather than the other way around - a whole bunch of wchar_t string functions would need to be implemented in libc, that likely no one will ever use, only to gain two typedefs in libstdc++.

@jwt27 jwt27 mentioned this pull request Apr 4, 2023
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