[flang] Warn about defined operator with too few/many dummy arguments

[klausler]

Summary

A function in a defined operator generic interface will syntactically
have one or two arguments. If a defined operator includes a specific
function with 0 or more than 2 dummy arguments, there's no way that it
could be invoked by way of the interface. Emit a warning.

[PeteSteinfeld]

All builds and tests correctly and looks good.

flang/include/flang/Parser/message.h

public:
   constexpr MessageFixedText &operator=(MessageFixedText &&) = default;
 
   CharBlock text() const { return text_; }
   Severity severity() const { return severity_; }
   MessageFixedText &set_severity(Severity severity) {
     severity_ = severity;
     return *this;
   }
-  bool isFatal() const {
+  bool IsFatal() const {
     return severity_ == Severity::Error || severity_ == Severity::Todo;
   }
 
 private:
   CharBlock text_;
   Severity severity_{Severity::None};
 };
 
MessageFormattedText(const MessageFixedText &text, A &&...x)
       : severity_{text.severity()} {
     Format(&text, Convert(std::forward<A>(x))...);
   }
   MessageFormattedText(const MessageFormattedText &) = default;
   MessageFormattedText(MessageFormattedText &&) = default;
   MessageFormattedText &operator=(const MessageFormattedText &) = default;
   MessageFormattedText &operator=(MessageFormattedText &&) = default;
   const std::string &string() const { return string_; }
-  bool isFatal() const {
+  bool IsFatal() const {
     return severity_ == Severity::Error || severity_ == Severity::Todo;
   }
   Severity severity() const { return severity_; }
   MessageFormattedText &set_severity(Severity severity) {
     severity_ = severity;
     return *this;
   }
   std::string MoveString() { return std::move(string_); }

flang/lib/Semantics/check-declarations.cpp

if (specific.attrs().test(Attr::NOPASS)) { // C774
     msg = std::move(m);
   } else if (!checkDefinedOperatorArgs(opName, specific, proc)) {
     return false; // error was reported
   } else if (ConflictsWithIntrinsicOperator(kind, proc)) {
     msg = "%s function '%s' conflicts with intrinsic operator"_err_en_US;
   } else {
     return true; // OK
   }
+  bool isFatal{msg->IsFatal()};
   SayWithDeclaration(
       specific, std::move(*msg), MakeOpName(opName), specific.name());
+  if (isFatal) {
     context_.SetError(specific);
+  }
   return false;
 }
 
 // If the number of arguments is wrong for this intrinsic operator, return
 // false and return the error message in msg.
 std::optional<parser::MessageFixedText> CheckHelper::CheckNumberOfArgs(
     const GenericKind &kind, std::size_t nargs) {
   if (!kind.IsIntrinsicOperator()) {
+    if (nargs < 1 || nargs > 2) {
+      return "%s function '%s' should have 1 or 2 dummy arguments"_warn_en_US;
+    }
     return std::nullopt;
   }
   std::size_t min{2}, max{2}; // allowed number of args; default is binary
   common::visit(common::visitors{
                     [&](const common::NumericOperator &x) {
                       if (x == common::NumericOperator::Add ||
                           x == common::NumericOperator::Subtract) {
                         min = 1; // + and - are unary or binary

flang/lib/Semantics/resolve-names.cpp

void ScopeHandler::SayAlreadyDeclared(
   } else {
     Say(name1, "'%s' is already declared in this scoping unit"_err_en_US)
         .Attach(name2, "Previous declaration of '%s'"_en_US, name2);
   }
 }
 
 void ScopeHandler::SayWithReason(const parser::Name &name, Symbol &symbol,
     MessageFixedText &&msg1, Message &&msg2) {
+  bool isFatal{msg1.IsFatal()};
   Say(name, std::move(msg1), symbol.name()).Attach(std::move(msg2));
-  context().SetError(symbol, msg1.isFatal());
+  context().SetError(symbol, isFatal);
 }
 
 void ScopeHandler::SayWithDecl(
     const parser::Name &name, Symbol &symbol, MessageFixedText &&msg) {
+  bool isFatal{msg.IsFatal()};
   Say(name, std::move(msg), symbol.name())
       .Attach(Message{name.source,
           symbol.test(Symbol::Flag::Implicit)
               ? "Implicit declaration of '%s'"_en_US
               : "Declaration of '%s'"_en_US,
           name.source});
-  context().SetError(symbol, msg.isFatal());
+  context().SetError(symbol, isFatal);
 }
 
 void ScopeHandler::SayLocalMustBeVariable(
     const parser::Name &name, Symbol &symbol) {
   SayWithDecl(name, symbol,
       "The name '%s' must be a variable to appear"
       " in a locality-spec"_err_en_US);
 }
 
 void ScopeHandler::SayDerivedType(
     const SourceName &name, MessageFixedText &&msg, const Scope &type) {
   const Symbol &typeSymbol{DEREF(type.GetSymbol())};
   Say(name, std::move(msg), name, typeSymbol.name())
       .Attach(typeSymbol.name(), "Declaration of derived type '%s'"_en_US,
           typeSymbol.name());
 }
 void ScopeHandler::Say2(const SourceName &name1, MessageFixedText &&msg1,
     const SourceName &name2, MessageFixedText &&msg2) {
   Say(name1, std::move(msg1)).Attach(name2, std::move(msg2), name2);
 }
 void ScopeHandler::Say2(const SourceName &name, MessageFixedText &&msg1,
     Symbol &symbol, MessageFixedText &&msg2) {
+  bool isFatal{msg1.IsFatal()};
   Say2(name, std::move(msg1), symbol.name(), std::move(msg2));
-  context().SetError(symbol, msg1.isFatal());
+  context().SetError(symbol, isFatal);
 }
 void ScopeHandler::Say2(const parser::Name &name, MessageFixedText &&msg1,
     Symbol &symbol, MessageFixedText &&msg2) {
+  bool isFatal{msg1.IsFatal()};
   Say2(name.source, std::move(msg1), symbol.name(), std::move(msg2));
-  context().SetError(symbol, msg1.isFatal());
+  context().SetError(symbol, isFatal);
 }
 
 // This is essentially GetProgramUnitContaining(), but it can return
 // a mutable Scope &, it ignores statement functions, and it fails
 // gracefully for error recovery (returning the original Scope).
 template <typename T> static T &GetInclusiveScope(T &scope) {
   for (T *s{&scope}; !s->IsGlobal(); s = &s->parent()) {
     switch (s->kind()) {

flang/test/Semantics/generic04.f90

+! RUN: %python %S/test_errors.py %s %flang_fc1
+! Warn about inaccessible specific procedures in a generic defined operator
+module m
+  interface operator (.foo.)
+    !WARN: OPERATOR(.foo.) function 'noargs' must have 1 or 2 dummy arguments
+    module procedure noargs
+    !WARN: OPERATOR(.foo.) function 'noargs' must have 1 or 2 dummy arguments
+    module procedure threeargs
+  end interface
+  type t
+   contains
+    procedure :: bad
+    !WARN: OPERATOR(.bar.) function 'bad' should have 1 or 2 dummy arguments
+    generic :: operator (.bar.) => bad
+  end type
+ contains
+  real function noargs()
+    noargs = 0.
+  end
+  real function threeargs(fee,fie,foe)
+    real, intent(in) :: fee, fie, foe
+  end
+  function bad(this,x,y)
+    type(t) :: bad
+    class(t), intent(in) :: this, x, y
+    bad = x
+  end
+end