gherkin: (C) Fix Windows related issues #193
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
On Windows wchar_t is 2 bytes large, and use UTF-16. This means that for the case of code points > 0xFFFF (and wchar_t is only 2 bytes large), the code point read from the UTF-8 source need to be converted to two UTF-16 surrogates (wchar_t wide characters).
Windows does not use UTF-8 by default. To make sure that UTF-8 is used in the output, do manual conversion from unicode/UTF-16 to an UTF-8 byte sequence for output. It seems safer that to try to add Windows specific code to set UTF-8 output on that platform.
To support a wider range of compilers, remove the C90 breaches so that the code compiles also with -ansi and/or -std=c90 (using gcc). Another reason to not use swprintf is that on many Windows compilers swprintf has a different signature than the ISO standard specification. When cleaning up commented out code in gherkin_generate_tokens.c (to remove the usage of // for comments), settle to use the FileTokenScanner to get coverage of it in the acceptance tests. The code does not compile with -std=c90 -pedantic (using gcc), the conformance to C90 is not taken that far.
Technically C does not support repeated typedefs until C11, however modern compilers do allow it (gcc also using -std=c90). But since gcc <v4.6 does not support that, remove the usage of them.
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fix Windows related issues:
wchar_tis 2 bytes in size.swprintfIn addition make the code conform to C90.
Details
When looking into #168, a set of problem were found and fixed.
wchar_tis 2 bytes in size.Most Windows compilers use 2 bytes for wide characters (
wchar_t), that mean the not all unicode code points can be represented by a wide character. To encode code points from the Supplementary Planes UTF-16 are used (using two wide characters for each code point). The decoding of UTF-8 (which is done directly in the code) need to use UTF-16 in the internal wide character representation when necessary.Windows generally does not use UTF-8, even when using unicode it is most likely UTF-16 and not UTF-8. Therefore it seems best to always do the UTF-8 encoding directly in the code, instead of relying on print functions form the complier libraries.
swprintfMany (older) Windows compilers use a different signature for
swprintfthan the ISO standard (excluding the count argument in the signature), therefore it is better to avoid using it and instead calculate the string representation of the error location directly (which was the only use ofswprintf).Another reason to avoid
swprintfis that it did not exist in the ISO standard until C99.In addition make the code conform to C90.
After the use of
swprintfwas removed, only the use of//for some commented out code was only thing that inhibit the compilation of the code with GCC (>v4.6) using-std=c90(or-ansi), so the use of//was removed.Interestingly enough using -std=c90` with GCC (>v4.6) did not complain on repeated typedefs even though they are not allowed until C11. GCC (<v4.6) on the other hand does not allow them at all, so they are removed.
Motivation and Context
Fixes #168.
How Has This Been Tested?
On
OS name: "linux", version: "4.8.0-36-generic", arch: "amd64", family: "unix"(ubuntu 16.04) using:gcc (Ubuntu 5.4.0-6ubuntu1~16.04.4) 5.4.0 20160609clang version 3.8.0-2ubuntu4 (tags/RELEASE_380/final)gcc (Alpine 6.2.1) 6.2.1 20160822(docker)On
OS name: "linux", version: "4.4.0-72-generic", arch: "i386", family: "unix"(ubuntu 16.04) using:gcc (Ubuntu 5.4.0-6ubuntu1~16.04.4) 5.4.0 20160609clang version 3.8.0-2ubuntu4 (tags/RELEASE_380/final)On
OS name: "windows 7", version: "6.1", arch: "amd64", family: "windows"using:x86_64-w64-mingw32-gcc (GCC) 5.4.0x86_64-pc-cygwin-gcc (GCC) 5.4.0On
OS name: "windows xp", version: "5.1", arch: "x86", family: "windows"using:i686-pc-mingw32-gcc (GCC) 4.5.2i686-pc-cygwin-gcc (GCC) 4.7.3Types of changes
Checklist: