Skip to content

Commit 732afdd

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 llvm-svn: 283671
1 parent 30cbd1a commit 732afdd

File tree

47 files changed

+106
-181
lines changed

Some content is hidden

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

47 files changed

+106
-181
lines changed

‎clang-tools-extra/include-fixer/tool/ClangIncludeFixer.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,7 @@ enum DatabaseFormatTy {
9090
cl::opt<DatabaseFormatTy> DatabaseFormat(
9191
"db", cl::desc("Specify input format"),
9292
cl::values(clEnumVal(fixed, "Hard-coded mapping"),
93-
clEnumVal(yaml, "Yaml database created by find-all-symbols"),
94-
clEnumValEnd),
93+
clEnumVal(yaml, "Yaml database created by find-all-symbols")),
9594
cl::init(yaml), cl::cat(IncludeFixerCategory));
9695

9796
cl::opt<std::string> Input("input",

‎clang/tools/c-index-test/core_main.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,7 @@ static cl::opt<ActionType>
4141
Action(cl::desc("Action:"), cl::init(ActionType::None),
4242
cl::values(
4343
clEnumValN(ActionType::PrintSourceSymbols,
44-
"print-source-symbols", "Print symbols from source"),
45-
clEnumValEnd),
44+
"print-source-symbols", "Print symbols from source")),
4645
cl::cat(IndexTestCoreCategory));
4746

4847
static cl::extrahelp MoreHelp(

‎clang/utils/TableGen/TableGen.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,7 @@ cl::opt<ActionType> Action(
135135
clEnumValN(GenAttrDocs, "gen-attr-docs",
136136
"Generate attribute documentation"),
137137
clEnumValN(GenDiagDocs, "gen-diag-docs",
138-
"Generate attribute documentation"),
139-
clEnumValEnd));
138+
"Generate attribute documentation")));
140139

141140
cl::opt<std::string>
142141
ClangComponent("clang-component",

‎llvm/docs/CommandLine.rst

Lines changed: 5 additions & 10 deletions
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:

‎llvm/include/llvm/CodeGen/CommandFlags.h

Lines changed: 11 additions & 21 deletions
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.

‎llvm/include/llvm/MC/MCTargetOptionsCommandFlags.h

Lines changed: 1 addition & 2 deletions
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, "

‎llvm/include/llvm/Support/CommandLine.h

Lines changed: 24 additions & 33 deletions
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
//===----------------------------------------------------------------------===//

‎llvm/lib/Analysis/BlockFrequencyInfo.cpp

Lines changed: 1 addition & 2 deletions
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,

‎llvm/lib/Analysis/RegionInfo.cpp

Lines changed: 1 addition & 2 deletions
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
//===----------------------------------------------------------------------===//

‎llvm/lib/Analysis/TargetLibraryInfo.cpp

Lines changed: 1 addition & 2 deletions
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

0 commit comments

Comments
 (0)