lvandeve / lodepng

PNG encoder and decoder in C and C++.
zlib License
2.07k stars 422 forks source link

Replace "//" to "/* text */" #6

Closed DeXP closed 9 years ago

DeXP commented 9 years ago

GCC warn me, that C90 is not supports "//" (C++ commentary style). Can you replace it in original files too? I do not want to use "Altered source" mark.

lvandeve commented 9 years ago

Hello,

LodePNG is designed to work with C90. I don't get this warning with "gcc lodepng.c --ansi --pedantic -W -Wall -Wextra".

Could you please let me know which compiler flags you are using, and your gcc version (gcc -v), then I can try to reproduce it.

Thanks a lot for your help!

-Lode

DeXP commented 9 years ago

Hi! "-c -Wall -O2 -pedantic -Wextra -Wall" "-O2" enables this warning in that flags combination. First got this error on Windows, CodeBlocks 13.12, and old GCC packeged in. Now I reproduced it in Arch Linux with latest gcc:

$ gcc -v COLLECT_GCC=gcc COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-unknown-linux-gnu/4.9.2/lto-wrapper Целевая архитектура: x86_64-unknown-linux-gnu Параметры конфигурации: /build/gcc/src/gcc-4.9-20150204/configure --prefix=/usr --libdir=/usr/lib --libexecdir=/usr/lib --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=https://bugs.archlinux.org/ --enable-languages=c,c++,ada,fortran,go,lto,objc,obj-c++ --enable-shared --enable-threads=posix --with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions --enable-clocale=gnu --disable-libstdcxx-pch --disable-libssp --enable-gnu-unique-object --enable-linker-build-id --enable-cloog-backend=isl --enable-lto --enable-plugin --enable-install-libiberty --with-linker-hash-style=gnu --disable-multilib --disable-werror --enable-checking=release Модель многопоточности: posix gcc версия 4.9.2 20150204 (prerelease) (GCC)

lvandeve commented 9 years ago

Interesting. LodePNG has some code inside of an #ifdef __cplusplus, and that code uses "//" style comments. Normally a C compiler is not even supposed to see that code.

Also, this warning appears when using the -pedantic flag but not using the -ansi or -std=c89 flag. So if you add -std=c89 or -ansi, it works without the warning. Is that an acceptable solution for you?

I could also change the // to /* */ inside of the __cplusplus blocks, though I wonder if this isn't a gcc quirk...

DeXP commented 9 years ago

lodepng.h has a lot of "//" in C-block.

Why not replace all "//" to "/ * * /"? It is a minute of work, and all compilers supports "/ *" comments.

"-ansi" is good workaround for me, thanks.

lvandeve commented 9 years ago

LodePNG is indeed intended to work with C89/C90 so I fully support fixing this if needed.

I can confirm that currently, there are no "//" comments in C-code, only in code behind __cplusplus or LODEPNG_COMPILE_CPP #ifdefs. Otherwise it would have triggered an error with -ansi.

However there is a subtle cause:

Apparently the C99 spec says that comments are parsed in the translation phase, before the preprocessor. C89/C90 on the other hand doesn't specify this.

So maybe without the -std=c89 or -ansi flag, it is processing the comments first and triggering that warning (a warning that ironically is about C90), while with that flag, it does the preprocessor first...

In any case, maybe this does warrant changing the // comments to /**/ inside the C++ blocks. Sadly that ruins the C++ codestyle of those blocks a little bit... I'll see.

Note that the clang compiler, having the same interface as gcc, never warns about this even with "-c -Wall -O2 -pedantic -Wextra -Wall".

Here is a minimal piece of code that shows the gcc behaviour:

int main() { return 0; }

ifdef __cplusplus

// C++ style comment in __cplusplus #ifdef

endif

Thanks a lot for pointing this out!

lvandeve commented 9 years ago

Changed all comments.