Should build with -Wunused and -pedantic

[lattner]

Bugzilla Link	786
Resolution	FIXED
Resolved on	Mar 06, 2010 14:00
Version	trunk
OS	All
CC	@asl

Extended Description

This isn't really a makefile bug, but there is no better place to put it.

We should stop passing -Wno-unused to the LLVM build, and we should start passing -pedantic to the
LLVM build.

Making the change is easy, the "hard" part is trying it out any fixing any of the minor issues that pop up.

-Chris

[lattner]

assigned to @asl

[asl]

We should supply -Wno-long-long in this case. Or do something with "long long".

[lattner]

Is long long a problem? If so, I'd suggest making it a separate bug, because fixing it would be
significantly harder than fixing -Wunused and -pedantic.

-Chris

[asl]

According to http://gcc.gnu.org/onlinedocs/gcc/Long-Long.html#Long%20Long long long is an GCC extension for C89 & C++ (not C99). That's why -pedantic produces
an error telling something like "ISO C++ does not support `long long'".

[lattner]

Ooooh, I was confused, sorry! Right, using "-pedantic -Wno-long-long" would be a great first step. As a
second step, we could remove -Wno-long-long.

I'm sorry, for some reason I thought -Wno-long-long would make gcc warn about long long. :)

-Chris

[llvmbot]

I have committed sufficient changes to allow LLVM to build with -pedantic. There
are still more warnings to take care of. Almost all of them are "long long" usage.

One option for this bug is to now turn -pedantic on and have various owners fix
the warnings in their code.

Here's my patches:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20060522/035052.html
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20060522/035053.html
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20060522/035054.html
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20060522/035055.html
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20060522/035056.html
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20060522/035059.html
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20060522/035061.html
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20060522/035057.html
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20060522/035060.html
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20060522/035058.html

[llvmbot]

I recompiled again with -Wno-long-long and the only warning was in
lib/Target/CBackend/Writer.cpp:

In function bool isFPCSafeToPrint(const llvm::ConstantFP*)': Writer.cpp:535: warning: ISO C++ does not support the %a' printf format
Writer.cpp: In member function void <unnamed>::CWriter::printConstant(llvm::Constant*)': Writer.cpp:709: warning: ISO C++ does not support the %a' printf format

The function is safely wrapped in HAVE_PRINTF_A and looks like:

static bool isFPCSafeToPrint(const ConstantFP *CFP) {
#if HAVE_PRINTF_A
char Buffer[100];
sprintf(Buffer, "%a", CFP->getValue());

if (!strncmp(Buffer, "0x", 2) ||
!strncmp(Buffer, "-0x", 3) ||
!strncmp(Buffer, "+0x", 3))
return atof(Buffer) == CFP->getValue();
return false;
#else

I'm not sure what to do about this one. All the others are long long related.

[lattner]

Thanks Reid!

I think GCC has an extension token, maybe it can be used here?

I'm not in favor of turning of turning on the option until the code is completely cleaned up. I will see if I
can help out when I have time.

-Chris

[llvmbot]

Just a note to indicate that -Wno-long-long is useful in conjunction with
-pedantic. A case in point is Target/CBackend/Writer.cpp, line 688:

sprintf(Buffer, "0x%llx", static_cast(uint64_t(ll)));

With -pedantic we get an error (don't support long long) and a warning (don't
support 0x%llx). With -Wno-long-long we get neither.

[llvmbot]

As of this date, -pedantic and -Wno-long-long are turned on by default. The
local makefiles in a few places turn it off again to squelch warnings. These
things should be taken care of before this bug is resolved. The remaining
offenders are:

lib/System/rtld.c
lib/Target/CBackend/Writer.cpp
projects/Stacker/lib/runtime/stacker_rt.cpp
projects/sample/lib/sample/sample.cpp
runtime/libdummy/dummylib.c
runtime/GC/SemiSpace/semispace.c
runtime/GCCLibraries/libc/atox.c
runtime/libtrace/tracelib.c

[llvmbot]

These need to be addressed:

/proj/llvm/build/../llvm/lib/Target/CBackend/Writer.cpp: In function bool isFPCSafeToPrint(const llvm::ConstantFP*)': /proj/llvm/build/../llvm/lib/Target/CBackend/Writer.cpp:535: warning: ISO C++ does not support the %a' printf format
/proj/llvm/build/../llvm/lib/Target/CBackend/Writer.cpp: In member function
void <unnamed>::CWriter::printConstant(llvm::Constant*)': /proj/llvm/build/../llvm/lib/Target/CBackend/Writer.cpp:709: warning: ISO C++ does not support the %a' printf format

[llvmbot]

Another hard error:
LiveIntervalAnalysis.cpp:218: error: floating constant exceeds range of 'double'

This only happens on the persephone nightly tester (Mac OS/X).

This has been temporarily disabled until we can get a configure test going to
check for HUGE_VAL

[lattner]

Reid, don't we compile with -pedantic now? If so, should we split this into two bugs (one for -pedantic
and one for -Wunused) and close the former?

-Chris

[llvmbot]

Chris,

Currently both -pedantic and -Wno-unused are turned on by default (in
Makefile.rules). However, several directories override this. For example, see
lib/System/Makefile.

I don't think there's much point in create a bug for each option. We are not
fully -pedantic nor fully -Wno-unused clean. The details are in this bug and we
should keep them here.

Reid.

[lattner]

Ok, sounds good, makes sense.  I forgot that some dirs disabled the warning.  BTW, we want -Wunused,
not -Wno-unused (which disables the warning).

Thanks,

-Chris

[llvmbot]

The -pedantic option is done. Its turned off where it needs to be.

We currently compile with -Wno-unused. Changing it to -Wunused leads to hundreds
of warnings. Either we keep it as is and resolve this bug or change it to
-Wunused and start fixing the warnings. I vote for the former

[asl]

Well, ok. Will try to fight with -Wunused this weekend.

[asl]

It seems, we should really use -Wno-unused-parameter.

[llvmbot]

I think the combination of -Wunused and then -Wno-unused-parameter would be
good. We don't care about unused parameters and those warnings can be silenced.
How many warnings do you get if you use that combination?

[llvmbot]

I just tried it. That seems like a good combination. Its only picking up unused
variables now, which is something we definitely want to be warned about.

[asl]

Output from compiler
Please find the complete "warnings" list produced by gcc. Some of unused
variables were in fact already removed by recent commits.

[lattner]

Thanks Anton! It would be great to be -Wunused clean for the release!

-Chris

[llvmbot]

The -Wunused and -Wno-unused-parameter flags are now turned on by default. Most
of the changes necessary to clean up LLVM have been made. A few remain, in these
categories:

1. Unused functions that I didn't want to delete without review.
2. Unused variables in generated code (not much we can do)

Developers and maintainers should strive to clean up the remaining few warnings
as they encounter them.

This bug is done.

[llvmbot]

These are the ones that remain:

include/llvm/Analysis/DataStructure/DSGraphTraits.h:113:
warning: 'const llvm::DSNode& llvm::dereferenceC(const llvm::DSNode*)' defined
but not used

utils/PerfectShuffle/PerfectShuffle.cpp:64:
warning: 'short unsigned int getLHSOnlyMask(short unsigned int)' defined but
not used

utils/TableGen/Lexer.cpp:886:
warning: label `find_rule' defined but not used

lib/Transforms/Scalar/CorrelatedExprs.cpp:1152:
warning: 'bool CheckCondition(llvm::Constant*, llvm::Constant*,
llvm::Instruction::BinaryOps)' defined but not used

lib/AsmParser/Lexer.cpp:1222:
warning: label `find_rule' defined but not used

lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp:161:
warning: 'const llvm::TargetRegisterClass* getRegClass(llvm::SUnit*, const
llvm::TargetInstrInfo*, const llvm::MRegisterInfo*, llvm::SSARegMap*)' defined
but not used

lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp:183:
warning: 'unsigned int getNumResults(llvm::SUnit*)' defined but not used

lib/CodeGen/SelectionDAG/SelectionDAG.cpp:44:
warning: 'bool isInvertibleForFree(llvm::SDOperand)' defined but not used

lib/CodeGen/SelectionDAG/SelectionDAG.cpp:260:
warning: 'unsigned int getNodeIDOpcode(llvm::FoldingSetNodeID&)' defined but
not used

lib/Target/X86/X86ISelDAGToDAG.cpp:934:
warning: 'bool isRegister0(llvm::SDOperand)' defined but not used

lib/Target/X86/X86RegisterInfo.cpp:1068:
warning: unused variable 'EBPOffset'

lib/Target/Sparc/Sparc.h:82:
warning: 'const char* llvm::SPARCCondCodeToString(llvm::SPCC::CondCodes)'
defined but not used

lib/Target/PowerPC/PPCGenSubtarget.inc:468:
warning: 'NoItineraries' defined but not used

lib/Target/Alpha/AlphaGenDAGISel.inc:290:
warning: unused variable 'N'

lib/Target/Alpha/AlphaRegisterInfo.cpp:55:
warning: 'int getUID()' defined but not used

lib/Target/Alpha/AlphaGenSubtarget.inc:118:
warning: 'NoItineraries' defined but not used

lib/Target/IA64/IA64ISelLowering.cpp:134:
warning: 'bool isFloatingPointZero(llvm::SDOperand)' defined but not used

[llvmbot]

One more thing, we still have -pedantic turned off in:

lib/AsmParser/Makefile
lib/System/Makefile (ltdl.c)
projects/Stacker/lib/compiler/Makefile
runtime/GC/SemiSpace/Makefile
runtime/GCCLibraries/libc/Makefile
runtime/libdummy/Makefile
runtime/libtrace/Makefile
utils/TableGen/Makefile

These should eventually be cleaned up too.

[lattner]

mentioned in issue llvm/llvm-bugzilla-archive#981