Bugzilla – Bug 114
LLVM header files should be -Wold-style-cast clean
Last modified: 2003-11-16 14:50:09
You need to log in before you can comment on or make changes to this bug.
Some of the LLVM header files (notably include/Support/Casting.h) use C Style casting. Given that LLVM is a modern C++ application, it should really be using C++ style casting so that users that choose to utilize the -Wold-style-cast option to the compiler don't get warnings.
Changed the subject line to indicate why this matters. -Chris
This isn't working out so well. I'm comfortable changing: (Type*)x to: static_cast<Type*>(x) but this fails to compile. This means that there are potential bugs in the program. The problem has to do with the lack of definition of "Type" in the header files. In the cases where it doesn't work "Type" is just declared with: class Type; This prevents the compiler from understanding what Type is and whether x can safely be casted as a Type without introducing additional code (what a static cast does). Using dynamic_cast (which might introduce additional code) has the same problem. To get it to compile I have to use reinterpret_cast which is not comfortable. This basically tells the compiler to forget about casting, trust the programmer, reinterpret the bits as the new type. Although (Type*)x == reinterpret_cast<Type*>(x), I'm uncomfortable because it completely subverts the compilers attempt to check type cast correctness. I'm going to do the conversion, using reinterpret_cast where I must, and run the regression tests. If that works reasonably well, I'll submit the patch. Otherwise, I'll resolve this as WONTFIX. Reid.
That should be fine. There are many places in the compiler where it assumes that the value heirarchy is single inheritence from the Value class down. This dramatically reduces header file interdepenencies, so this is a good thing. You patch sounds fine, though I agree that reinterpret_cast is unsightly. -Chris
Regression Test Results After Changes: --- STATISTICS --------------------------------------------------------------- 721 tests total 701 ( 97%) tests as expected 8 ( 1%) tests unexpected FAIL 4 ( 1%) tests unexpected UNTESTED 8 ( 1%) tests unexpected PASS --- TESTS WITH UNEXPECTED OUTCOMES ------------------------------------------- Feature.ad.core : UNTESTED, expected PASS Could not load test. Feature.cc.core : UNTESTED, expected PASS Could not load test. Feature.mc.core : UNTESTED, expected PASS Could not load test. Feature.opt.core : UNTESTED, expected PASS Could not load test. Regression.C++Frontend.2003-08-28-ForwardType : XPASS Regression.C++Frontend.2003-11-08-ArrayAddress: FAIL , expected PASS Script: /proj/work/llvmobj/test/tmp/tr2003-11-08-ArrayAddress.cpp.tr/testscript.2003-11-08-ArrayAddress.cpp.tr Output: /proj/work/llvmobj/test/tmp/tr2003-11-08-ArrayAddress.cpp.tr/testscript.2003-11-08-ArrayAddress.cpp.tr.out Regression.CFrontend.2003-08-17-DeadCodeShortCircuit: XPASS Regression.CFrontend.2003-11-12-VoidString : FAIL , expected PASS Compiling C code failed Regression.CFrontend.2003-11-13-TypeSafety : FAIL , expected PASS Script: /proj/work/llvmobj/test/tmp/tr2003-11-13-TypeSafety.c.tr/testscript.2003-11-13-TypeSafety.c.tr Output: /proj/work/llvmobj/test/tmp/tr2003-11-13-TypeSafety.c.tr/testscript.2003-11-13-TypeSafety.c.tr.out Regression.Jello.test-arith : FAIL , expected PASS LLI failed to execute bytecode Regression.Jello.test-fp : FAIL , expected PASS LLI failed to execute bytecode Regression.Jello.test-shift : FAIL , expected PASS LLI failed to execute bytecode Regression.LLC.select : FAIL , expected PASS Failed to assemble /proj/work/llvm/test/Regression/LLC/select.ll to file /proj/work/llvmobj/test/tmp/feature-mc-select.ll Regression.Transforms.BasicAA.2003-11-04-SimpleCases: FAIL , expected PASS Script: /proj/work/llvmobj/test/tmp/tr2003-11-04-SimpleCases.ll/testscript.2003-11-04-SimpleCases.ll Output: /proj/work/llvmobj/test/tmp/tr2003-11-04-SimpleCases.ll/testscript.2003-11-04-SimpleCases.ll.out Regression.Transforms.ConstProp.logicaltest : XPASS Regression.Transforms.FunctionResolve.2003-04-18-ForwardDeclGlobal: XPASS Regression.Transforms.IndVarsSimplify.2003-09-23-NotAtTop: XPASS Regression.Transforms.Inline.alloca_test : XPASS Regression.Transforms.LICM.2003-08-04-TrappingInstHoist: XPASS Regression.Transforms.LICM.2003-08-04-TrappingInstOkHoist: XPASS
Created an attachment (id=22) [details] Patch to remove C-style casts from header files. Since the regression test results turned out okay, I'm submitting this patch. It contains more reinterpret_cast<> casts than I would prefer, but these were necessary and it didn't seem to break anything.
The four "UNTESTED" tests resulted from previous test runs that produced core files. The testing script was attempting to run the core files as tests. We might want to consider adjusting the QMTest configuration to use a filename pattern match before it attempts to run a test (I'd do it but I don't know how).
Look good, my only comment is that the lines should be wrapped at 80 columns. I've wrapped them and applied the patch: http://mail.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20031110/009559.html http://mail.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20031110/009560.html Thanks a lot! -Chris
Oops, right. Forgot to mark this fixed. -Chris