[flang] Downgrade error to warning for consistency

[klausler]

When a procedure name is declared EXTERNAL and then followed up with an explicit INTERFACE, the compiler emits an error when the procedure is a dummy procedure but only a warning otherwise. Since the EXTERNAL statement is harmless in both cases, accept the case of a dummy procedure as well for consistency.

Fixes #157162.

[llvmbot]

@llvm/pr-subscribers-flang-semantics

Author: Peter Klausler (klausler)

Changes

When a procedure name is declared EXTERNAL and then followed up with an explicit INTERFACE, the compiler emits an error when the procedure is a dummy procedure but only a warning otherwise. Since the EXTERNAL statement is harmless in both cases, accept the case of a dummy procedure as well for consistency.

Fixes #157162.

---

Full diff: https://github.com/llvm/llvm-project/pull/157191.diff

3 Files Affected:

• (modified) flang/lib/Semantics/resolve-names.cpp (+8-2)
• (modified) flang/lib/Semantics/symbol.cpp (+10-6)
• (modified) flang/test/Semantics/resolve20.f90 (+8)

diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index 4720932780472..077bee930675e 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -646,12 +646,18 @@ class ScopeHandler : public ImplicitRulesVisitor {
     }
     if (symbol->CanReplaceDetails(details)) {
       // update the existing symbol
-      CheckDuplicatedAttrs(name, *symbol, attrs);
-      SetExplicitAttrs(*symbol, attrs);
       if constexpr (std::is_same_v<SubprogramDetails, D>) {
         // Dummy argument defined by explicit interface?
         details.set_isDummy(IsDummy(*symbol));
+        if (symbol->has<ProcEntityDetails>()) {
+          // Bare "EXTERNAL" dummy replaced with explicit INTERFACE
+          context().Warn(common::LanguageFeature::RedundantAttribute, name,
+              "Dummy argument '%s' was declared earlier as EXTERNAL"_warn_en_US,
+              name);
+        }
       }
+      CheckDuplicatedAttrs(name, *symbol, attrs);
+      SetExplicitAttrs(*symbol, attrs);
       symbol->set_details(std::move(details));
       return *symbol;
     } else if constexpr (std::is_same_v<UnknownDetails, D>) {
diff --git a/flang/lib/Semantics/symbol.cpp b/flang/lib/Semantics/symbol.cpp
index a6b402c48d4ff..ea7eeac80a2d9 100644
--- a/flang/lib/Semantics/symbol.cpp
+++ b/flang/lib/Semantics/symbol.cpp
@@ -330,8 +330,14 @@ bool Symbol::CanReplaceDetails(const Details &details) const {
         common::visitors{
             [](const UseErrorDetails &) { return true; },
             [&](const ObjectEntityDetails &) { return has<EntityDetails>(); },
-            [&](const ProcEntityDetails &) { return has<EntityDetails>(); },
+            [&](const ProcEntityDetails &x) { return has<EntityDetails>(); },
             [&](const SubprogramDetails &) {
+              if (const auto *oldProc{detailsIf<ProcEntityDetails>()}) {
+                // Can replace bare "EXTERNAL dummy" with explicit INTERFACE
+                return oldProc->isDummy() && !oldProc->procInterface() &&
+                    attrs().test(Attr::EXTERNAL) && !test(Flag::Function) &&
+                    !test(Flag::Subroutine);
+              }
               return has<SubprogramNameDetails>() || has<EntityDetails>();
             },
             [&](const DerivedTypeDetails &) {
@@ -339,14 +345,12 @@ bool Symbol::CanReplaceDetails(const Details &details) const {
               return derived && derived->isForwardReferenced();
             },
             [&](const UseDetails &x) {
-              const auto *use{this->detailsIf<UseDetails>()};
+              const auto *use{detailsIf<UseDetails>()};
               return use && use->symbol() == x.symbol();
             },
-            [&](const HostAssocDetails &) {
-              return this->has<HostAssocDetails>();
-            },
+            [&](const HostAssocDetails &) { return has<HostAssocDetails>(); },
             [&](const UserReductionDetails &) {
-              return this->has<UserReductionDetails>();
+              return has<UserReductionDetails>();
             },
             [](const auto &) { return false; },
         },
diff --git a/flang/test/Semantics/resolve20.f90 b/flang/test/Semantics/resolve20.f90
index 8b8d190206689..f1a1a30cc714e 100644
--- a/flang/test/Semantics/resolve20.f90
+++ b/flang/test/Semantics/resolve20.f90
@@ -89,4 +89,12 @@ subroutine test
     !ERROR: Abstract procedure interface 'f' may not be referenced
     x = f()
   end subroutine
+  subroutine baz(foo)
+    external foo
+    interface
+      !WARNING: Dummy argument 'foo' was declared earlier as EXTERNAL [-Wredundant-attribute]
+      subroutine foo(x)
+      end
+    end interface
+  end
 end module

[kkwli]

LG. Thanks for the quick fix.

[kkwli]

Sorry, I miss this case. Should it be covered in this fix?

      interface
        subroutine g(f)
          external f
          interface
            subroutine f()
            end
          end interface
        end
      end interface
      end

Output:

error: Semantic errors in dd2.f90
./dd2.f90:5:24: error: 'f' is already declared in this scoping unit
              subroutine f()
                         ^
./dd2.f90:2:22: Previous declaration of 'f'
          subroutine g(f)
                       ^

[kkwli]

If f is not a dummy procedure, the warning is issue.

      interface
        subroutine g()
          external f
          interface
            subroutine f()
            end
          end interface
        end
      end interface
      end

Output:

./dd2.f90:5:24: warning: EXTERNAL attribute was already specified on 'f' [-Wredundant-attribute]
              subroutine f()
                         ^

[klausler]

@kkwli both of the examples in your comments above should now be warnings. I'll check them and update the patch if necessary

UPDATE: Both of your examples above now produce only warnings with my patch. Are you seeing something different, @kkwli?

[kkwli]

@kkwli both of the examples in your comments above should now be warnings. I'll check them and update the patch if necessary

UPDATE: Both of your examples above now produce only warnings with my patch. Are you seeing something different, @kkwli?

@klausler My bad! I messed up the build! A warning message is emitted in both cases. Everything looks fine.

[klausler]

@kkwli both of the examples in your comments above should now be warnings. I'll check them and update the patch if necessary
UPDATE: Both of your examples above now produce only warnings with my patch. Are you seeing something different, @kkwli?

@klausler My bad! I messed up the build! A warning message is emitted in both cases. Everything looks fine.

Thanks for checking.

[kparzysz]

What happened to the CI checks in this PR?