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

Update miniz_tdef.c to enable compiling in forced-C++ mode #280

Merged
merged 1 commit into from
Sep 28, 2023

Conversation

nyq
Copy link
Contributor

@nyq nyq commented Aug 15, 2023

Summary:

Merged definition of static const mz_uint s_tdefl_num_probes[11] with its declaration to avoid compilation error when compiling in forced-C++mode

Details:

When miniz_tdef.c is compiled in C++ mode (either by forcing the compiler to treat the input as C++ or by renaming the file into miniz_tdef.cpp), MSVC17 produces the following error:

miniz_tdef.cpp(2113,22): error C2086: 'const mz_uint s_tdefl_num_probes[11]': redefinition
miniz_tdef.cpp(1254,22): message : see declaration of 's_tdefl_num_probes'

This happens because in miniz_tdef.c we have the following:

/*Line 606:*/ static const mz_uint s_tdefl_num_probes[11];
/*Line 1465:*/ static const mz_uint s_tdefl_num_probes[11] = { 0, 1, 6, 32, 16, 32, 128, 256, 512, 768, 1500 };

Such construct is legal in C, but not legal in C++. While miniz_tdef.c is a C source file and not C++, sometimes it is used in C++ projects where settings are such that mixed C/C++ compilation is not allowed and all input source files are treated as forced C++. So there would be no harm to make a small adjustment so that the source code is conformant with both C and C++ requirements.

There are two ways it can be done:

  1. Option A: change line 606 from static const mz_uint s_tdefl_num_probes[11]; to extern const mz_uint s_tdefl_num_probes[11];
  2. Option B: move line 1465 into line 606 so that the code looks like this:
/*Line 606:*/ static const mz_uint s_tdefl_num_probes[11] = { 0, 1, 6, 32, 16, 32, 128, 256, 512, 768, 1500 };
/*Line 1465:*/ //Nothing here

Either option works for both C and C++ and really there is no harm in simply moving the full definition up like in option B and avoid duplication.

This change implements option B. It was also tested and proven to work in amalgamated version.

Summary:
Merged definition of static const mz_uint s_tdefl_num_probes[11] with its declaration to avoid compilation error when compiling in forced-C++mode

Details:
When miniz_tdef.c is compiled in C++ mode (either by forcing the compiler to treat the input as C++ or by renaming the file into miniz.cpp), MSVC17 produces the following error:
```
miniz_tdef.cpp(2113,22): error C2086: 'const mz_uint s_tdefl_num_probes[11]': redefinition
miniz_tdef.cpp(1254,22): message : see declaration of 's_tdefl_num_probes'
```
This happens because in miniz_tdef.c we have the following:

```
/*Line 606:*/ static const mz_uint s_tdefl_num_probes[11];
/*Line 1465:*/ static const mz_uint s_tdefl_num_probes[11] = { 0, 1, 6, 32, 16, 32, 128, 256, 512, 768, 1500 };
```
While miniz_tdef.c is a C source file and not C++, sometimes it is used in C++ projects where settings are such that mixed C/C++ compilation is not allowed and all input source files are treated as forced C++. So there would be no harm to make a small adjustment so that the source code is conformant with both C and C++ requirements.

There are two ways it can be done:
Option 1: change line 606 from `static const mz_uint s_tdefl_num_probes[11];` to `extern const mz_uint s_tdefl_num_probes[11];`
Option2: eliminate line 1465 entirely and move line 2113 into line 1254 so that the code looks like this:
```
/*Line 606:*/ static const mz_uint s_tdefl_num_probes[11] = { 0, 1, 6, 32, 16, 32, 128, 256, 512, 768, 1500 };
/*Line 1465:*/ //Nothing here
```
Either option works for both C and C++ and really there is no harm in simply moving the full definition up like in option B and avoid duplication.

This change implements option B.
@uroni uroni merged commit acfba6a into richgel999:master Sep 28, 2023
@nyq nyq deleted the nyq-s_tdefl_num_probes-cpp branch September 28, 2023 20:16
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