Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

Old cast warning in Extern "C" code. #36323

Open Quuxplusone opened 6 years ago

Quuxplusone commented 6 years ago
Bugzilla Link PR37350
Status REOPENED
Importance P normal
Reported by Francois Budin (francois.budin@kitware.com)
Reported on 2018-05-07 06:15:12 -0700
Last modified on 2018-05-07 11:51:05 -0700
Version trunk
Hardware All All
CC dblaikie@gmail.com, dgregor@apple.com, llvm-bugs@lists.llvm.org, t.p.northover@gmail.com
Fixed by commit(s)
Attachments main.cxx (139 bytes, text/x-c++src)
Blocks
Blocked by
See also

Created attachment 20268 Simple C++ code to reproduce the warning.

The bug I found is a warning that should not be thrown, and is not thrown with GCC. When compiling a piece of code that is inside an -- extern "C" -- statement, and that contains a C-style casting, clang throws an old style cast warning (or error if the -Werror flag is used). But since it is inside a section of code that is declared as C, I don't think that there should be any warning.

Note that GCC does not throw this warning when compiling the same piece of code. I found a link to a patch proposed a long time ago for GCC to solve that same bug. I did not explore GCC's code to see if that exact patch had been merged or if this problem had been solved in a different way. For reference, here is the discussion I found about the same bug for GCC: https://gcc.gnu.org/ml/gcc-patches/1999-07n/msg00229.html

Note that this bug is reproducible with the latest compilation of clang. A simple way to reproduce this problem is to try to compile the code attached.

Thanks, Francois

Command line to run:

Note that this command line is the sub-command line that actually produces the problem and allows to add a break point where the warning is thrown [1]. One could run a simple compilation command line [2] and get the warning, but it is more difficult to debug as the warning appears in a compilation sub-process in that case.

Note also that {clang_build} should be replaced by the directory in which clang is built, and ${test_directory} should be replaced by where main.cxx is saved.

clang-7 -cc1 -triple x86_64-unknown-linux-gnu -emit-obj -mrelax-all -disable-free -main-file-name main.cxx -mrelocation-model static -mthread-model posix -mdisable-fp-elim -fmath-errno -masm-verbose -mconstructor-aliases -munwind-tables -fuse-init-array -target-cpu x86-64 -dwarf-column-info -debugger-tuning=gdb -v -resource-dir ${clang_build}/lib/clang/7.0.0 -internal-isystem /usr/lib/gcc/x86_64-linux-gnu/7.3.0/../../../../include/c++/7.3.0 -internal-isystem /usr/lib/gcc/x86_64-linux-gnu/7.3.0/../../../../include/x86_64-linux-gnu/c++/7.3.0 -internal-isystem /usr/lib/gcc/x86_64-linux-gnu/7.3.0/../../../../include/x86_64-linux-gnu/c++/7.3.0 -internal-isystem /usr/lib/gcc/x86_64-linux-gnu/7.3.0/../../../../include/c++/7.3.0/backward -internal-isystem /usr/local/include -internal-isystem ${clang_build}/lib/clang/7.0.0/include -internal-externc-isystem /usr/include/x86_64-linux-gnu -internal-externc-isystem /include -internal-externc-isystem /usr/include -Wold-style-cast -fdeprecated-macro -fdebug-compilation-dir ${test_directory} -ferror-limit 19 -fmessage-length 204 -fobjc-runtime=gcc -fcxx-exceptions -fexceptions -fdiagnostics-show-option -fcolor-diagnostics -o /tmp/main-d474b2.o -x c++ main.cxx

Output

main.cxx:6:10: warning: use of old-style cast [-Wold-style-cast] return (float)i; ^ ~ 1 warning generated.


[1] https://github.com/llvm-project/llvm-project-20170507/blob/master/clang/lib/Sema/SemaExpr.cpp#L6144-L6146 [2] clang++ -v -Wold-style-cast main.cxx

Quuxplusone commented 6 years ago

Attached main.cxx (139 bytes, text/x-c++src): Simple C++ code to reproduce the warning.

Quuxplusone commented 6 years ago

But since it is inside a section of code that is declared as C, I don't think that there should be any warning.

This isn't what 'extern "C"' means. It changes the low-level name of functions so that they can be conveniently called from C code, but the source inside an 'extern "C"' block is still normal C++. It can use the usual C++ style casts and there's no more (or less) reason to prefer the old-style ones.

Also, this warning has to be enabled manually (it's not even in -Wall) so I see no reason to change its behaviour inside that kind of block.

Quuxplusone commented 6 years ago

Actually, outright closing it now might be a bit hasty. Let's see if anything develops.

Quuxplusone commented 6 years ago
Thank you for the explanation. Since 'extern "C"' is to change the linkage and
not the function language, it is indeed reasonable to print a warning in this
case. It is interesting though that GCC interpreted this differently. For
reference, I looked into the GCC source code and the line that is important
here is line 9049 in:
https://raw.githubusercontent.com/gcc-mirror/gcc/master/gcc/cp/parser.c (line
9049).
Quuxplusone commented 6 years ago

As a side note, Visual Studio compiler is not taking a stand in the interpretation of this issue as it does not seem to implement this warning.