[PATCH] D93628: [clang] NFC: Refactor custom class into a lambda in CompilerInvocation
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbef9eb84b2fb: [clang] NFC: Refactor custom class into a
lambda in CompilerInvocation (authored by jansvoboda11).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D93628/new/
https://reviews.llvm.org/D93628
Files:
clang/lib/Frontend/CompilerInvocation.cpp
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -157,33 +157,26 @@
Args.push_back(Spelling);
}
-namespace {
-template struct FlagToValueNormalizer {
- T Value;
+template static constexpr bool is_uint64_t_convertible() {
+ return !std::is_same::value &&
+ llvm::is_integral_or_enum::value;
+}
- Optional operator()(OptSpecifier Opt, unsigned, const ArgList &Args,
- DiagnosticsEngine &) {
+template (), bool> = false>
+static auto makeFlagToValueNormalizer(T Value) {
+ return [Value](OptSpecifier Opt, unsigned, const ArgList &Args,
+ DiagnosticsEngine &) -> Optional {
if (Args.hasArg(Opt))
return Value;
return None;
- }
-};
-} // namespace
-
-template static constexpr bool is_int_convertible() {
- return sizeof(T) <= sizeof(uint64_t) &&
- std::is_trivially_constructible::value &&
- std::is_trivially_constructible::value;
-}
-
-template (), bool> = false>
-static FlagToValueNormalizer makeFlagToValueNormalizer(T Value) {
- return FlagToValueNormalizer{Value};
+ };
}
-template (), bool> = false>
-static FlagToValueNormalizer makeFlagToValueNormalizer(T Value) {
- return FlagToValueNormalizer{std::move(Value)};
+template (), bool> = false>
+static auto makeFlagToValueNormalizer(T Value) {
+ return makeFlagToValueNormalizer(uint64_t(Value));
}
static auto makeBooleanOptionNormalizer(bool Value, bool OtherValue,
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -157,33 +157,26 @@
Args.push_back(Spelling);
}
-namespace {
-template struct FlagToValueNormalizer {
- T Value;
+template static constexpr bool is_uint64_t_convertible() {
+ return !std::is_same::value &&
+ llvm::is_integral_or_enum::value;
+}
- Optional operator()(OptSpecifier Opt, unsigned, const ArgList &Args,
- DiagnosticsEngine &) {
+template (), bool> = false>
+static auto makeFlagToValueNormalizer(T Value) {
+ return [Value](OptSpecifier Opt, unsigned, const ArgList &Args,
+ DiagnosticsEngine &) -> Optional {
if (Args.hasArg(Opt))
return Value;
return None;
- }
-};
-} // namespace
-
-template static constexpr bool is_int_convertible() {
- return sizeof(T) <= sizeof(uint64_t) &&
- std::is_trivially_constructible::value &&
- std::is_trivially_constructible::value;
-}
-
-template (), bool> = false>
-static FlagToValueNormalizer makeFlagToValueNormalizer(T Value) {
- return FlagToValueNormalizer{Value};
+ };
}
-template (), bool> = false>
-static FlagToValueNormalizer makeFlagToValueNormalizer(T Value) {
- return FlagToValueNormalizer{std::move(Value)};
+template (), bool> = false>
+static auto makeFlagToValueNormalizer(T Value) {
+ return makeFlagToValueNormalizer(uint64_t(Value));
}
static auto makeBooleanOptionNormalizer(bool Value, bool OtherValue,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D93628: [clang] NFC: Refactor custom class into a lambda in CompilerInvocation
jansvoboda11 added inline comments.
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:176
+static auto makeFlagToValueNormalizer(T Value) {
+ return makeFlagToValueNormalizer(uint64_t(Value));
}
dexonsmith wrote:
> (not a behaviour change but) I wonder if this correct for signed types, where
> the conversion back to `T` could change sign if it's a negative value. Should
> there be an assertion that the value is `>= 0`? (Probably to do separately /
> outside this patch)
It might be good to be defensive here. I'll put that into a follow-up.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D93628/new/
https://reviews.llvm.org/D93628
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D93628: [clang] NFC: Refactor custom class into a lambda in CompilerInvocation
jansvoboda11 updated this revision to Diff 313309.
jansvoboda11 added a comment.
Extract SFINAE check into function again
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D93628/new/
https://reviews.llvm.org/D93628
Files:
clang/lib/Frontend/CompilerInvocation.cpp
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -157,33 +157,26 @@
Args.push_back(Spelling);
}
-namespace {
-template struct FlagToValueNormalizer {
- T Value;
+template static constexpr bool is_uint64_t_convertible() {
+ return !std::is_same::value &&
+ llvm::is_integral_or_enum::value;
+}
- Optional operator()(OptSpecifier Opt, unsigned, const ArgList &Args,
- DiagnosticsEngine &) {
+template (), bool> = false>
+static auto makeFlagToValueNormalizer(T Value) {
+ return [Value](OptSpecifier Opt, unsigned, const ArgList &Args,
+ DiagnosticsEngine &) -> Optional {
if (Args.hasArg(Opt))
return Value;
return None;
- }
-};
-} // namespace
-
-template static constexpr bool is_int_convertible() {
- return sizeof(T) <= sizeof(uint64_t) &&
- std::is_trivially_constructible::value &&
- std::is_trivially_constructible::value;
-}
-
-template (), bool> = false>
-static FlagToValueNormalizer makeFlagToValueNormalizer(T Value) {
- return FlagToValueNormalizer{Value};
+ };
}
-template (), bool> = false>
-static FlagToValueNormalizer makeFlagToValueNormalizer(T Value) {
- return FlagToValueNormalizer{std::move(Value)};
+template (), bool> = false>
+static auto makeFlagToValueNormalizer(T Value) {
+ return makeFlagToValueNormalizer(uint64_t(Value));
}
static auto makeBooleanOptionNormalizer(bool Value, bool OtherValue,
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -157,33 +157,26 @@
Args.push_back(Spelling);
}
-namespace {
-template struct FlagToValueNormalizer {
- T Value;
+template static constexpr bool is_uint64_t_convertible() {
+ return !std::is_same::value &&
+ llvm::is_integral_or_enum::value;
+}
- Optional operator()(OptSpecifier Opt, unsigned, const ArgList &Args,
- DiagnosticsEngine &) {
+template (), bool> = false>
+static auto makeFlagToValueNormalizer(T Value) {
+ return [Value](OptSpecifier Opt, unsigned, const ArgList &Args,
+ DiagnosticsEngine &) -> Optional {
if (Args.hasArg(Opt))
return Value;
return None;
- }
-};
-} // namespace
-
-template static constexpr bool is_int_convertible() {
- return sizeof(T) <= sizeof(uint64_t) &&
- std::is_trivially_constructible::value &&
- std::is_trivially_constructible::value;
-}
-
-template (), bool> = false>
-static FlagToValueNormalizer makeFlagToValueNormalizer(T Value) {
- return FlagToValueNormalizer{Value};
+ };
}
-template (), bool> = false>
-static FlagToValueNormalizer makeFlagToValueNormalizer(T Value) {
- return FlagToValueNormalizer{std::move(Value)};
+template (), bool> = false>
+static auto makeFlagToValueNormalizer(T Value) {
+ return makeFlagToValueNormalizer(uint64_t(Value));
}
static auto makeBooleanOptionNormalizer(bool Value, bool OtherValue,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D93628: [clang] NFC: Refactor custom class into a lambda in CompilerInvocation
dexonsmith accepted this revision.
dexonsmith added a comment.
This revision is now accepted and ready to land.
This looks like an improvement, so LGTM, but I have a couple of comments.
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:160-161
-namespace {
-template struct FlagToValueNormalizer {
- T Value;
-
- Optional operator()(OptSpecifier Opt, unsigned, const ArgList &Args,
- DiagnosticsEngine &) {
+template ::value &&
+ llvm::is_integral_or_enum::value),
+ bool> = false>
I missed the double negation here for a while; might be simpler as:
```
::value ||
!llvm::is_integral_or_neum::value>
```
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:172-174
+template ::value &&
+llvm::is_integral_or_enum::value),
+ bool> = false>
Ah, I guess you want this to match the above. I guess that's the benefit of the
old `is_int_convertible` -- it didn't need double-negation to make it clear
that the two functions were alternatives. I might prefer the previous option of
having a stand-alone function (possibly renamed if you think the old name isn't
correct anymore). (Regardless, you can drop the unnecessary parentheses.)
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:176
+static auto makeFlagToValueNormalizer(T Value) {
+ return makeFlagToValueNormalizer(uint64_t(Value));
}
(not a behaviour change but) I wonder if this correct for signed types, where
the conversion back to `T` could change sign if it's a negative value. Should
there be an assertion that the value is `>= 0`? (Probably to do separately /
outside this patch)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D93628/new/
https://reviews.llvm.org/D93628
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D93628: [clang] NFC: Refactor custom class into a lambda in CompilerInvocation
jansvoboda11 created this revision.
jansvoboda11 added reviewers: dexonsmith, Bigcheese.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Change `makeFlagToValueNormalizer` so that one specialization converts all
integral/enum arguments into `uint64_t` and forwards them to the more generic
version.
This makes it easy to replace the custom `FlagToValueNormalizer` struct with a
lambda, which is the common approach in other (de)normalizers.
Finally, drop custom `is_int_convertbile` in favor of
`llvm::is_integral_or_enum`.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D93628
Files:
clang/lib/Frontend/CompilerInvocation.cpp
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -157,33 +157,23 @@
Args.push_back(Spelling);
}
-namespace {
-template struct FlagToValueNormalizer {
- T Value;
-
- Optional operator()(OptSpecifier Opt, unsigned, const ArgList &Args,
- DiagnosticsEngine &) {
+template ::value &&
+ llvm::is_integral_or_enum::value),
+ bool> = false>
+static auto makeFlagToValueNormalizer(T Value) {
+ return [Value](OptSpecifier Opt, unsigned, const ArgList &Args,
+ DiagnosticsEngine &) -> Optional {
if (Args.hasArg(Opt))
return Value;
return None;
- }
-};
-} // namespace
-
-template static constexpr bool is_int_convertible() {
- return sizeof(T) <= sizeof(uint64_t) &&
- std::is_trivially_constructible::value &&
- std::is_trivially_constructible::value;
-}
-
-template (), bool> = false>
-static FlagToValueNormalizer makeFlagToValueNormalizer(T Value) {
- return FlagToValueNormalizer{Value};
+ };
}
-template (), bool> = false>
-static FlagToValueNormalizer makeFlagToValueNormalizer(T Value) {
- return FlagToValueNormalizer{std::move(Value)};
+template ::value &&
+llvm::is_integral_or_enum::value),
+ bool> = false>
+static auto makeFlagToValueNormalizer(T Value) {
+ return makeFlagToValueNormalizer(uint64_t(Value));
}
static auto makeBooleanOptionNormalizer(bool Value, bool OtherValue,
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -157,33 +157,23 @@
Args.push_back(Spelling);
}
-namespace {
-template struct FlagToValueNormalizer {
- T Value;
-
- Optional operator()(OptSpecifier Opt, unsigned, const ArgList &Args,
- DiagnosticsEngine &) {
+template ::value &&
+ llvm::is_integral_or_enum::value),
+ bool> = false>
+static auto makeFlagToValueNormalizer(T Value) {
+ return [Value](OptSpecifier Opt, unsigned, const ArgList &Args,
+ DiagnosticsEngine &) -> Optional {
if (Args.hasArg(Opt))
return Value;
return None;
- }
-};
-} // namespace
-
-template static constexpr bool is_int_convertible() {
- return sizeof(T) <= sizeof(uint64_t) &&
- std::is_trivially_constructible::value &&
- std::is_trivially_constructible::value;
-}
-
-template (), bool> = false>
-static FlagToValueNormalizer makeFlagToValueNormalizer(T Value) {
- return FlagToValueNormalizer{Value};
+ };
}
-template (), bool> = false>
-static FlagToValueNormalizer makeFlagToValueNormalizer(T Value) {
- return FlagToValueNormalizer{std::move(Value)};
+template ::value &&
+llvm::is_integral_or_enum::value),
+ bool> = false>
+static auto makeFlagToValueNormalizer(T Value) {
+ return makeFlagToValueNormalizer(uint64_t(Value));
}
static auto makeBooleanOptionNormalizer(bool Value, bool OtherValue,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
