Skip to content
This repository was archived by the owner on Nov 1, 2021. It is now read-only.

Commit 3ffe113

Browse files
joker-ephjoker-eph
committed
Turn cl::values() (for enum) from a vararg function to using C++ variadic template
The core of the change is supposed to be NFC, however it also fixes what I believe was an undefined behavior when calling: va_start(ValueArgs, Desc); with Desc being a StringRef. Differential Revision: https://reviews.llvm.org/D25342 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@283671 91177308-0d34-0410-b5e6-96231b3b80d8
1 parent 49695dd commit 3ffe113

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

41 files changed

+93
-158
lines changed

Diff for: ‎docs/CommandLine.rst

+5-10
Original file line numberDiff line numberDiff line change
@@ -355,8 +355,7 @@ library fill it in with the appropriate level directly, which is used like this:
355355
clEnumVal(g , "No optimizations, enable debugging"),
356356
clEnumVal(O1, "Enable trivial optimizations"),
357357
clEnumVal(O2, "Enable default optimizations"),
358-
clEnumVal(O3, "Enable expensive optimizations"),
359-
clEnumValEnd));
358+
clEnumVal(O3, "Enable expensive optimizations")));
360359

361360
...
362361
if (OptimizationLevel >= O2) doPartialRedundancyElimination(...);
@@ -401,8 +400,7 @@ program. Because of this, we can alternatively write this example like this:
401400
clEnumValN(Debug, "g", "No optimizations, enable debugging"),
402401
clEnumVal(O1 , "Enable trivial optimizations"),
403402
clEnumVal(O2 , "Enable default optimizations"),
404-
clEnumVal(O3 , "Enable expensive optimizations"),
405-
clEnumValEnd));
403+
clEnumVal(O3 , "Enable expensive optimizations")));
406404

407405
...
408406
if (OptimizationLevel == Debug) outputDebugInfo(...);
@@ -436,8 +434,7 @@ the code looks like this:
436434
cl::values(
437435
clEnumValN(nodebuginfo, "none", "disable debug information"),
438436
clEnumVal(quick, "enable quick debug information"),
439-
clEnumVal(detailed, "enable detailed debug information"),
440-
clEnumValEnd));
437+
clEnumVal(detailed, "enable detailed debug information")));
441438

442439
This definition defines an enumerated command line variable of type "``enum
443440
DebugLev``", which works exactly the same way as before. The difference here is
@@ -498,8 +495,7 @@ Then define your "``cl::list``" variable:
498495
clEnumVal(dce , "Dead Code Elimination"),
499496
clEnumVal(constprop , "Constant Propagation"),
500497
clEnumValN(inlining, "inline", "Procedure Integration"),
501-
clEnumVal(strip , "Strip Symbols"),
502-
clEnumValEnd));
498+
clEnumVal(strip , "Strip Symbols")));
503499

504500
This defines a variable that is conceptually of the type
505501
"``std::vector<enum Opts>``". Thus, you can access it with standard vector
@@ -558,8 +554,7 @@ Reworking the above list example, we could replace `cl::list`_ with `cl::bits`_:
558554
clEnumVal(dce , "Dead Code Elimination"),
559555
clEnumVal(constprop , "Constant Propagation"),
560556
clEnumValN(inlining, "inline", "Procedure Integration"),
561-
clEnumVal(strip , "Strip Symbols"),
562-
clEnumValEnd));
557+
clEnumVal(strip , "Strip Symbols")));
563558

564559
To test to see if ``constprop`` was specified, we can use the ``cl:bits::isSet``
565560
function:

Diff for: ‎include/llvm/CodeGen/CommandFlags.h

+11-21
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,7 @@ cl::opt<Reloc::Model> RelocModel(
5858
clEnumValN(Reloc::RWPI, "rwpi",
5959
"Read-write data relocatable, accessed relative to static base"),
6060
clEnumValN(Reloc::ROPI_RWPI, "ropi-rwpi",
61-
"Combination of ropi and rwpi"),
62-
clEnumValEnd));
61+
"Combination of ropi and rwpi")));
6362

6463
static inline Optional<Reloc::Model> getRelocModel() {
6564
if (RelocModel.getNumOccurrences()) {
@@ -76,8 +75,7 @@ TMModel("thread-model",
7675
cl::values(clEnumValN(ThreadModel::POSIX, "posix",
7776
"POSIX thread model"),
7877
clEnumValN(ThreadModel::Single, "single",
79-
"Single thread model"),
80-
clEnumValEnd));
78+
"Single thread model")));
8179

8280
cl::opt<llvm::CodeModel::Model>
8381
CMModel("code-model",
@@ -92,8 +90,7 @@ CMModel("code-model",
9290
clEnumValN(CodeModel::Medium, "medium",
9391
"Medium code model"),
9492
clEnumValN(CodeModel::Large, "large",
95-
"Large code model"),
96-
clEnumValEnd));
93+
"Large code model")));
9794

9895
cl::opt<llvm::ExceptionHandling>
9996
ExceptionModel("exception-model",
@@ -108,8 +105,7 @@ ExceptionModel("exception-model",
108105
clEnumValN(ExceptionHandling::ARM, "arm",
109106
"ARM EHABI exceptions"),
110107
clEnumValN(ExceptionHandling::WinEH, "wineh",
111-
"Windows exception model"),
112-
clEnumValEnd));
108+
"Windows exception model")));
113109

114110
cl::opt<TargetMachine::CodeGenFileType>
115111
FileType("filetype", cl::init(TargetMachine::CGFT_AssemblyFile),
@@ -120,8 +116,7 @@ FileType("filetype", cl::init(TargetMachine::CGFT_AssemblyFile),
120116
clEnumValN(TargetMachine::CGFT_ObjectFile, "obj",
121117
"Emit a native object ('.o') file"),
122118
clEnumValN(TargetMachine::CGFT_Null, "null",
123-
"Emit nothing, for performance testing"),
124-
clEnumValEnd));
119+
"Emit nothing, for performance testing")));
125120

126121
cl::opt<bool>
127122
EnableFPMAD("enable-fp-mad",
@@ -165,8 +160,7 @@ DenormalMode("denormal-fp-math",
165160
"the sign of a flushed-to-zero number is preserved "
166161
"in the sign of 0"),
167162
clEnumValN(FPDenormal::PositiveZero, "positive-zero",
168-
"denormals are flushed to positive zero"),
169-
clEnumValEnd));
163+
"denormals are flushed to positive zero")));
170164

171165
cl::opt<bool>
172166
EnableHonorSignDependentRoundingFPMath("enable-sign-dependent-rounding-fp-math",
@@ -184,8 +178,7 @@ FloatABIForCalls("float-abi",
184178
clEnumValN(FloatABI::Soft, "soft",
185179
"Soft float ABI (implied by -soft-float)"),
186180
clEnumValN(FloatABI::Hard, "hard",
187-
"Hard float ABI (uses FP registers)"),
188-
clEnumValEnd));
181+
"Hard float ABI (uses FP registers)")));
189182

190183
cl::opt<llvm::FPOpFusion::FPOpFusionMode>
191184
FuseFPOps("fp-contract",
@@ -197,8 +190,7 @@ FuseFPOps("fp-contract",
197190
clEnumValN(FPOpFusion::Standard, "on",
198191
"Only fuse 'blessed' FP ops."),
199192
clEnumValN(FPOpFusion::Strict, "off",
200-
"Only fuse FP ops when the result won't be affected."),
201-
clEnumValEnd));
193+
"Only fuse FP ops when the result won't be affected.")));
202194

203195
cl::opt<bool>
204196
DontPlaceZerosInBSS("nozero-initialized-in-bss",
@@ -269,8 +261,7 @@ JTableType("jump-table-type",
269261
clEnumValN(JumpTable::Simplified, "simplified",
270262
"Create one table per simplified function type."),
271263
clEnumValN(JumpTable::Full, "full",
272-
"Create one table per unique function type."),
273-
clEnumValEnd));
264+
"Create one table per unique function type.")));
274265

275266
cl::opt<llvm::EABI> EABIVersion(
276267
"meabi", cl::desc("Set EABI type (default depends on triple):"),
@@ -279,7 +270,7 @@ cl::opt<llvm::EABI> EABIVersion(
279270
"Triple default EABI version"),
280271
clEnumValN(EABI::EABI4, "4", "EABI version 4"),
281272
clEnumValN(EABI::EABI5, "5", "EABI version 5"),
282-
clEnumValN(EABI::GNU, "gnu", "EABI GNU"), clEnumValEnd));
273+
clEnumValN(EABI::GNU, "gnu", "EABI GNU")));
283274

284275
cl::opt<DebuggerKind>
285276
DebuggerTuningOpt("debugger-tune",
@@ -289,8 +280,7 @@ DebuggerTuningOpt("debugger-tune",
289280
clEnumValN(DebuggerKind::GDB, "gdb", "gdb"),
290281
clEnumValN(DebuggerKind::LLDB, "lldb", "lldb"),
291282
clEnumValN(DebuggerKind::SCE, "sce",
292-
"SCE targets (e.g. PS4)"),
293-
clEnumValEnd));
283+
"SCE targets (e.g. PS4)")));
294284

295285
// Common utility function tightly tied to the options listed here. Initializes
296286
// a TargetOptions object with CodeGen flags and returns it.

Diff for: ‎include/llvm/MC/MCTargetOptionsCommandFlags.h

+1-2
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,7 @@ cl::opt<MCTargetOptions::AsmInstrumentation> AsmInstrumentation(
2626
cl::values(clEnumValN(MCTargetOptions::AsmInstrumentationNone, "none",
2727
"no instrumentation at all"),
2828
clEnumValN(MCTargetOptions::AsmInstrumentationAddress, "address",
29-
"instrument instructions with memory arguments"),
30-
clEnumValEnd));
29+
"instrument instructions with memory arguments")));
3130

3231
cl::opt<bool> RelaxAll("mc-relax-all",
3332
cl::desc("When used with filetype=obj, "

Diff for: ‎include/llvm/Support/CommandLine.h

+24-33
Original file line numberDiff line numberDiff line change
@@ -556,52 +556,43 @@ struct OptionValue<std::string> final : OptionValueCopy<std::string> {
556556
//===----------------------------------------------------------------------===//
557557
// Enum valued command line option
558558
//
559-
#define clEnumVal(ENUMVAL, DESC) #ENUMVAL, int(ENUMVAL), DESC
560-
#define clEnumValN(ENUMVAL, FLAGNAME, DESC) FLAGNAME, int(ENUMVAL), DESC
561-
#define clEnumValEnd (reinterpret_cast<void *>(0))
559+
560+
// This represents a single enum value, using "int" as the underlying type.
561+
struct OptionEnumValue {
562+
StringRef Name;
563+
int Value;
564+
StringRef Description;
565+
};
566+
567+
#define clEnumVal(ENUMVAL, DESC) \
568+
cl::OptionEnumValue { #ENUMVAL, int(ENUMVAL), DESC }
569+
#define clEnumValN(ENUMVAL, FLAGNAME, DESC) \
570+
cl::OptionEnumValue { FLAGNAME, int(ENUMVAL), DESC }
562571

563572
// values - For custom data types, allow specifying a group of values together
564-
// as the values that go into the mapping that the option handler uses. Note
565-
// that the values list must always have a 0 at the end of the list to indicate
566-
// that the list has ended.
573+
// as the values that go into the mapping that the option handler uses.
567574
//
568-
template <class DataType> class ValuesClass {
575+
class ValuesClass {
569576
// Use a vector instead of a map, because the lists should be short,
570577
// the overhead is less, and most importantly, it keeps them in the order
571578
// inserted so we can print our option out nicely.
572-
SmallVector<std::pair<StringRef , std::pair<int, StringRef >>, 4> Values;
573-
void processValues(va_list Vals);
579+
SmallVector<OptionEnumValue, 4> Values;
574580

575581
public:
576-
ValuesClass(StringRef EnumName, DataType Val, StringRef Desc,
577-
va_list ValueArgs) {
578-
// Insert the first value, which is required.
579-
Values.push_back(std::make_pair(EnumName, std::make_pair(Val, Desc)));
580-
581-
// Process the varargs portion of the values...
582-
while (const char *enumName = va_arg(ValueArgs, const char * )) {
583-
DataType EnumVal = static_cast<DataType>(va_arg(ValueArgs, int));
584-
auto EnumDesc = StringRef(va_arg(ValueArgs, const char * ));
585-
Values.push_back(std::make_pair(StringRef(enumName), // Add value to value map
586-
std::make_pair(EnumVal, EnumDesc)));
587-
}
588-
}
582+
ValuesClass(std::initializer_list<OptionEnumValue> Options)
583+
: Values(Options) {}
589584

590585
template <class Opt> void apply(Opt &O) const {
591-
for (size_t i = 0, e = Values.size(); i != e; ++i)
592-
O.getParser().addLiteralOption(Values[i].first, Values[i].second.first,
593-
Values[i].second.second);
586+
for (auto Value : Values)
587+
O.getParser().addLiteralOption(Value.Name, Value.Value,
588+
Value.Description);
594589
}
595590
};
596591

597-
template <class DataType>
598-
ValuesClass<DataType> LLVM_END_WITH_NULL
599-
values(StringRef Arg, DataType Val, StringRef Desc, ...) {
600-
va_list ValueArgs;
601-
va_start(ValueArgs, Desc);
602-
ValuesClass<DataType> Vals(Arg, Val, Desc, ValueArgs);
603-
va_end(ValueArgs);
604-
return Vals;
592+
/// Helper to build a ValuesClass by forwarding a variable number of arguments
593+
/// as an initializer list to the ValuesClass constructor.
594+
template <typename... OptsTy> ValuesClass values(OptsTy... Options) {
595+
return ValuesClass({Options...});
605596
}
606597

607598
//===----------------------------------------------------------------------===//

Diff for: ‎lib/Analysis/BlockFrequencyInfo.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,7 @@ static cl::opt<GVDAGType> ViewBlockFreqPropagationDAG(
3939
"display a graph using the raw "
4040
"integer fractional block frequency representation."),
4141
clEnumValN(GVDT_Count, "count", "display a graph using the real "
42-
"profile count if available."),
43-
clEnumValEnd));
42+
"profile count if available.")));
4443

4544
cl::opt<std::string>
4645
ViewBlockFreqFuncName("view-bfi-func-name", cl::Hidden,

Diff for: ‎lib/Analysis/RegionInfo.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,7 @@ static cl::opt<Region::PrintStyle, true> printStyleX("print-region-style",
5454
clEnumValN(Region::PrintBB, "bb",
5555
"print regions in detail with block_iterator"),
5656
clEnumValN(Region::PrintRN, "rn",
57-
"print regions in detail with element_iterator"),
58-
clEnumValEnd));
57+
"print regions in detail with element_iterator")));
5958

6059

6160
//===----------------------------------------------------------------------===//

Diff for: ‎lib/Analysis/TargetLibraryInfo.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,7 @@ static cl::opt<TargetLibraryInfoImpl::VectorLibrary> ClVectorLibrary(
2424
clEnumValN(TargetLibraryInfoImpl::Accelerate, "Accelerate",
2525
"Accelerate framework"),
2626
clEnumValN(TargetLibraryInfoImpl::SVML, "SVML",
27-
"Intel SVML library"),
28-
clEnumValEnd));
27+
"Intel SVML library")));
2928

3029
StringRef const TargetLibraryInfoImpl::StandardNames[LibFunc::NumLibFuncs] = {
3130
#define TLI_DEFINE_STRING

Diff for: ‎lib/CodeGen/AsmPrinter/DwarfDebug.cpp

+4-5
Original file line numberDiff line numberDiff line change
@@ -86,23 +86,23 @@ DwarfAccelTables("dwarf-accel-tables", cl::Hidden,
8686
cl::desc("Output prototype dwarf accelerator tables."),
8787
cl::values(clEnumVal(Default, "Default for platform"),
8888
clEnumVal(Enable, "Enabled"),
89-
clEnumVal(Disable, "Disabled"), clEnumValEnd),
89+
clEnumVal(Disable, "Disabled")),
9090
cl::init(Default));
9191

9292
static cl::opt<DefaultOnOff>
9393
SplitDwarf("split-dwarf", cl::Hidden,
9494
cl::desc("Output DWARF5 split debug info."),
9595
cl::values(clEnumVal(Default, "Default for platform"),
9696
clEnumVal(Enable, "Enabled"),
97-
clEnumVal(Disable, "Disabled"), clEnumValEnd),
97+
clEnumVal(Disable, "Disabled")),
9898
cl::init(Default));
9999

100100
static cl::opt<DefaultOnOff>
101101
DwarfPubSections("generate-dwarf-pub-sections", cl::Hidden,
102102
cl::desc("Generate DWARF pubnames and pubtypes sections"),
103103
cl::values(clEnumVal(Default, "Default for platform"),
104104
clEnumVal(Enable, "Enabled"),
105-
clEnumVal(Disable, "Disabled"), clEnumValEnd),
105+
clEnumVal(Disable, "Disabled")),
106106
cl::init(Default));
107107

108108
enum LinkageNameOption {
@@ -117,8 +117,7 @@ static cl::opt<LinkageNameOption>
117117
"Default for platform"),
118118
clEnumValN(AllLinkageNames, "All", "All"),
119119
clEnumValN(AbstractLinkageNames, "Abstract",
120-
"Abstract subprograms"),
121-
clEnumValEnd),
120+
"Abstract subprograms")),
122121
cl::init(DefaultLinkageNames));
123122

124123
static const char *const DWARFGroupName = "DWARF Emission";

Diff for: ‎lib/CodeGen/GlobalISel/RegBankSelect.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,7 @@ static cl::opt<RegBankSelect::Mode> RegBankSelectMode(
3333
cl::values(clEnumValN(RegBankSelect::Mode::Fast, "regbankselect-fast",
3434
"Run the Fast mode (default mapping)"),
3535
clEnumValN(RegBankSelect::Mode::Greedy, "regbankselect-greedy",
36-
"Use the Greedy mode (best local mapping)"),
37-
clEnumValEnd));
36+
"Use the Greedy mode (best local mapping)")));
3837

3938
char RegBankSelect::ID = 0;
4039
INITIALIZE_PASS_BEGIN(RegBankSelect, DEBUG_TYPE,

Diff for: ‎lib/CodeGen/MachineBlockFrequencyInfo.cpp

+1-3
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,7 @@ static cl::opt<GVDAGType> ViewMachineBlockFreqPropagationDAG(
4242
"display a graph using the raw "
4343
"integer fractional block frequency representation."),
4444
clEnumValN(GVDT_Count, "count", "display a graph using the real "
45-
"profile count if available."),
46-
47-
clEnumValEnd));
45+
"profile count if available.")));
4846

4947
extern cl::opt<std::string> ViewBlockFreqFuncName;
5048
extern cl::opt<unsigned> ViewHotFreqPercent;

Diff for: ‎lib/CodeGen/RegAllocGreedy.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,7 @@ static cl::opt<SplitEditor::ComplementSpillMode> SplitSpillMode(
6161
cl::desc("Spill mode for splitting live ranges"),
6262
cl::values(clEnumValN(SplitEditor::SM_Partition, "default", "Default"),
6363
clEnumValN(SplitEditor::SM_Size, "size", "Optimize for size"),
64-
clEnumValN(SplitEditor::SM_Speed, "speed", "Optimize for speed"),
65-
clEnumValEnd),
64+
clEnumValN(SplitEditor::SM_Speed, "speed", "Optimize for speed")),
6665
cl::init(SplitEditor::SM_Speed));
6766

6867
static cl::opt<unsigned>

Diff for: ‎lib/CodeGen/SafeStack.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,7 @@ static cl::opt<UnsafeStackPtrStorageVal> USPStorage("safe-stack-usp-storage",
6060
cl::values(clEnumValN(ThreadLocalUSP, "thread-local",
6161
"Thread-local storage"),
6262
clEnumValN(SingleThreadUSP, "single-thread",
63-
"Non-thread-local storage"),
64-
clEnumValEnd));
63+
"Non-thread-local storage")));
6564

6665
namespace llvm {
6766

Diff for: ‎lib/CodeGen/TargetPassConfig.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -129,8 +129,7 @@ static cl::opt<CFLAAType> UseCFLAA(
129129
clEnumValN(CFLAAType::Andersen, "anders",
130130
"Enable inclusion-based CFL-AA"),
131131
clEnumValN(CFLAAType::Both, "both",
132-
"Enable both variants of CFL-AA"),
133-
clEnumValEnd));
132+
"Enable both variants of CFL-AA")));
134133

135134
/// Allow standard passes to be disabled by command line options. This supports
136135
/// simple binary flags that either suppress the pass or do nothing.

Diff for: ‎lib/IR/LegacyPassManager.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,7 @@ PassDebugging("debug-pass", cl::Hidden,
5656
clEnumVal(Arguments , "print pass arguments to pass to 'opt'"),
5757
clEnumVal(Structure , "print pass structure before run()"),
5858
clEnumVal(Executions, "print pass name before it is executed"),
59-
clEnumVal(Details , "print pass details when it is executed"),
60-
clEnumValEnd));
59+
clEnumVal(Details , "print pass details when it is executed")));
6160

6261
namespace {
6362
typedef llvm::cl::list<const llvm::PassInfo *, bool, PassNameParser>

Diff for: ‎lib/Target/AArch64/MCTargetDesc/AArch64MCAsmInfo.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,7 @@ static cl::opt<AsmWriterVariantTy> AsmWriterVariant(
2929
"aarch64-neon-syntax", cl::init(Default),
3030
cl::desc("Choose style of NEON code to emit from AArch64 backend:"),
3131
cl::values(clEnumValN(Generic, "generic", "Emit generic NEON assembly"),
32-
clEnumValN(Apple, "apple", "Emit Apple-style NEON assembly"),
33-
clEnumValEnd));
32+
clEnumValN(Apple, "apple", "Emit Apple-style NEON assembly")));
3433

3534
AArch64MCAsmInfoDarwin::AArch64MCAsmInfoDarwin() {
3635
// We prefer NEON instructions to be printed in the short form.

Diff for: ‎lib/Target/ARM/ARMSubtarget.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,7 @@ IT(cl::desc("IT block support"), cl::Hidden, cl::init(DefaultIT),
5959
clEnumValN(RestrictedIT, "arm-restrict-it",
6060
"Disallow deprecated IT based on ARMv8"),
6161
clEnumValN(NoRestrictedIT, "arm-no-restrict-it",
62-
"Allow IT blocks based on ARMv7"),
63-
clEnumValEnd));
62+
"Allow IT blocks based on ARMv7")));
6463

6564
/// ForceFastISel - Use the fast-isel, even for subtargets where it is not
6665
/// currently supported (for testing only).

0 commit comments

Comments
 (0)