Skip to content

Disable MSVC warning 4819#1786

Closed
FantasqueX wants to merge 1 commit intozlib-ng:developfrom
FantasqueX:disable-4819
Closed

Disable MSVC warning 4819#1786
FantasqueX wants to merge 1 commit intozlib-ng:developfrom
FantasqueX:disable-4819

Conversation

@FantasqueX
Copy link
Copy Markdown
Contributor

Currently, zlib-ng doesn't compile on my machine due to warning C4819: The file contains a character that cannot be represented in the current code page (936). Save the file in Unicode format to prevent data loss.

The non-ascii characters live in zbuild.h.

 * gcc says: "warning: implicit declaration of function ‘...’"
 * g++ says: "error: new declaration ‘...’ ambiguates built-in declaration ‘...’"

Because zlib-ng treats warning as error, the project doesn't compile. If this warning is disabled, zlib-ng compiles well.

Currently, zlib-ng doesn't compile on my machine due to warning C4819: The file contains a character that cannot be represented in the current code page (936). Save the file in Unicode format to prevent data loss.

The non-ascii characters live in zbuild.h.
```
 * gcc says: "warning: implicit declaration of function ‘...’"
 * g++ says: "error: new declaration ‘...’ ambiguates built-in declaration ‘...’"
```

Because zlib-ng treats warning as error, the project doesn't compile. If
this warning is disabled, zlib-ng compiles well.
@nmoinvaz
Copy link
Copy Markdown
Member

Is it possible to remove the character that is none ascii?

@FantasqueX
Copy link
Copy Markdown
Contributor Author

Is it possible to remove the character that is none ascii?

It's a possible solution if maintainers think removing those characters is OK.

@codecov
Copy link
Copy Markdown

codecov bot commented Sep 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 33.23%. Comparing base (c939498) to head (3a5919e).
Report is 6 commits behind head on develop.

❗ There is a different number of reports uploaded between BASE (c939498) and HEAD (3a5919e). Click for more details.

HEAD has 6 uploads less than BASE
Flag BASE (c939498) HEAD (3a5919e)
7 1
Additional details and impacted files
@@             Coverage Diff              @@
##           develop    #1786       +/-   ##
============================================
- Coverage    83.34%   33.23%   -50.12%     
============================================
  Files          132       66       -66     
  Lines        10019     5504     -4515     
  Branches      2687     1227     -1460     
============================================
- Hits          8350     1829     -6521     
- Misses        1010     3419     +2409     
+ Partials       659      256      -403     

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

@mtl1979
Copy link
Copy Markdown
Collaborator

mtl1979 commented Sep 18, 2024

As far as I know full UTF-8 support (without requiring BOM at start of file) only exists in later versions of Windows 10, and Windows 11, so if the file was edited in Windows, it would default to ISO-8859-15 (Latin-15, or Latin-1 + Euro sign), which is the default in a lot of English-speaking countries in Europe.

I'm all for making sure the source files either use UTF-8 or 7-bit ASCII, which are safe in all regions except some Asian countries using for example EUC...

@nmoinvaz
Copy link
Copy Markdown
Member

What is the characters or lines of code?

@mtl1979
Copy link
Copy Markdown
Collaborator

mtl1979 commented Sep 18, 2024

What is the characters or lines of code?

Lines 278-279 in current develop branch...

See also: https://learn.microsoft.com/en-us/cpp/build/reference/utf-8-set-source-and-executable-character-sets-to-utf-8?view=msvc-170

@nmoinvaz
Copy link
Copy Markdown
Member

If it is in a comment, then the comment should be fixed to be ascii characters instead of changing warning flag.

@FantasqueX
Copy link
Copy Markdown
Contributor Author

If it is in a comment, then the comment should be fixed to be ascii characters instead of changing warning flag.

OK, I'll create a PR to replace non-ascii characters later today.

@FantasqueX
Copy link
Copy Markdown
Contributor Author

#1791

@FantasqueX FantasqueX closed this Sep 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants