Skip to content

[flang] Fix spurious error messages due to INTRINSIC nested in BLOCK #115889

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 14, 2024

Conversation

klausler
Copy link
Contributor

When skimmming executable parts to collect names used in procedure calls, it is important to exclude names that have local declarations in nested BLOCK constructs. The mechanism for handling these nested declarations was catching only names whose declarations include an "entity-decl", and so names appearing in other declaration statements (like INTRINSIC and EXTERNAL statements) were not hidden from the scan, leading to absurd error messages when such names turn out to be procedures in the nested BLOCK construct but to not be procedures outside it.

This patch fixes the code that detects local declarations in BLOCK for all of the missed cases that don't use entity-decls; only INTRINSIC and EXTERNAL could affect the procedures whose names are of interest to the executable part skimmer, but perhaps future work will want to collect non-procedures as well, so I plugged all of the holes that I could find.

Fixes #115674.

When skimmming executable parts to collect names used in procedure calls,
it is important to exclude names that have local declarations in nested BLOCK
constructs.  The mechanism for handling these nested declarations was catching
only names whose declarations include an "entity-decl", and so names appearing
in other declaration statements (like INTRINSIC and EXTERNAL statements) were
not hidden from the scan, leading to absurd error messages when such names
turn out to be procedures in the nested BLOCK construct but to not be
procedures outside it.

This patch fixes the code that detects local declarations in BLOCK
for all of the missed cases that don't use entity-decls; only INTRINSIC
and EXTERNAL could affect the procedures whose names are of interest
to the executable part skimmer, but perhaps future work will want to
collect non-procedures as well, so I plugged all of the holes that I
could find.

Fixes llvm#115674.
@klausler klausler requested a review from clementval November 12, 2024 16:03
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:semantics labels Nov 12, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 12, 2024

@llvm/pr-subscribers-flang-semantics

Author: Peter Klausler (klausler)

Changes

When skimmming executable parts to collect names used in procedure calls, it is important to exclude names that have local declarations in nested BLOCK constructs. The mechanism for handling these nested declarations was catching only names whose declarations include an "entity-decl", and so names appearing in other declaration statements (like INTRINSIC and EXTERNAL statements) were not hidden from the scan, leading to absurd error messages when such names turn out to be procedures in the nested BLOCK construct but to not be procedures outside it.

This patch fixes the code that detects local declarations in BLOCK for all of the missed cases that don't use entity-decls; only INTRINSIC and EXTERNAL could affect the procedures whose names are of interest to the executable part skimmer, but perhaps future work will want to collect non-procedures as well, so I plugged all of the holes that I could find.

Fixes #115674.


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

2 Files Affected:

  • (modified) flang/lib/Semantics/resolve-names.cpp (+50-3)
  • (added) flang/test/Semantics/bug115674.f90 (+9)
diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index e0a8246ebc752e..4b8cd447bcf19f 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -4787,10 +4787,13 @@ void DeclarationVisitor::Post(const parser::EntityDecl &x) {
   } else if (attrs.test(Attr::PARAMETER)) { // C882, C883
     Say(name, "Missing initialization for parameter '%s'"_err_en_US);
   }
-  if (auto *scopeSymbol{currScope().symbol()})
-    if (auto *details{scopeSymbol->detailsIf<DerivedTypeDetails>()})
-      if (details->isDECStructure())
+  if (auto *scopeSymbol{currScope().symbol()}) {
+    if (auto *details{scopeSymbol->detailsIf<DerivedTypeDetails>()}) {
+      if (details->isDECStructure()) {
         details->add_component(symbol);
+      }
+    }
+  }
 }
 
 void DeclarationVisitor::Post(const parser::PointerDecl &x) {
@@ -7660,10 +7663,54 @@ class ExecutionPartSkimmerBase {
     --blockDepth_;
     PopScope();
   }
+  // Note declarations of local names in BLOCK constructs.
+  // Don't have to worry about INTENT(), VALUE, or OPTIONAL
+  // (pertinent only to dummy arguments), ASYNCHRONOUS/VOLATILE,
+  // or accessibility attributes,
   bool Pre(const parser::EntityDecl &x) {
     Hide(std::get<parser::ObjectName>(x.t));
     return true;
   }
+  bool Pre(const parser::ObjectDecl &x) {
+    Hide(std::get<parser::ObjectName>(x.t));
+    return true;
+  }
+  bool Pre(const parser::PointerDecl &x) {
+    Hide(std::get<parser::Name>(x.t));
+    return true;
+  }
+  bool Pre(const parser::BindEntity &x) {
+    Hide(std::get<parser::Name>(x.t));
+    return true;
+  }
+  bool Pre(const parser::ContiguousStmt &x) {
+    for (const parser::Name &name : x.v) {
+      Hide(name);
+    }
+    return true;
+  }
+  bool Pre(const parser::DimensionStmt::Declaration &x) {
+    Hide(std::get<parser::Name>(x.t));
+    return true;
+  }
+  bool Pre(const parser::ExternalStmt &x) {
+    for (const parser::Name &name : x.v) {
+      Hide(name);
+    }
+    return true;
+  }
+  bool Pre(const parser::IntrinsicStmt &x) {
+    for (const parser::Name &name : x.v) {
+      Hide(name);
+    }
+    return true;
+  }
+  bool Pre(const parser::CodimensionStmt &x) {
+    for (const parser::CodimensionDecl &decl : x.v) {
+      Hide(std::get<parser::Name>(decl.t));
+    }
+    return true;
+  }
   void Post(const parser::ImportStmt &x) {
     if (x.kind == common::ImportKind::None ||
         x.kind == common::ImportKind::Only) {
diff --git a/flang/test/Semantics/bug115674.f90 b/flang/test/Semantics/bug115674.f90
new file mode 100644
index 00000000000000..87c527bb4297a8
--- /dev/null
+++ b/flang/test/Semantics/bug115674.f90
@@ -0,0 +1,9 @@
+!RUN: %flang_fc1 -fsyntax-only %s 2>&1 | FileCheck --allow-empty %s
+!CHECK-NOT: error:
+program main
+  sin = 1
+  block
+    intrinsic sin
+    print *, sin(0.)
+  end block
+end

Copy link
Contributor

@clementval clementval left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@klausler klausler merged commit d68332d into llvm:main Nov 14, 2024
11 checks passed
@klausler klausler deleted the bug115674 branch November 14, 2024 22:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:semantics flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Flang] Compilation error when the variable sin is used as sin intrinsic function in block construct
3 participants