Skip to content

Conversation

@luporl
Copy link
Contributor

@luporl luporl commented Sep 6, 2024

The previous assert was not considering programs with semantic errors.

Fixes #107495
Fixes #93437

The previous assert was not considering programs with semantic errors.

Fixes llvm#107495
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:openmp flang:semantics labels Sep 6, 2024
@llvmbot
Copy link
Member

llvmbot commented Sep 6, 2024

@llvm/pr-subscribers-flang-openmp

@llvm/pr-subscribers-flang-semantics

Author: Leandro Lupori (luporl)

Changes

The previous assert was not considering programs with semantic errors.

Fixes #107495


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

1 Files Affected:

  • (modified) flang/lib/Semantics/resolve-directives.cpp (+3-1)
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 4aecb8b8e7b479..01ce8d989fdd15 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -2152,7 +2152,9 @@ void OmpAttributeVisitor::CreateImplicitSymbols(
         dirContext.defaultDSA == Symbol::Flag::OmpShared) {
       // 1) default
       // Allowed only with parallel, teams and task generating constructs.
-      assert(parallelDir || taskGenDir || teamsDir);
+      if (!parallelDir && !taskGenDir && !teamsDir) {
+        return;
+      }
       if (dirContext.defaultDSA != Symbol::Flag::OmpShared)
         makePrivateSymbol(dirContext.defaultDSA);
       else

@luporl
Copy link
Contributor Author

luporl commented Sep 6, 2024

Now that I noticed that this is practically the same fix as in #93438.

Copy link
Contributor

@tblah tblah left a comment

Choose a reason for hiding this comment

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

Looks good.

I see this now hits a semantic error instead: "DEFAULT clause is not allowed on the DO directive". There doesn't seem to be a unit test for this currently. Please could you add one so we can verify that it doesn't crash.

Copy link
Contributor

@mjklemm mjklemm left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@NimishMishra NimishMishra left a comment

Choose a reason for hiding this comment

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

Thanks @luporl . This looks good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

flang:openmp 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][OpenMP] Assertion `parallelDir || taskGenDir || teamsDir' failed. [flang][OpenMP] Assertion failure in incorrect use of default clause

5 participants