[flang-commits] [flang] 196204c - [flang] Fix usage & catch errors for MAX/MIN with keyword= arguments

Peter Klausler via flang-commits flang-commits at lists.llvm.org
Tue Nov 30 12:53:56 PST 2021


Author: Peter Klausler
Date: 2021-11-30T12:53:47-08:00
New Revision: 196204c72c68a577c72af95d70f18e3550939a5e

URL: https://github.com/llvm/llvm-project/commit/196204c72c68a577c72af95d70f18e3550939a5e
DIFF: https://github.com/llvm/llvm-project/commit/196204c72c68a577c72af95d70f18e3550939a5e.diff

LOG: [flang] Fix usage & catch errors for MAX/MIN with keyword= arguments

Max(), MIN(), and their specific variants are defined with an unlimited
number of dummy arguments named A1=, A2=, &c. whose names are almost never
used in practice but should be allowed for and properly checked for the
usual errors when they do appear.  The intrinsic table's entries otherwise
have fixed numbers of dummy argument definitions, so add some special
case handling in a few spots for MAX/MIN/&c. checking and procedure
characteristics construction.

Differential Revision: https://reviews.llvm.org/D114750

Added: 
    flang/test/Semantics/call23.f90

Modified: 
    flang/lib/Evaluate/intrinsics.cpp
    flang/test/Evaluate/folding01.f90

Removed: 
    


################################################################################
diff  --git a/flang/lib/Evaluate/intrinsics.cpp b/flang/lib/Evaluate/intrinsics.cpp
index 39e901c4c7763..d647bdc3399d1 100644
--- a/flang/lib/Evaluate/intrinsics.cpp
+++ b/flang/lib/Evaluate/intrinsics.cpp
@@ -1165,6 +1165,36 @@ static DynamicType GetBuiltinDerivedType(
   return DynamicType{derived};
 }
 
+// Ensure that the keywords of arguments to MAX/MIN and their variants
+// are of the form A123 with no duplicates.
+static bool CheckMaxMinArgument(std::optional<parser::CharBlock> keyword,
+    std::set<parser::CharBlock> &set, const char *intrinsicName,
+    parser::ContextualMessages &messages) {
+  if (keyword) {
+    std::size_t j{1};
+    for (; j < keyword->size(); ++j) {
+      char ch{(*keyword)[j]};
+      if (ch < '0' || ch > '9') {
+        break;
+      }
+    }
+    if (keyword->size() < 2 || (*keyword)[0] != 'a' || j < keyword->size()) {
+      messages.Say(*keyword,
+          "Argument keyword '%s=' is not known in call to '%s'"_err_en_US,
+          *keyword, intrinsicName);
+      return false;
+    }
+    auto [_, wasInserted]{set.insert(*keyword)};
+    if (!wasInserted) {
+      messages.Say(*keyword,
+          "Argument keyword '%s=' was repeated in call to '%s'"_err_en_US,
+          *keyword, intrinsicName);
+      return false;
+    }
+  }
+  return true;
+}
+
 // Intrinsic interface matching against the arguments of a particular
 // procedure reference.
 std::optional<SpecificCall> IntrinsicInterface::Match(
@@ -1182,28 +1212,32 @@ std::optional<SpecificCall> IntrinsicInterface::Match(
   }
   // MAX and MIN (and others that map to them) allow their last argument to
   // be repeated indefinitely.  The actualForDummy vector is sized
-  // and null-initialized to the non-repeated dummy argument count,
-  // but additional actual argument pointers can be pushed on it
-  // when this flag is set.
-  bool repeatLastDummy{dummyArgPatterns > 0 &&
+  // and null-initialized to the non-repeated dummy argument count
+  // for other instrinsics.
+  bool isMaxMin{dummyArgPatterns > 0 &&
       dummy[dummyArgPatterns - 1].optionality == Optionality::repeats};
-  std::size_t nonRepeatedDummies{
-      repeatLastDummy ? dummyArgPatterns - 1 : dummyArgPatterns};
-  std::vector<ActualArgument *> actualForDummy(nonRepeatedDummies, nullptr);
+  std::vector<ActualArgument *> actualForDummy(
+      isMaxMin ? 0 : dummyArgPatterns, nullptr);
   int missingActualArguments{0};
+  std::set<parser::CharBlock> maxMinKeywords;
   for (std::optional<ActualArgument> &arg : arguments) {
     if (!arg) {
       ++missingActualArguments;
-    } else {
-      if (arg->isAlternateReturn()) {
-        messages.Say(
-            "alternate return specifier not acceptable on call to intrinsic '%s'"_err_en_US,
-            name);
+    } else if (arg->isAlternateReturn()) {
+      messages.Say(
+          "alternate return specifier not acceptable on call to intrinsic '%s'"_err_en_US,
+          name);
+      return std::nullopt;
+    } else if (isMaxMin) {
+      if (CheckMaxMinArgument(arg->keyword(), maxMinKeywords, name, messages)) {
+        actualForDummy.push_back(&*arg);
+      } else {
         return std::nullopt;
       }
+    } else {
       bool found{false};
       int slot{missingActualArguments};
-      for (std::size_t j{0}; j < nonRepeatedDummies && !found; ++j) {
+      for (std::size_t j{0}; j < dummyArgPatterns && !found; ++j) {
         if (dummy[j].optionality == Optionality::missing) {
           continue;
         }
@@ -1232,19 +1266,14 @@ std::optional<SpecificCall> IntrinsicInterface::Match(
         }
       }
       if (!found) {
-        if (repeatLastDummy && !arg->keyword()) {
-          // MAX/MIN argument after the 2nd
-          actualForDummy.push_back(&*arg);
+        if (arg->keyword()) {
+          messages.Say(*arg->keyword(),
+              "unknown keyword argument to intrinsic '%s'"_err_en_US, name);
         } else {
-          if (arg->keyword()) {
-            messages.Say(*arg->keyword(),
-                "unknown keyword argument to intrinsic '%s'"_err_en_US, name);
-          } else {
-            messages.Say(
-                "too many actual arguments for intrinsic '%s'"_err_en_US, name);
-          }
-          return std::nullopt;
+          messages.Say(
+              "too many actual arguments for intrinsic '%s'"_err_en_US, name);
         }
+        return std::nullopt;
       }
     }
   }
@@ -1750,8 +1779,12 @@ std::optional<SpecificCall> IntrinsicInterface::Match(
     const IntrinsicDummyArgument &d{dummy[std::min(j, dummyArgPatterns - 1)]};
     if (const auto &arg{rearranged[j]}) {
       if (const Expr<SomeType> *expr{arg->UnwrapExpr()}) {
+        std::string kw{d.keyword};
+        if (isMaxMin) {
+          kw = "a"s + std::to_string(j + 1);
+        }
         auto dc{characteristics::DummyArgument::FromActual(
-            std::string{d.keyword}, *expr, context)};
+            std::move(kw), *expr, context)};
         if (!dc) {
           common::die("INTERNAL: could not characterize intrinsic function "
                       "actual argument '%s'",

diff  --git a/flang/test/Evaluate/folding01.f90 b/flang/test/Evaluate/folding01.f90
index cc3b17e4c2c43..39a35e094b3e3 100644
--- a/flang/test/Evaluate/folding01.f90
+++ b/flang/test/Evaluate/folding01.f90
@@ -102,7 +102,7 @@ module m
 
   ! test MIN and MAX
   real, parameter :: x1 = -35., x2= -35.05, x3=0., x4=35.05, x5=35.
-  real, parameter :: res_max_r = max(x1, x2, x3, x4, x5)
+  real, parameter :: res_max_r = max(a1=x1, a2=x2, a3=x3, a4=x4, a5=x5)
   real, parameter :: res_min_r = min(x1, x2, x3, x4, x5)
   logical, parameter :: test_max_r = res_max_r.EQ.x4
   logical, parameter :: test_min_r = res_min_r.EQ.x2

diff  --git a/flang/test/Semantics/call23.f90 b/flang/test/Semantics/call23.f90
new file mode 100644
index 0000000000000..706d82371c27a
--- /dev/null
+++ b/flang/test/Semantics/call23.f90
@@ -0,0 +1,10 @@
+! RUN: %python %S/test_errors.py %s %flang_fc1
+! Check errors on MAX/MIN with keywords, a weird case in Fortran
+real :: x = 0.0 ! prevent folding
+!ERROR: Argument keyword 'a1=' was repeated in call to 'max'
+print *, max(a1=x,a1=1)
+!ERROR: Keyword argument 'a1=' has already been specified positionally (#1) in this procedure reference
+print *, max(x,a1=1)
+!ERROR: Argument keyword 'a6=' is not recognized for this procedure reference
+print *, max(a1=x,a2=0,a3=0,a4=0,a6=0)
+end


        


More information about the flang-commits mailing list