[PATCH] D83071: Add support for options with two flags for controlling the same field.
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2f3055c543f8: [clang][cli] Add support for options with two
flags for controlling the same… (authored by jansvoboda11).
Changed prior to commit:
https://reviews.llvm.org/D83071?vs=304491=305428#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83071/new/
https://reviews.llvm.org/D83071
Files:
clang/include/clang/Driver/Options.td
clang/lib/Frontend/CompilerInvocation.cpp
llvm/include/llvm/Option/OptParser.td
llvm/utils/TableGen/OptParserEmitter.cpp
Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -63,8 +63,9 @@
class MarshallingInfo {
public:
- static constexpr const char *MacroName = "OPTION_WITH_MARSHALLING";
+ using Ptr = std::unique_ptr;
+ const char *MacroName;
const Record
bool ShouldAlwaysEmit;
StringRef KeyPath;
@@ -80,6 +81,8 @@
std::vector NormalizedValues;
std::string ValueTableName;
+ static size_t NextTableIndex;
+
static constexpr const char *ValueTablePreamble = R"(
struct SimpleEnumValue {
const char *Name;
@@ -95,7 +98,14 @@
static constexpr const char *ValueTablesDecl =
"static const SimpleEnumValueTable SimpleEnumValueTables[] = ";
- void emit(raw_ostream ) const {
+ MarshallingInfo(const Record )
+ : MacroName("OPTION_WITH_MARSHALLING"), R(R) {}
+ MarshallingInfo(const char *MacroName, const Record )
+ : MacroName(MacroName), R(R){};
+
+ virtual ~MarshallingInfo() = default;
+
+ virtual void emit(raw_ostream ) const {
write_cstring(OS, StringRef(getOptionSpelling(R)));
OS << ", ";
OS << ShouldAlwaysEmit;
@@ -133,53 +143,6 @@
return StringRef(ValueTableName);
}
- static MarshallingInfo create(const Record ) {
-assert(!isa(R.getValueInit("KeyPath")) &&
- !isa(R.getValueInit("DefaultValue")) &&
- !isa(R.getValueInit("NormalizerRetTy")) &&
- !isa(R.getValueInit("ValueMerger")) &&
- "MarshallingInfo must have a type");
-
-MarshallingInfo Ret(R);
-Ret.ShouldAlwaysEmit = R.getValueAsBit("ShouldAlwaysEmit");
-Ret.KeyPath = R.getValueAsString("KeyPath");
-Ret.DefaultValue = R.getValueAsString("DefaultValue");
-Ret.NormalizedValuesScope = R.getValueAsString("NormalizedValuesScope");
-Ret.NormalizerRetTy = R.getValueAsString("NormalizerRetTy");
-
-Ret.Normalizer = R.getValueAsString("Normalizer");
-Ret.Denormalizer = R.getValueAsString("Denormalizer");
-Ret.ValueMerger = R.getValueAsString("ValueMerger");
-Ret.ValueExtractor = R.getValueAsString("ValueExtractor");
-
-if (!isa(R.getValueInit("NormalizedValues"))) {
- assert(!isa(R.getValueInit("Values")) &&
- "Cannot provide normalized values for value-less options");
- Ret.TableIndex = NextTableIndex++;
- Ret.NormalizedValues = R.getValueAsListOfStrings("NormalizedValues");
- Ret.Values.reserve(Ret.NormalizedValues.size());
- Ret.ValueTableName = getOptionName(R) + "ValueTable";
-
- StringRef ValuesStr = R.getValueAsString("Values");
- for (;;) {
-size_t Idx = ValuesStr.find(',');
-if (Idx == StringRef::npos)
- break;
-if (Idx > 0)
- Ret.Values.push_back(ValuesStr.slice(0, Idx));
-ValuesStr = ValuesStr.slice(Idx + 1, StringRef::npos);
- }
- if (!ValuesStr.empty())
-Ret.Values.push_back(ValuesStr);
-
- assert(Ret.Values.size() == Ret.NormalizedValues.size() &&
- "The number of normalized values doesn't match the number of "
- "values");
-}
-
-return Ret;
- }
-
private:
void emitScopedNormalizedValue(raw_ostream ,
StringRef NormalizedValue) const {
@@ -187,13 +150,85 @@
OS << NormalizedValuesScope << "::";
OS << NormalizedValue;
}
+};
+
+size_t MarshallingInfo::NextTableIndex = 0;
- MarshallingInfo(const Record ) : R(R){};
+class MarshallingInfoBooleanFlag : public MarshallingInfo {
+public:
+ const Record
- static size_t NextTableIndex;
+ MarshallingInfoBooleanFlag(const Record , const Record )
+ : MarshallingInfo("OPTION_WITH_MARSHALLING_BOOLEAN", Option),
+NegOption(NegOption) {}
+
+ void emit(raw_ostream ) const override {
+MarshallingInfo::emit(OS);
+OS << ", ";
+OS << getOptionName(NegOption);
+OS << ", ";
+write_cstring(OS, getOptionSpelling(NegOption));
+ }
};
-size_t MarshallingInfo::NextTableIndex = 0;
+static MarshallingInfo::Ptr createMarshallingInfo(const Record ) {
+ assert(!isa(R.getValueInit("KeyPath")) &&
+ !isa(R.getValueInit("DefaultValue")) &&
+ !isa(R.getValueInit("NormalizerRetTy")) &&
+
[PATCH] D83071: Add support for options with two flags for controlling the same field.
jansvoboda11 updated this revision to Diff 304491.
jansvoboda11 added a comment.
Apply unique_ptr workaround for older Clang versions
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83071/new/
https://reviews.llvm.org/D83071
Files:
clang/include/clang/Driver/Options.td
clang/lib/Frontend/CompilerInvocation.cpp
llvm/include/llvm/Option/OptParser.td
llvm/utils/TableGen/OptParserEmitter.cpp
Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -63,8 +63,9 @@
class MarshallingInfo {
public:
- static constexpr const char *MacroName = "OPTION_WITH_MARSHALLING";
+ using Ptr = std::unique_ptr;
+ const char *MacroName;
const Record
bool ShouldAlwaysEmit;
StringRef KeyPath;
@@ -80,6 +81,8 @@
std::vector NormalizedValues;
std::string ValueTableName;
+ static size_t NextTableIndex;
+
static constexpr const char *ValueTablePreamble = R"(
struct SimpleEnumValue {
const char *Name;
@@ -95,7 +98,14 @@
static constexpr const char *ValueTablesDecl =
"static const SimpleEnumValueTable SimpleEnumValueTables[] = ";
- void emit(raw_ostream ) const {
+ MarshallingInfo(const Record )
+ : MacroName("OPTION_WITH_MARSHALLING"), R(R) {}
+ MarshallingInfo(const char *MacroName, const Record )
+ : MacroName(MacroName), R(R){};
+
+ virtual ~MarshallingInfo() = default;
+
+ virtual void emit(raw_ostream ) const {
write_cstring(OS, StringRef(getOptionSpelling(R)));
OS << ", ";
OS << ShouldAlwaysEmit;
@@ -133,53 +143,6 @@
return StringRef(ValueTableName);
}
- static MarshallingInfo create(const Record ) {
-assert(!isa(R.getValueInit("KeyPath")) &&
- !isa(R.getValueInit("DefaultValue")) &&
- !isa(R.getValueInit("NormalizerRetTy")) &&
- !isa(R.getValueInit("ValueMerger")) &&
- "MarshallingInfo must have a type");
-
-MarshallingInfo Ret(R);
-Ret.ShouldAlwaysEmit = R.getValueAsBit("ShouldAlwaysEmit");
-Ret.KeyPath = R.getValueAsString("KeyPath");
-Ret.DefaultValue = R.getValueAsString("DefaultValue");
-Ret.NormalizedValuesScope = R.getValueAsString("NormalizedValuesScope");
-Ret.NormalizerRetTy = R.getValueAsString("NormalizerRetTy");
-
-Ret.Normalizer = R.getValueAsString("Normalizer");
-Ret.Denormalizer = R.getValueAsString("Denormalizer");
-Ret.ValueMerger = R.getValueAsString("ValueMerger");
-Ret.ValueExtractor = R.getValueAsString("ValueExtractor");
-
-if (!isa(R.getValueInit("NormalizedValues"))) {
- assert(!isa(R.getValueInit("Values")) &&
- "Cannot provide normalized values for value-less options");
- Ret.TableIndex = NextTableIndex++;
- Ret.NormalizedValues = R.getValueAsListOfStrings("NormalizedValues");
- Ret.Values.reserve(Ret.NormalizedValues.size());
- Ret.ValueTableName = getOptionName(R) + "ValueTable";
-
- StringRef ValuesStr = R.getValueAsString("Values");
- for (;;) {
-size_t Idx = ValuesStr.find(',');
-if (Idx == StringRef::npos)
- break;
-if (Idx > 0)
- Ret.Values.push_back(ValuesStr.slice(0, Idx));
-ValuesStr = ValuesStr.slice(Idx + 1, StringRef::npos);
- }
- if (!ValuesStr.empty())
-Ret.Values.push_back(ValuesStr);
-
- assert(Ret.Values.size() == Ret.NormalizedValues.size() &&
- "The number of normalized values doesn't match the number of "
- "values");
-}
-
-return Ret;
- }
-
private:
void emitScopedNormalizedValue(raw_ostream ,
StringRef NormalizedValue) const {
@@ -187,13 +150,85 @@
OS << NormalizedValuesScope << "::";
OS << NormalizedValue;
}
+};
+
+size_t MarshallingInfo::NextTableIndex = 0;
- MarshallingInfo(const Record ) : R(R){};
+class MarshallingInfoBooleanFlag : public MarshallingInfo {
+public:
+ const Record
- static size_t NextTableIndex;
+ MarshallingInfoBooleanFlag(const Record , const Record )
+ : MarshallingInfo("OPTION_WITH_MARSHALLING_BOOLEAN", Option),
+NegOption(NegOption) {}
+
+ void emit(raw_ostream ) const override {
+MarshallingInfo::emit(OS);
+OS << ", ";
+OS << getOptionName(NegOption);
+OS << ", ";
+write_cstring(OS, getOptionSpelling(NegOption));
+ }
};
-size_t MarshallingInfo::NextTableIndex = 0;
+static MarshallingInfo::Ptr createMarshallingInfo(const Record ) {
+ assert(!isa(R.getValueInit("KeyPath")) &&
+ !isa(R.getValueInit("DefaultValue")) &&
+ !isa(R.getValueInit("NormalizerRetTy")) &&
+ !isa(R.getValueInit("ValueMerger")) &&
+ "MarshallingInfo must have a type");
+
+ MarshallingInfo::Ptr Ret;
+ StringRef KindString = R.getValueAsString("MarshallingInfoKind");
+ if (KindString == "Default") {
+Ret =
[PATCH] D83071: Add support for options with two flags for controlling the same field.
jansvoboda11 updated this revision to Diff 304487.
jansvoboda11 added a comment.
Rebase on top of recent changes.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83071/new/
https://reviews.llvm.org/D83071
Files:
clang/include/clang/Driver/Options.td
clang/lib/Frontend/CompilerInvocation.cpp
llvm/include/llvm/Option/OptParser.td
llvm/utils/TableGen/OptParserEmitter.cpp
Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -63,8 +63,9 @@
class MarshallingInfo {
public:
- static constexpr const char *MacroName = "OPTION_WITH_MARSHALLING";
+ using Ptr = std::unique_ptr;
+ const char *MacroName;
const Record
bool ShouldAlwaysEmit;
StringRef KeyPath;
@@ -80,6 +81,8 @@
std::vector NormalizedValues;
std::string ValueTableName;
+ static size_t NextTableIndex;
+
static constexpr const char *ValueTablePreamble = R"(
struct SimpleEnumValue {
const char *Name;
@@ -95,7 +98,14 @@
static constexpr const char *ValueTablesDecl =
"static const SimpleEnumValueTable SimpleEnumValueTables[] = ";
- void emit(raw_ostream ) const {
+ MarshallingInfo(const Record )
+ : MacroName("OPTION_WITH_MARSHALLING"), R(R) {}
+ MarshallingInfo(const char *MacroName, const Record )
+ : MacroName(MacroName), R(R){};
+
+ virtual ~MarshallingInfo() = default;
+
+ virtual void emit(raw_ostream ) const {
write_cstring(OS, StringRef(getOptionSpelling(R)));
OS << ", ";
OS << ShouldAlwaysEmit;
@@ -133,53 +143,6 @@
return StringRef(ValueTableName);
}
- static MarshallingInfo create(const Record ) {
-assert(!isa(R.getValueInit("KeyPath")) &&
- !isa(R.getValueInit("DefaultValue")) &&
- !isa(R.getValueInit("NormalizerRetTy")) &&
- !isa(R.getValueInit("ValueMerger")) &&
- "MarshallingInfo must have a type");
-
-MarshallingInfo Ret(R);
-Ret.ShouldAlwaysEmit = R.getValueAsBit("ShouldAlwaysEmit");
-Ret.KeyPath = R.getValueAsString("KeyPath");
-Ret.DefaultValue = R.getValueAsString("DefaultValue");
-Ret.NormalizedValuesScope = R.getValueAsString("NormalizedValuesScope");
-Ret.NormalizerRetTy = R.getValueAsString("NormalizerRetTy");
-
-Ret.Normalizer = R.getValueAsString("Normalizer");
-Ret.Denormalizer = R.getValueAsString("Denormalizer");
-Ret.ValueMerger = R.getValueAsString("ValueMerger");
-Ret.ValueExtractor = R.getValueAsString("ValueExtractor");
-
-if (!isa(R.getValueInit("NormalizedValues"))) {
- assert(!isa(R.getValueInit("Values")) &&
- "Cannot provide normalized values for value-less options");
- Ret.TableIndex = NextTableIndex++;
- Ret.NormalizedValues = R.getValueAsListOfStrings("NormalizedValues");
- Ret.Values.reserve(Ret.NormalizedValues.size());
- Ret.ValueTableName = getOptionName(R) + "ValueTable";
-
- StringRef ValuesStr = R.getValueAsString("Values");
- for (;;) {
-size_t Idx = ValuesStr.find(',');
-if (Idx == StringRef::npos)
- break;
-if (Idx > 0)
- Ret.Values.push_back(ValuesStr.slice(0, Idx));
-ValuesStr = ValuesStr.slice(Idx + 1, StringRef::npos);
- }
- if (!ValuesStr.empty())
-Ret.Values.push_back(ValuesStr);
-
- assert(Ret.Values.size() == Ret.NormalizedValues.size() &&
- "The number of normalized values doesn't match the number of "
- "values");
-}
-
-return Ret;
- }
-
private:
void emitScopedNormalizedValue(raw_ostream ,
StringRef NormalizedValue) const {
@@ -187,13 +150,81 @@
OS << NormalizedValuesScope << "::";
OS << NormalizedValue;
}
+};
+
+size_t MarshallingInfo::NextTableIndex = 0;
- MarshallingInfo(const Record ) : R(R){};
+class MarshallingInfoBooleanFlag : public MarshallingInfo {
+public:
+ const Record
- static size_t NextTableIndex;
+ MarshallingInfoBooleanFlag(const Record , const Record )
+ : MarshallingInfo("OPTION_WITH_MARSHALLING_BOOLEAN", Option),
+NegOption(NegOption) {}
+
+ void emit(raw_ostream ) const override {
+MarshallingInfo::emit(OS);
+OS << ", ";
+OS << getOptionName(NegOption);
+OS << ", ";
+write_cstring(OS, getOptionSpelling(NegOption));
+ }
};
-size_t MarshallingInfo::NextTableIndex = 0;
+static MarshallingInfo::Ptr createMarshallingInfo(const Record ) {
+ assert(!isa(R.getValueInit("KeyPath")) &&
+ !isa(R.getValueInit("DefaultValue")) &&
+ !isa(R.getValueInit("NormalizerRetTy")) &&
+ !isa(R.getValueInit("ValueMerger")) &&
+ "MarshallingInfo must have a type");
+
+ MarshallingInfo::Ptr Ret;
+ StringRef KindString = R.getValueAsString("MarshallingInfoKind");
+ if (KindString == "Default") {
+Ret = std::make_unique(R);
+ } else if
[PATCH] D83071: Add support for options with two flags for controlling the same field.
jansvoboda11 commandeered this revision. jansvoboda11 added a reviewer: dang. jansvoboda11 added a comment. Taking control of this revision, as Daniel is no longer involved. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83071/new/ https://reviews.llvm.org/D83071 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D83071: Add support for options with two flags for controlling the same field.
dang updated this revision to Diff 280945.
dang added a comment.
Update with latest changes
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83071/new/
https://reviews.llvm.org/D83071
Files:
clang/include/clang/Driver/Options.td
clang/lib/Frontend/CompilerInvocation.cpp
llvm/include/llvm/Option/OptParser.td
llvm/utils/TableGen/OptParserEmitter.cpp
Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -61,8 +61,9 @@
class MarshallingInfo {
public:
- static constexpr const char *MacroName = "OPTION_WITH_MARSHALLING";
+ using Ptr = std::unique_ptr;
+ const char *MacroName;
const Record
bool ShouldAlwaysEmit;
StringRef KeyPath;
@@ -78,6 +79,8 @@
std::vector NormalizedValues;
std::string ValueTableName;
+ static size_t NextTableIndex;
+
static constexpr const char *ValueTablePreamble = R"(
struct SimpleEnumValue {
const char *Name;
@@ -93,7 +96,14 @@
static constexpr const char *ValueTablesDecl =
"static const SimpleEnumValueTable SimpleEnumValueTables[] = ";
- void emit(raw_ostream ) const {
+ MarshallingInfo(const Record )
+ : MacroName("OPTION_WITH_MARSHALLING"), R(R) {}
+ MarshallingInfo(const char *MacroName, const Record )
+ : MacroName(MacroName), R(R){};
+
+ virtual ~MarshallingInfo() = default;
+
+ virtual void emit(raw_ostream ) const {
write_cstring(OS, StringRef(getOptionSpelling(R)));
OS << ", ";
OS << ShouldAlwaysEmit;
@@ -131,53 +141,6 @@
return StringRef(ValueTableName);
}
- static MarshallingInfo create(const Record ) {
-assert(!isa(R.getValueInit("KeyPath")) &&
- !isa(R.getValueInit("DefaultValue")) &&
- !isa(R.getValueInit("NormalizerRetTy")) &&
- !isa(R.getValueInit("ValueMerger")) &&
- "MarshallingInfo must have a type");
-
-MarshallingInfo Ret(R);
-Ret.ShouldAlwaysEmit = R.getValueAsBit("ShouldAlwaysEmit");
-Ret.KeyPath = R.getValueAsString("KeyPath");
-Ret.DefaultValue = R.getValueAsString("DefaultValue");
-Ret.NormalizedValuesScope = R.getValueAsString("NormalizedValuesScope");
-Ret.NormalizerRetTy = R.getValueAsString("NormalizerRetTy");
-
-Ret.Normalizer = R.getValueAsString("Normalizer");
-Ret.Denormalizer = R.getValueAsString("Denormalizer");
-Ret.ValueMerger = R.getValueAsString("ValueMerger");
-Ret.ValueExtractor = R.getValueAsString("ValueExtractor");
-
-if (!isa(R.getValueInit("NormalizedValues"))) {
- assert(!isa(R.getValueInit("Values")) &&
- "Cannot provide normalized values for value-less options");
- Ret.TableIndex = NextTableIndex++;
- Ret.NormalizedValues = R.getValueAsListOfStrings("NormalizedValues");
- Ret.Values.reserve(Ret.NormalizedValues.size());
- Ret.ValueTableName = getOptionName(R) + "ValueTable";
-
- StringRef ValuesStr = R.getValueAsString("Values");
- for (;;) {
-size_t Idx = ValuesStr.find(',');
-if (Idx == StringRef::npos)
- break;
-if (Idx > 0)
- Ret.Values.push_back(ValuesStr.slice(0, Idx));
-ValuesStr = ValuesStr.slice(Idx + 1, StringRef::npos);
- }
- if (!ValuesStr.empty())
-Ret.Values.push_back(ValuesStr);
-
- assert(Ret.Values.size() == Ret.NormalizedValues.size() &&
- "The number of normalized values doesn't match the number of "
- "values");
-}
-
-return Ret;
- }
-
private:
void emitScopedNormalizedValue(raw_ostream ,
StringRef NormalizedValue) const {
@@ -185,13 +148,81 @@
OS << NormalizedValuesScope << "::";
OS << NormalizedValue;
}
+};
+
+size_t MarshallingInfo::NextTableIndex = 0;
- MarshallingInfo(const Record ) : R(R){};
+class MarshallingInfoBooleanFlag : public MarshallingInfo {
+public:
+ const Record
- static size_t NextTableIndex;
+ MarshallingInfoBooleanFlag(const Record , const Record )
+ : MarshallingInfo("OPTION_WITH_MARSHALLING_BOOLEAN", Option),
+NegOption(NegOption) {}
+
+ void emit(raw_ostream ) const override {
+MarshallingInfo::emit(OS);
+OS << ", ";
+OS << getOptionName(NegOption);
+OS << ", ";
+write_cstring(OS, getOptionSpelling(NegOption));
+ }
};
-size_t MarshallingInfo::NextTableIndex = 0;
+static MarshallingInfo::Ptr createMarshallingInfo(const Record ) {
+ assert(!isa(R.getValueInit("KeyPath")) &&
+ !isa(R.getValueInit("DefaultValue")) &&
+ !isa(R.getValueInit("NormalizerRetTy")) &&
+ !isa(R.getValueInit("ValueMerger")) &&
+ "MarshallingInfo must have a type");
+
+ MarshallingInfo::Ptr Ret;
+ StringRef KindString = R.getValueAsString("MarshallingInfoKind");
+ if (KindString == "Default") {
+Ret = std::make_unique(R);
+ } else if (KindString ==
[PATCH] D83071: Add support for options with two flags for controlling the same field.
dang updated this revision to Diff 276400.
dang added a comment.
Split into two macro kinds.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83071/new/
https://reviews.llvm.org/D83071
Files:
clang/include/clang/Driver/Options.td
clang/lib/Frontend/CompilerInvocation.cpp
llvm/include/llvm/Option/OptParser.td
llvm/utils/TableGen/OptParserEmitter.cpp
Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -61,8 +61,9 @@
class MarshallingInfo {
public:
- static constexpr const char *MacroName = "OPTION_WITH_MARSHALLING";
+ using Ptr = std::unique_ptr;
+ const char *MacroName;
const Record
bool ShouldAlwaysEmit;
StringRef KeyPath;
@@ -78,6 +79,8 @@
std::vector NormalizedValues;
std::string ValueTableName;
+ static size_t NextTableIndex;
+
static constexpr const char *ValueTablePreamble = R"(
struct SimpleEnumValue {
const char *Name;
@@ -93,7 +96,14 @@
static constexpr const char *ValueTablesDecl =
"static const SimpleEnumValueTable SimpleEnumValueTables[] = ";
- void emit(raw_ostream ) const {
+ MarshallingInfo(const Record )
+ : MacroName("OPTION_WITH_MARSHALLING"), R(R) {}
+ MarshallingInfo(const char *MacroName, const Record )
+ : MacroName(MacroName), R(R){};
+
+ virtual ~MarshallingInfo() = default;
+
+ virtual void emit(raw_ostream ) const {
write_cstring(OS, StringRef(getOptionSpelling(R)));
OS << ", ";
OS << ShouldAlwaysEmit;
@@ -131,53 +141,6 @@
return StringRef(ValueTableName);
}
- static MarshallingInfo create(const Record ) {
-assert(!isa(R.getValueInit("KeyPath")) &&
- !isa(R.getValueInit("DefaultValue")) &&
- !isa(R.getValueInit("NormalizerRetTy")) &&
- !isa(R.getValueInit("ValueMerger")) &&
- "MarshallingInfo must have a type");
-
-MarshallingInfo Ret(R);
-Ret.ShouldAlwaysEmit = R.getValueAsBit("ShouldAlwaysEmit");
-Ret.KeyPath = R.getValueAsString("KeyPath");
-Ret.DefaultValue = R.getValueAsString("DefaultValue");
-Ret.NormalizedValuesScope = R.getValueAsString("NormalizedValuesScope");
-Ret.NormalizerRetTy = R.getValueAsString("NormalizerRetTy");
-
-Ret.Normalizer = R.getValueAsString("Normalizer");
-Ret.Denormalizer = R.getValueAsString("Denormalizer");
-Ret.ValueMerger = R.getValueAsString("ValueMerger");
-Ret.ValueExtractor = R.getValueAsString("ValueExtractor");
-
-if (!isa(R.getValueInit("NormalizedValues"))) {
- assert(!isa(R.getValueInit("Values")) &&
- "Cannot provide normalized values for value-less options");
- Ret.TableIndex = NextTableIndex++;
- Ret.NormalizedValues = R.getValueAsListOfStrings("NormalizedValues");
- Ret.Values.reserve(Ret.NormalizedValues.size());
- Ret.ValueTableName = getOptionName(R) + "ValueTable";
-
- StringRef ValuesStr = R.getValueAsString("Values");
- for (;;) {
-size_t Idx = ValuesStr.find(',');
-if (Idx == StringRef::npos)
- break;
-if (Idx > 0)
- Ret.Values.push_back(ValuesStr.slice(0, Idx));
-ValuesStr = ValuesStr.slice(Idx + 1, StringRef::npos);
- }
- if (!ValuesStr.empty())
-Ret.Values.push_back(ValuesStr);
-
- assert(Ret.Values.size() == Ret.NormalizedValues.size() &&
- "The number of normalized values doesn't match the number of "
- "values");
-}
-
-return Ret;
- }
-
private:
void emitScopedNormalizedValue(raw_ostream ,
StringRef NormalizedValue) const {
@@ -185,13 +148,79 @@
OS << NormalizedValuesScope << "::";
OS << NormalizedValue;
}
+};
+
+size_t MarshallingInfo::NextTableIndex = 0;
- MarshallingInfo(const Record ) : R(R){};
+class MarshallingInfoBooleanFlag : public MarshallingInfo {
+public:
+ const Record
- static size_t NextTableIndex;
+ MarshallingInfoBooleanFlag(const Record , const Record )
+ : MarshallingInfo("OPTION_WITH_MARSHALLING_BOOLEAN", Option),
+NegOption(NegOption) {}
+
+ void emit(raw_ostream ) const override {
+MarshallingInfo::emit(OS);
+OS << ", ";
+OS << getOptionName(NegOption);
+OS << ", ";
+write_cstring(OS, getOptionSpelling(NegOption));
+ }
};
-size_t MarshallingInfo::NextTableIndex = 0;
+static MarshallingInfo::Ptr createMarshallingInfo(const Record ) {
+ assert(!isa(R.getValueInit("KeyPath")) &&
+ !isa(R.getValueInit("DefaultValue")) &&
+ !isa(R.getValueInit("NormalizerRetTy")) &&
+ !isa(R.getValueInit("ValueMerger")) &&
+ "MarshallingInfo must have a type");
+
+ MarshallingInfo::Ptr Ret;
+ if (Record *MaybeNegOption = R.getValueAsOptionalDef("NegOption")) {
+Ret = std::make_unique(R, *MaybeNegOption);
+ } else {
+Ret = std::make_unique(R);
+
[PATCH] D83071: Add support for options with two flags for controlling the same field.
dang updated this revision to Diff 276009.
dang added a comment.
Rebase on top of some changes to parent patches.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83071/new/
https://reviews.llvm.org/D83071
Files:
clang/include/clang/Driver/Options.td
clang/lib/Frontend/CompilerInvocation.cpp
llvm/include/llvm/Option/OptParser.td
Index: llvm/include/llvm/Option/OptParser.td
===
--- llvm/include/llvm/Option/OptParser.td
+++ llvm/include/llvm/Option/OptParser.td
@@ -156,6 +156,7 @@
: MarshallingInfo {
code NormalizerRetTy = ty;
code Normalizer = "normalizeSimpleFlag";
+ code Denormalizer = "denormalizeSimpleFlag";
}
class MarshallingInfoBitfieldFlag : MarshallingInfoFlag {
@@ -164,6 +165,20 @@
code ValueExtractor = "(extractMaskValue)";
}
+class MarshallingInfoBooleanTrueFlag
+ : MarshallingInfoFlag {
+ bit ShouldAlwaysEmit = 1;
+ code Normalizer = "normalizeBooleanTrueFlag";
+ code Denormalizer = "denormalizeBooleanFlag";
+}
+
+class MarshallingInfoBooleanFalseFlag
+ : MarshallingInfoFlag {
+ bit ShouldAlwaysEmit = 1;
+ code Normalizer = "normalizeBooleanFalseFlag";
+ code Denormalizer = "denormalizeBooleanFlag";
+}
+
// Mixins for additional marshalling attributes.
class IsNegative {
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -138,6 +138,13 @@
return !Args.hasArg(Opt);
}
+void denormalizeSimpleFlag(SmallVectorImpl ,
+ const char *Spelling,
+ CompilerInvocation::StringAllocator SA,
+ unsigned TableIndex, unsigned Value) {
+ Args.push_back(Spelling);
+}
+
template
static llvm::Optional
normalizeFlagToValue(OptSpecifier Opt, unsigned TableIndex, const ArgList ,
@@ -147,6 +154,33 @@
return None;
}
+template
+static Optional
+normalizeBooleanTrueFlag(OptSpecifier PosOpt, unsigned TableIndex,
+ const ArgList , DiagnosticsEngine ) {
+ if (const Arg *A = Args.getLastArg(PosOpt, NegOpt))
+return A->getOption().matches(PosOpt);
+ return None;
+}
+
+template
+static Optional
+normalizeBooleanFalseFlag(OptSpecifier NegOpt, unsigned TableIndex,
+ const ArgList , DiagnosticsEngine ) {
+ if (const Arg *A = Args.getLastArg(PosOpt, NegOpt))
+return A->getOption().matches(PosOpt);
+ return None;
+}
+
+template
+static void denormalizeBooleanFlag(SmallVectorImpl ,
+ const char *Spelling,
+ CompilerInvocation::StringAllocator SA,
+ unsigned TableIndex, unsigned Value) {
+ if (Value == IsPositive)
+Args.push_back(Spelling);
+}
+
static llvm::Optional normalizeSimpleEnum(OptSpecifier Opt,
unsigned TableIndex,
const ArgList ,
@@ -169,12 +203,14 @@
}
static void denormalizeSimpleEnum(SmallVectorImpl ,
+ const char *Spelling,
CompilerInvocation::StringAllocator SA,
unsigned TableIndex, unsigned Value) {
assert(TableIndex < SimpleEnumValueTablesSize);
const SimpleEnumValueTable = SimpleEnumValueTables[TableIndex];
for (int I = 0, E = Table.Size; I != E; ++I) {
if (Value == Table.Table[I].Value) {
+ Args.push_back(Spelling);
Args.push_back(Table.Table[I].Name);
return;
}
@@ -185,8 +221,10 @@
}
static void denormalizeString(SmallVectorImpl ,
+ const char *Spelling,
CompilerInvocation::StringAllocator SA,
unsigned TableIndex, const std::string ) {
+ Args.push_back(Spelling);
Args.push_back(SA(Value));
}
@@ -781,10 +819,6 @@
}
}
- Opts.ExperimentalNewPassManager = Args.hasFlag(
- OPT_fexperimental_new_pass_manager, OPT_fno_experimental_new_pass_manager,
- /* Default */ ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER);
-
Opts.DebugPassManager =
Args.hasFlag(OPT_fdebug_pass_manager, OPT_fno_debug_pass_manager,
/* Default */ false);
@@ -3894,13 +3928,7 @@
TYPE, NORMALIZER, DENORMALIZER, MERGER, EXTRACTOR, TABLE_INDEX)\
if (((FLAGS)::CC1Option) && \
(ALWAYS_EMIT || EXTRACTOR(this->KEYPATH) != DEFAULT_VALUE)) {\
-if (Option::KIND##Class == Option::FlagClass) {\
- Args.push_back(SPELLING);\
-} \
-if (Option::KIND##Class ==
[PATCH] D83071: Add support for options with two flags for controlling the same field.
dang updated this revision to Diff 276007.
dang added a comment.
Make mergers use values directly instead of constant references
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83071/new/
https://reviews.llvm.org/D83071
Files:
clang/include/clang/Driver/Options.td
clang/lib/Frontend/CompilerInvocation.cpp
llvm/include/llvm/Option/OptParser.td
Index: llvm/include/llvm/Option/OptParser.td
===
--- llvm/include/llvm/Option/OptParser.td
+++ llvm/include/llvm/Option/OptParser.td
@@ -156,6 +156,7 @@
: MarshallingInfo {
code NormalizerRetTy = ty;
code Normalizer = "normalizeSimpleFlag";
+ code Denormalizer = "denormalizeSimpleFlag";
}
class MarshallingInfoBitfieldFlag : MarshallingInfoFlag {
@@ -164,6 +165,20 @@
code ValueExtractor = "(extractMaskValue)";
}
+class MarshallingInfoBooleanTrueFlag
+ : MarshallingInfoFlag {
+ bit ShouldAlwaysEmit = 1;
+ code Normalizer = "normalizeBooleanTrueFlag";
+ code Denormalizer = "denormalizeBooleanFlag";
+}
+
+class MarshallingInfoBooleanFalseFlag
+ : MarshallingInfoFlag {
+ bit ShouldAlwaysEmit = 1;
+ code Normalizer = "normalizeBooleanFalseFlag";
+ code Denormalizer = "denormalizeBooleanFlag";
+}
+
// Mixins for additional marshalling attributes.
class IsNegative {
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -138,6 +138,13 @@
return !Args.hasArg(Opt);
}
+void denormalizeSimpleFlag(SmallVectorImpl ,
+ const char *Spelling,
+ CompilerInvocation::StringAllocator SA,
+ unsigned TableIndex, unsigned Value) {
+ Args.push_back(Spelling);
+}
+
template
static llvm::Optional
normalizeFlagToValue(OptSpecifier Opt, unsigned TableIndex, const ArgList ,
@@ -147,6 +154,33 @@
return None;
}
+template
+static Optional
+normalizeBooleanTrueFlag(OptSpecifier PosOpt, unsigned TableIndex,
+ const ArgList , DiagnosticsEngine ) {
+ if (const Arg *A = Args.getLastArg(PosOpt, NegOpt))
+return A->getOption().matches(PosOpt);
+ return None;
+}
+
+template
+static Optional
+normalizeBooleanFalseFlag(OptSpecifier NegOpt, unsigned TableIndex,
+ const ArgList , DiagnosticsEngine ) {
+ if (const Arg *A = Args.getLastArg(PosOpt, NegOpt))
+return A->getOption().matches(PosOpt);
+ return None;
+}
+
+template
+static void denormalizeBooleanFlag(SmallVectorImpl ,
+ const char *Spelling,
+ CompilerInvocation::StringAllocator SA,
+ unsigned TableIndex, unsigned Value) {
+ if (Value == IsPositive)
+Args.push_back(Spelling);
+}
+
static llvm::Optional normalizeSimpleEnum(OptSpecifier Opt,
unsigned TableIndex,
const ArgList ,
@@ -169,12 +203,14 @@
}
static void denormalizeSimpleEnum(SmallVectorImpl ,
+ const char *Spelling,
CompilerInvocation::StringAllocator SA,
unsigned TableIndex, unsigned Value) {
assert(TableIndex < SimpleEnumValueTablesSize);
const SimpleEnumValueTable = SimpleEnumValueTables[TableIndex];
for (int I = 0, E = Table.Size; I != E; ++I) {
if (Value == Table.Table[I].Value) {
+ Args.push_back(Spelling);
Args.push_back(Table.Table[I].Name);
return;
}
@@ -185,8 +221,10 @@
}
static void denormalizeString(SmallVectorImpl ,
+ const char *Spelling,
CompilerInvocation::StringAllocator SA,
unsigned TableIndex, const std::string ) {
+ Args.push_back(Spelling);
Args.push_back(SA(Value));
}
@@ -200,12 +238,11 @@
}
template
-static T mergeForwardValue(T KeyPath, const U ) {
+static T mergeForwardValue(T KeyPath, U Value) {
return Value;
}
-template
-static T mergeMaskValue(T KeyPath, const U ) {
+template static T mergeMaskValue(T KeyPath, U Value) {
return KeyPath | Value;
}
@@ -782,10 +819,6 @@
}
}
- Opts.ExperimentalNewPassManager = Args.hasFlag(
- OPT_fexperimental_new_pass_manager, OPT_fno_experimental_new_pass_manager,
- /* Default */ ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER);
-
Opts.DebugPassManager =
Args.hasFlag(OPT_fdebug_pass_manager, OPT_fno_debug_pass_manager,
/* Default */ false);
@@ -3895,13 +3928,7 @@
TYPE, NORMALIZER, DENORMALIZER, MERGER, EXTRACTOR, TABLE_INDEX)\
if (((FLAGS)::CC1Option) && \
(ALWAYS_EMIT || EXTRACTOR(this->KEYPATH) !=
[PATCH] D83071: Add support for options with two flags for controlling the same field.
dang marked an inline comment as done.
dang added inline comments.
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3931-3932
if (((FLAGS)::CC1Option) &&
\
(ALWAYS_EMIT || EXTRACTOR(this->KEYPATH) != DEFAULT_VALUE)) {
\
-if (Option::KIND##Class == Option::FlagClass) {
\
- Args.push_back(SPELLING);
\
-}
\
-if (Option::KIND##Class == Option::SeparateClass) {
\
- Args.push_back(SPELLING);
\
- DENORMALIZER(Args, SA, TABLE_INDEX, EXTRACTOR(this->KEYPATH));
\
-}
\
+DENORMALIZER(Args, SPELLING, SA, TABLE_INDEX, EXTRACTOR(this->KEYPATH));
\
}
dang wrote:
> dexonsmith wrote:
> > I realize this commit doesn't introduce it, but it seems unfortunate to
> > call `EXTRACTOR` twice. Maybe in a follow-up or prep commit you can fix
> > that... maybe something like this?
> > ```
> > if ((FLAGS)::CC1Option) {
> > const auto = EXTRACTOR(this->KEYPATH);
> > if (ALWAYS_EMIT || Extracted != DEFAULT_VALUE)
> > DENORMALIZER(Args, SPELLING, SA, TABLE_INDEX,
> > EXTRACTOR(this->KEYPATH));
> > }
> > ```
> Yes I can do that of course. Although EXTRACTOR is meant to be very cheap and
> in most cases it expands to just `this->KEYPATH`
See D83211
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83071/new/
https://reviews.llvm.org/D83071
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D83071: Add support for options with two flags for controlling the same field.
dang updated this revision to Diff 275591.
dang marked an inline comment as done.
dang added a comment.
Address some code review feedback.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83071/new/
https://reviews.llvm.org/D83071
Files:
clang/include/clang/Driver/Options.td
clang/lib/Frontend/CompilerInvocation.cpp
llvm/include/llvm/Option/OptParser.td
Index: llvm/include/llvm/Option/OptParser.td
===
--- llvm/include/llvm/Option/OptParser.td
+++ llvm/include/llvm/Option/OptParser.td
@@ -156,6 +156,7 @@
: MarshallingInfo {
code NormalizerRetTy = ty;
code Normalizer = "normalizeSimpleFlag";
+ code Denormalizer = "denormalizeSimpleFlag";
}
class MarshallingInfoBitfieldFlag : MarshallingInfoFlag {
@@ -164,6 +165,20 @@
code ValueExtractor = "(extractMaskValue)";
}
+class MarshallingInfoBooleanTrueFlag
+ : MarshallingInfoFlag {
+ bit ShouldAlwaysEmit = 1;
+ code Normalizer = "normalizeBooleanTrueFlag";
+ code Denormalizer = "denormalizeBooleanFlag";
+}
+
+class MarshallingInfoBooleanFalseFlag
+ : MarshallingInfoFlag {
+ bit ShouldAlwaysEmit = 1;
+ code Normalizer = "normalizeBooleanFalseFlag";
+ code Denormalizer = "denormalizeBooleanFlag";
+}
+
// Mixins for additional marshalling attributes.
class IsNegative {
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -138,6 +138,13 @@
return !Args.hasArg(Opt);
}
+void denormalizeSimpleFlag(SmallVectorImpl ,
+ const char *Spelling,
+ CompilerInvocation::StringAllocator SA,
+ unsigned TableIndex, unsigned Value) {
+ Args.push_back(Spelling);
+}
+
template
static llvm::Optional
normalizeFlagToValue(OptSpecifier Opt, unsigned TableIndex, const ArgList ,
@@ -147,6 +154,33 @@
return None;
}
+template
+static Optional
+normalizeBooleanTrueFlag(OptSpecifier PosOpt, unsigned TableIndex,
+ const ArgList , DiagnosticsEngine ) {
+ if (const Arg *A = Args.getLastArg(PosOpt, NegOpt))
+return A->getOption().matches(PosOpt);
+ return None;
+}
+
+template
+static Optional
+normalizeBooleanFalseFlag(OptSpecifier NegOpt, unsigned TableIndex,
+ const ArgList , DiagnosticsEngine ) {
+ if (const Arg *A = Args.getLastArg(PosOpt, NegOpt))
+return A->getOption().matches(PosOpt);
+ return None;
+}
+
+template
+static void denormalizeBooleanFlag(SmallVectorImpl ,
+ const char *Spelling,
+ CompilerInvocation::StringAllocator SA,
+ unsigned TableIndex, unsigned Value) {
+ if (Value == IsPositive)
+Args.push_back(Spelling);
+}
+
static llvm::Optional normalizeSimpleEnum(OptSpecifier Opt,
unsigned TableIndex,
const ArgList ,
@@ -169,12 +203,14 @@
}
static void denormalizeSimpleEnum(SmallVectorImpl ,
+ const char *Spelling,
CompilerInvocation::StringAllocator SA,
unsigned TableIndex, unsigned Value) {
assert(TableIndex < SimpleEnumValueTablesSize);
const SimpleEnumValueTable = SimpleEnumValueTables[TableIndex];
for (int I = 0, E = Table.Size; I != E; ++I) {
if (Value == Table.Table[I].Value) {
+ Args.push_back(Spelling);
Args.push_back(Table.Table[I].Name);
return;
}
@@ -185,8 +221,10 @@
}
static void denormalizeString(SmallVectorImpl ,
+ const char *Spelling,
CompilerInvocation::StringAllocator SA,
unsigned TableIndex, const std::string ) {
+ Args.push_back(Spelling);
Args.push_back(SA(Value));
}
@@ -782,10 +820,6 @@
}
}
- Opts.ExperimentalNewPassManager = Args.hasFlag(
- OPT_fexperimental_new_pass_manager, OPT_fno_experimental_new_pass_manager,
- /* Default */ ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER);
-
Opts.DebugPassManager =
Args.hasFlag(OPT_fdebug_pass_manager, OPT_fno_debug_pass_manager,
/* Default */ false);
@@ -3895,13 +3929,7 @@
TYPE, NORMALIZER, DENORMALIZER, MERGER, EXTRACTOR, TABLE_INDEX)\
if (((FLAGS)::CC1Option) && \
(ALWAYS_EMIT || EXTRACTOR(this->KEYPATH) != DEFAULT_VALUE)) {\
-if (Option::KIND##Class == Option::FlagClass) {\
- Args.push_back(SPELLING);\
-} \
-if
[PATCH] D83071: Add support for options with two flags for controlling the same field.
dang marked 5 inline comments as done.
dang added inline comments.
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:150
-static llvm::Optional normalizeSimpleEnum(OptSpecifier Opt,
-unsigned TableIndex,
dexonsmith wrote:
> I'm not sure if removing the `llvm::` namespace is intentional here, but if
> so please do it in a separate NFC patch to avoid adding noise in this one.
Yes it was accidental sorry about that.
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:161-163
+ if (!Args.hasArg(PosOpt, NegOpt))
+return None;
+ return Args.hasFlag(PosOpt, NegOpt);
dexonsmith wrote:
> This should be:
> ```
> if (Arg *A = Args.getLastArg(PosOpt, NegOpt))
> return A->getOption().matches(PosOpt);
> return None;
> ```
> Note that `hasArg` and `hasFlag` both resolve to `getLastArg`, so calling
> them one after another would be unfortunate.
>
> ... but I'm not even sure where `NegOpt` is coming from here, that looks like
> it hasn't been passed in. I think you need to change the signature to
> something like this:
> ```
> static llvm::Optional
> normalizeSimpleFlag(OptSpecifier Opt,
> Optional NegOpt,
> unsigned TableIndex,
> const ArgList ,
> DiagnosticsEngine ) {
> // Handle case without a `no-*` flag.
> if (!NegOpt)
> return Args.hasArg(Opt);
>
> // Handle case with a `no-*` flag.
> return Args.hasFlagAsOptional(Opt, *NegOpt);
> }
> ```
>
> It's possible you'll need to split up `OPTION_WITH_MARSHALLING` into two
> disjoint lists of options:
> - The list of options that can't be negated.
> - The list of options that can be negated, calling a different macro that
> adds macro arguments for the `CANCEL_ID` and `CANCEL_SPELLING`. For the
> denormalizer you might also need `CANCEL_VALUE`.
> - Note: the negating options themselves wouldn't be visited in either list.
> - Note: the (de)normalizer APIs would ideally work naturally for something
> like `-farg=val1` vs. `-farg=val2` vs. `-fno-arg`.
`NegOpt` is passed in via the template parameter, the only weird bit about it
is that the option name (for example OPT_fno_experimental_pass_manager) is
constructed in tablegen by hardcoding the OPT_ prefix. What is currently there
supports the case where the positive option takes a value and the negative one
doesn't by using different normalizer/denormalizer pairs for the positive and
the negative option. The bad thing about the current setup is that both options
have identical normalizers, but I felt that was less bad than splitting the
list of options and using different macros.
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3931-3932
if (((FLAGS)::CC1Option) &&
\
(ALWAYS_EMIT || EXTRACTOR(this->KEYPATH) != DEFAULT_VALUE)) {
\
-if (Option::KIND##Class == Option::FlagClass) {
\
- Args.push_back(SPELLING);
\
-}
\
-if (Option::KIND##Class == Option::SeparateClass) {
\
- Args.push_back(SPELLING);
\
- DENORMALIZER(Args, SA, TABLE_INDEX, EXTRACTOR(this->KEYPATH));
\
-}
\
+DENORMALIZER(Args, SPELLING, SA, TABLE_INDEX, EXTRACTOR(this->KEYPATH));
\
}
dexonsmith wrote:
> I realize this commit doesn't introduce it, but it seems unfortunate to call
> `EXTRACTOR` twice. Maybe in a follow-up or prep commit you can fix that...
> maybe something like this?
> ```
> if ((FLAGS)::CC1Option) {
> const auto = EXTRACTOR(this->KEYPATH);
> if (ALWAYS_EMIT || Extracted != DEFAULT_VALUE)
> DENORMALIZER(Args, SPELLING, SA, TABLE_INDEX, EXTRACTOR(this->KEYPATH));
> }
> ```
Yes I can do that of course. Although EXTRACTOR is meant to be very cheap and
in most cases it expands to just `this->KEYPATH`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83071/new/
https://reviews.llvm.org/D83071
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D83071: Add support for options with two flags for controlling the same field.
dang updated this revision to Diff 275586.
dang added a comment.
Revert accidental namespace removal.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83071/new/
https://reviews.llvm.org/D83071
Files:
clang/include/clang/Driver/Options.td
clang/lib/Frontend/CompilerInvocation.cpp
llvm/include/llvm/Option/OptParser.td
Index: llvm/include/llvm/Option/OptParser.td
===
--- llvm/include/llvm/Option/OptParser.td
+++ llvm/include/llvm/Option/OptParser.td
@@ -156,6 +156,7 @@
: MarshallingInfo {
code NormalizerRetTy = ty;
code Normalizer = "normalizeSimpleFlag";
+ code Denormalizer = "denormalizeSimpleFlag";
}
class MarshallingInfoBitfieldFlag : MarshallingInfoFlag {
@@ -164,6 +165,20 @@
code ValueExtractor = "(extractMaskValue)";
}
+class MarshallingInfoBooleanTrueFlag
+ : MarshallingInfoFlag {
+ bit ShouldAlwaysEmit = 1;
+ code Normalizer ="normalizeBooleanTrueFlag";
+ code Denormalizer = "denormalizeBooleanFlag";
+}
+
+class MarshallingInfoBooleanFalseFlag
+ : MarshallingInfoFlag {
+ bit ShouldAlwaysEmit = 1;
+ code Normalizer ="normalizeBooleanFalseFlag";
+ code Denormalizer = "denormalizeBooleanFlag";
+}
+
// Mixins for additional marshalling attributes.
class IsNegative {
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -138,6 +138,13 @@
return !Args.hasArg(Opt);
}
+void denormalizeSimpleFlag(SmallVectorImpl ,
+ const char *Spelling,
+ CompilerInvocation::StringAllocator SA,
+ unsigned TableIndex, unsigned Value) {
+ Args.push_back(Spelling);
+}
+
template
static llvm::Optional
normalizeFlagToValue(OptSpecifier Opt, unsigned TableIndex, const ArgList ,
@@ -147,6 +154,33 @@
return None;
}
+template
+static Optional
+normalizeBooleanTrueFlag(OptSpecifier PosOpt, unsigned TableIndex,
+ const ArgList , DiagnosticsEngine ) {
+ if (!Args.hasArg(PosOpt, NegOpt))
+return None;
+ return Args.hasFlag(PosOpt, NegOpt);
+}
+
+template
+static Optional
+normalizeBooleanFalseFlag(OptSpecifier NegOpt, unsigned TableIndex,
+ const ArgList , DiagnosticsEngine ) {
+ if (!Args.hasArg(PosOpt, NegOpt))
+return None;
+ return Args.hasFlag(PosOpt, NegOpt);
+}
+
+template
+static void denormalizeBooleanFlag(SmallVectorImpl ,
+ const char *Spelling,
+ CompilerInvocation::StringAllocator SA,
+ unsigned TableIndex, unsigned Value) {
+ if (Value == IsPositive)
+Args.push_back(Spelling);
+}
+
static llvm::Optional normalizeSimpleEnum(OptSpecifier Opt,
unsigned TableIndex,
const ArgList ,
@@ -169,12 +203,14 @@
}
static void denormalizeSimpleEnum(SmallVectorImpl ,
+ const char *Spelling,
CompilerInvocation::StringAllocator SA,
unsigned TableIndex, unsigned Value) {
assert(TableIndex < SimpleEnumValueTablesSize);
const SimpleEnumValueTable = SimpleEnumValueTables[TableIndex];
for (int I = 0, E = Table.Size; I != E; ++I) {
if (Value == Table.Table[I].Value) {
+ Args.push_back(Spelling);
Args.push_back(Table.Table[I].Name);
return;
}
@@ -185,8 +221,10 @@
}
static void denormalizeString(SmallVectorImpl ,
+ const char *Spelling,
CompilerInvocation::StringAllocator SA,
unsigned TableIndex, const std::string ) {
+ Args.push_back(Spelling);
Args.push_back(SA(Value));
}
@@ -782,10 +820,6 @@
}
}
- Opts.ExperimentalNewPassManager = Args.hasFlag(
- OPT_fexperimental_new_pass_manager, OPT_fno_experimental_new_pass_manager,
- /* Default */ ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER);
-
Opts.DebugPassManager =
Args.hasFlag(OPT_fdebug_pass_manager, OPT_fno_debug_pass_manager,
/* Default */ false);
@@ -3895,13 +3929,7 @@
TYPE, NORMALIZER, DENORMALIZER, MERGER, EXTRACTOR, TABLE_INDEX)\
if (((FLAGS)::CC1Option) && \
(ALWAYS_EMIT || EXTRACTOR(this->KEYPATH) != DEFAULT_VALUE)) {\
-if (Option::KIND##Class == Option::FlagClass) {\
- Args.push_back(SPELLING);\
-} \
-if (Option::KIND##Class == Option::SeparateClass) {\
-
[PATCH] D83071: Add support for options with two flags for controlling the same field.
dexonsmith requested changes to this revision.
dexonsmith added inline comments.
This revision now requires changes to proceed.
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:150
-static llvm::Optional normalizeSimpleEnum(OptSpecifier Opt,
-unsigned TableIndex,
I'm not sure if removing the `llvm::` namespace is intentional here, but if so
please do it in a separate NFC patch to avoid adding noise in this one.
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:161-163
+ if (!Args.hasArg(PosOpt, NegOpt))
+return None;
+ return Args.hasFlag(PosOpt, NegOpt);
This should be:
```
if (Arg *A = Args.getLastArg(PosOpt, NegOpt))
return A->getOption().matches(PosOpt);
return None;
```
Note that `hasArg` and `hasFlag` both resolve to `getLastArg`, so calling them
one after another would be unfortunate.
... but I'm not even sure where `NegOpt` is coming from here, that looks like
it hasn't been passed in. I think you need to change the signature to something
like this:
```
static llvm::Optional
normalizeSimpleFlag(OptSpecifier Opt,
Optional NegOpt,
unsigned TableIndex,
const ArgList ,
DiagnosticsEngine ) {
// Handle case without a `no-*` flag.
if (!NegOpt)
return Args.hasArg(Opt);
// Handle case with a `no-*` flag.
return Args.hasFlagAsOptional(Opt, *NegOpt);
}
```
It's possible you'll need to split up `OPTION_WITH_MARSHALLING` into two
disjoint lists of options:
- The list of options that can't be negated.
- The list of options that can be negated, calling a different macro that adds
macro arguments for the `CANCEL_ID` and `CANCEL_SPELLING`. For the denormalizer
you might also need `CANCEL_VALUE`.
- Note: the negating options themselves wouldn't be visited in either list.
- Note: the (de)normalizer APIs would ideally work naturally for something like
`-farg=val1` vs. `-farg=val2` vs. `-fno-arg`.
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3931-3932
if (((FLAGS)::CC1Option) &&
\
(ALWAYS_EMIT || EXTRACTOR(this->KEYPATH) != DEFAULT_VALUE)) {
\
-if (Option::KIND##Class == Option::FlagClass) {
\
- Args.push_back(SPELLING);
\
-}
\
-if (Option::KIND##Class == Option::SeparateClass) {
\
- Args.push_back(SPELLING);
\
- DENORMALIZER(Args, SA, TABLE_INDEX, EXTRACTOR(this->KEYPATH));
\
-}
\
+DENORMALIZER(Args, SPELLING, SA, TABLE_INDEX, EXTRACTOR(this->KEYPATH));
\
}
I realize this commit doesn't introduce it, but it seems unfortunate to call
`EXTRACTOR` twice. Maybe in a follow-up or prep commit you can fix that...
maybe something like this?
```
if ((FLAGS)::CC1Option) {
const auto = EXTRACTOR(this->KEYPATH);
if (ALWAYS_EMIT || Extracted != DEFAULT_VALUE)
DENORMALIZER(Args, SPELLING, SA, TABLE_INDEX, EXTRACTOR(this->KEYPATH));
}
```
Comment at: llvm/include/llvm/Option/OptParser.td:171
+ bit ShouldAlwaysEmit = 1;
+ code Normalizer ="normalizeBooleanTrueFlag";
+ code Denormalizer = "denormalizeBooleanFlag";
Nit: missing a space in `... Normalizer ="norma...`; same typo repeats below.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83071/new/
https://reviews.llvm.org/D83071
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D83071: Add support for options with two flags for controlling the same field.
dang created this revision.
dang added a reviewer: Bigcheese.
Herald added subscribers: llvm-commits, cfe-commits, dexonsmith.
Herald added projects: clang, LLVM.
This enables automatically parsing and generating CC1 arguments for options
where two flags control the same field, e.g. -fexperimental-new-pass-manager
and -fno-experimental new pass manager.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D83071
Files:
clang/include/clang/Driver/Options.td
clang/lib/Frontend/CompilerInvocation.cpp
llvm/include/llvm/Option/OptParser.td
Index: llvm/include/llvm/Option/OptParser.td
===
--- llvm/include/llvm/Option/OptParser.td
+++ llvm/include/llvm/Option/OptParser.td
@@ -156,6 +156,7 @@
: MarshallingInfo {
code NormalizerRetTy = ty;
code Normalizer = "normalizeSimpleFlag";
+ code Denormalizer = "denormalizeSimpleFlag";
}
class MarshallingInfoBitfieldFlag : MarshallingInfoFlag {
@@ -164,6 +165,20 @@
code ValueExtractor = "(extractMaskValue)";
}
+class MarshallingInfoBooleanTrueFlag
+ : MarshallingInfoFlag {
+ bit ShouldAlwaysEmit = 1;
+ code Normalizer ="normalizeBooleanTrueFlag";
+ code Denormalizer = "denormalizeBooleanFlag";
+}
+
+class MarshallingInfoBooleanFalseFlag
+ : MarshallingInfoFlag {
+ bit ShouldAlwaysEmit = 1;
+ code Normalizer ="normalizeBooleanFalseFlag";
+ code Denormalizer = "denormalizeBooleanFlag";
+}
+
// Mixins for additional marshalling attributes.
class IsNegative {
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -138,6 +138,13 @@
return !Args.hasArg(Opt);
}
+void denormalizeSimpleFlag(SmallVectorImpl ,
+ const char *Spelling,
+ CompilerInvocation::StringAllocator SA,
+ unsigned TableIndex, unsigned Value) {
+ Args.push_back(Spelling);
+}
+
template
static llvm::Optional
normalizeFlagToValue(OptSpecifier Opt, unsigned TableIndex, const ArgList ,
@@ -147,10 +154,37 @@
return None;
}
-static llvm::Optional normalizeSimpleEnum(OptSpecifier Opt,
-unsigned TableIndex,
-const ArgList ,
-DiagnosticsEngine ) {
+template
+static Optional
+normalizeBooleanTrueFlag(OptSpecifier PosOpt, unsigned TableIndex,
+ const ArgList , DiagnosticsEngine ) {
+ if (!Args.hasArg(PosOpt, NegOpt))
+return None;
+ return Args.hasFlag(PosOpt, NegOpt);
+}
+
+template
+static Optional
+normalizeBooleanFalseFlag(OptSpecifier NegOpt, unsigned TableIndex,
+ const ArgList , DiagnosticsEngine ) {
+ if (!Args.hasArg(PosOpt, NegOpt))
+return None;
+ return Args.hasFlag(PosOpt, NegOpt);
+}
+
+template
+static void denormalizeBooleanFlag(SmallVectorImpl ,
+ const char *Spelling,
+ CompilerInvocation::StringAllocator SA,
+ unsigned TableIndex, unsigned Value) {
+ if (Value == IsPositive)
+Args.push_back(Spelling);
+}
+
+static Optional normalizeSimpleEnum(OptSpecifier Opt,
+ unsigned TableIndex,
+ const ArgList ,
+ DiagnosticsEngine ) {
assert(TableIndex < SimpleEnumValueTablesSize);
const SimpleEnumValueTable = SimpleEnumValueTables[TableIndex];
@@ -169,12 +203,14 @@
}
static void denormalizeSimpleEnum(SmallVectorImpl ,
+ const char *Spelling,
CompilerInvocation::StringAllocator SA,
unsigned TableIndex, unsigned Value) {
assert(TableIndex < SimpleEnumValueTablesSize);
const SimpleEnumValueTable = SimpleEnumValueTables[TableIndex];
for (int I = 0, E = Table.Size; I != E; ++I) {
if (Value == Table.Table[I].Value) {
+ Args.push_back(Spelling);
Args.push_back(Table.Table[I].Name);
return;
}
@@ -185,8 +221,10 @@
}
static void denormalizeString(SmallVectorImpl ,
+ const char *Spelling,
CompilerInvocation::StringAllocator SA,
unsigned TableIndex, const std::string ) {
+ Args.push_back(Spelling);
Args.push_back(SA(Value));
}
@@ -782,10 +820,6 @@
}
}
- Opts.ExperimentalNewPassManager = Args.hasFlag(
- OPT_fexperimental_new_pass_manager, OPT_fno_experimental_new_pass_manager,
- /* Default */ ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER);
-
Opts.DebugPassManager =
Args.hasFlag(OPT_fdebug_pass_manager, OPT_fno_debug_pass_manager,
