[flang-commits] [PATCH] D127805: Bitwise comparison intrinsics

[Peter Klausler via Phabricator via flang-commits]

klausler added inline comments.


================
Comment at: flang/lib/Evaluate/fold-logical.cpp:16
+template <typename T>
+static std::optional<Expr<SomeType>> zeroExtend(const Constant<T> &c) {
+  std::vector<Scalar<LargestInt>> exts;
----------------
In semantics, function names are capitalized.


================
Comment at: flang/lib/Evaluate/fold-logical.cpp:18
+  std::vector<Scalar<LargestInt>> exts;
+  for (auto &v : c.values()) {
+    exts.push_back(Scalar<LargestInt>::ConvertUnsigned(v).value);
----------------
can be "const auto &v : ..."


================
Comment at: flang/lib/Evaluate/fold-logical.cpp:80
+    // must be done here.
+    std::optional<Expr<SomeType>> constArgs[2]{std::nullopt, std::nullopt};
+    for (int i{0}; i <= 1; i++) {
----------------
The initializer is not necessary -- it matches the default behavior.


================
Comment at: flang/lib/Evaluate/fold-logical.cpp:85
+      } else if (auto *x{UnwrapExpr<Expr<SomeInteger>>(args[i])}) {
+        std::visit(
+            [&](const auto &ix) {
----------------
common::visit has replaced std::visit


================
Comment at: flang/lib/Evaluate/fold-logical.cpp:96
+
+    if (constArgs[0].has_value() and constArgs[1].has_value()) {
+      auto fptr{&Scalar<LargestInt>::BGE};
----------------
The ".has_value()" is not needed, and we use "&&" rather than "and".


================
Comment at: flang/lib/Evaluate/fold-logical.cpp:109
+
+      for (int i{0}; i <= 1; i++)
+        *args[i] = std::move(constArgs[i].value());
----------------
We always use braces in semantics around if/for/while bodies, even when they are single statements.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127805/new/

https://reviews.llvm.org/D127805