Skip to content

Conversation

@klausler
Copy link
Contributor

The logic for fixed form compiler directive line continuation has a hole that can apply continuation for !$ even if the next line does not begin with a fixed form comment character. Rearrange the nested if statements to enforce that requirement for all compiler directives.

The logic for fixed form compiler directive line continuation has a
hole that can apply  continuation for !$ even if the next line does not
begin with a fixed form comment character.  Rearrange the nested if statements
to enforce that requirement for all compiler directives.
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:openmp flang:parser labels Apr 11, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 11, 2025

@llvm/pr-subscribers-flang-parser

@llvm/pr-subscribers-flang-openmp

Author: Peter Klausler (klausler)

Changes

The logic for fixed form compiler directive line continuation has a hole that can apply continuation for !$ even if the next line does not begin with a fixed form comment character. Rearrange the nested if statements to enforce that requirement for all compiler directives.


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

2 Files Affected:

  • (modified) flang/lib/Parser/prescan.cpp (+10-12)
  • (added) flang/test/Parser/OpenMP/bug518.f (+24)
diff --git a/flang/lib/Parser/prescan.cpp b/flang/lib/Parser/prescan.cpp
index 43a6d8c76f067..c18142f00d46d 100644
--- a/flang/lib/Parser/prescan.cpp
+++ b/flang/lib/Parser/prescan.cpp
@@ -1309,22 +1309,22 @@ const char *Prescanner::FixedFormContinuationLine(bool mightNeedSpace) {
   tabInCurrentLine_ = false;
   char col1{*nextLine_};
   if (InCompilerDirective()) {
-    if (directiveSentinel_[0] == '$' && directiveSentinel_[1] == '\0') {
+    if (!IsFixedFormCommentChar(col1)) {
+      return nullptr;
+    } else if (directiveSentinel_[0] == '$' && directiveSentinel_[1] == '\0') {
       // !$ OpenMP conditional compilation
       if (preprocessingOnly_) {
         // in -E mode, don't treat "!$   &" as a continuation
         return nullptr;
-      } else if (IsFixedFormCommentChar(col1)) {
-        if (nextLine_[1] == '$') {
-          // accept but do not require a matching sentinel
-          if (!(nextLine_[2] == '&' || IsSpaceOrTab(&nextLine_[2]))) {
-            return nullptr;
-          }
-        } else {
-          return nullptr; // distinct directive
+      } else if (nextLine_[1] == '$') {
+        // accept but do not require a matching sentinel
+        if (!(nextLine_[2] == '&' || IsSpaceOrTab(&nextLine_[2]))) {
+          return nullptr;
         }
+      } else {
+        return nullptr; // distinct directive
       }
-    } else if (IsFixedFormCommentChar(col1)) {
+    } else { // all other directives
       int j{1};
       for (; j < 5; ++j) {
         char ch{directiveSentinel_[j - 1]};
@@ -1339,8 +1339,6 @@ const char *Prescanner::FixedFormContinuationLine(bool mightNeedSpace) {
           return nullptr;
         }
       }
-    } else {
-      return nullptr;
     }
     const char *col6{nextLine_ + 5};
     if (*col6 != '\n' && *col6 != '0' && !IsSpaceOrTab(col6)) {
diff --git a/flang/test/Parser/OpenMP/bug518.f b/flang/test/Parser/OpenMP/bug518.f
new file mode 100644
index 0000000000000..2dbacef59fa8a
--- /dev/null
+++ b/flang/test/Parser/OpenMP/bug518.f
@@ -0,0 +1,24 @@
+! RUN: %flang_fc1 -E %s 2>&1 | FileCheck %s --strict-whitespace --check-prefix=CHECK-E
+! RUN: %flang_fc1 -fopenmp -fdebug-unparse %s 2>&1 | FileCheck %s --check-prefix=CHECK-OMP
+! RUN: %flang_fc1 -fdebug-unparse %s 2>&1 | FileCheck %s --check-prefix=CHECK-NO-OMP
+
+!$    thread = OMP_GET_MAX_THREADS()
+
+!$omp parallel private(ia)
+!$    continue
+!$omp end parallel
+      end
+
+!CHECK-E:{{^}}!$     thread = OMP_GET_MAX_THREADS()
+!CHECK-E:{{^}}!$omp parallel private(ia)
+!CHECK-E:{{^}}!$     continue
+!CHECK-E:{{^}}!$omp end parallel
+
+!CHECK-OMP:thread=omp_get_max_threads()
+!CHECK-OMP:!$OMP PARALLEL  PRIVATE(ia)
+!CHECK-OMP: CONTINUE
+!CHECK-OMP:!$OMP END PARALLEL 
+
+!CHECK-NO-OMP-NOT:thread=
+!CHECK-NO-OMP-NOT:!$OMP
+!CHECK-NO-OMP:END PROGRAM

@klausler klausler requested a review from clementval April 11, 2025 20:42
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 8822006 into llvm:main Apr 14, 2025
15 checks passed
@klausler klausler deleted the bug519 branch April 14, 2025 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

flang:openmp flang:parser flang Flang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants