Skip to content

[flang] Accept a compiler directive sentinel after a semicolon #96966

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
Jun 28, 2024

Conversation

klausler
Copy link
Contributor

Don't treat !DIR$ or an active !$ACC, !$OMP, &c. as a comment when they appear after a semicolon, but instead treat them as a compiler directive sentinel.

@klausler klausler requested a review from razvanlupusoru June 27, 2024 20:55
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:parser labels Jun 27, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 27, 2024

@llvm/pr-subscribers-flang-parser

Author: Peter Klausler (klausler)

Changes

Don't treat !DIR$ or an active !$ACC, !$OMP, &c. as a comment when they appear after a semicolon, but instead treat them as a compiler directive sentinel.


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

4 Files Affected:

  • (modified) flang/lib/Parser/prescan.cpp (+28-26)
  • (modified) flang/lib/Parser/prescan.h (+3)
  • (modified) flang/lib/Parser/token-sequence.cpp (+4-7)
  • (added) flang/test/Preprocessing/sentinel-after-semi.F90 (+7)
diff --git a/flang/lib/Parser/prescan.cpp b/flang/lib/Parser/prescan.cpp
index 42aa829e0ed5b..d44bdde70f380 100644
--- a/flang/lib/Parser/prescan.cpp
+++ b/flang/lib/Parser/prescan.cpp
@@ -471,7 +471,7 @@ bool Prescanner::MustSkipToEndOfLine() const {
   if (inFixedForm_ && column_ > fixedFormColumnLimit_ && !tabInCurrentLine_) {
     return true; // skip over ignored columns in right margin (73:80)
   } else if (*at_ == '!' && !inCharLiteral_) {
-    return true; // inline comment goes to end of source line
+    return !IsCompilerDirectiveSentinel(at_);
   } else {
     return false;
   }
@@ -1345,32 +1345,12 @@ Prescanner::IsFixedFormCompilerDirectiveLine(const char *start) const {
 
 std::optional<Prescanner::LineClassification>
 Prescanner::IsFreeFormCompilerDirectiveLine(const char *start) const {
-  char sentinel[8];
-  const char *p{SkipWhiteSpace(start)};
-  if (*p++ != '!') {
-    return std::nullopt;
-  }
-  for (std::size_t j{0}; j + 1 < sizeof sentinel; ++p, ++j) {
-    if (*p == '\n') {
-      break;
-    }
-    if (*p == ' ' || *p == '\t' || *p == '&') {
-      if (j == 0) {
-        break;
-      }
-      sentinel[j] = '\0';
-      p = SkipWhiteSpace(p + 1);
-      if (*p == '!') {
-        break;
-      }
-      if (const char *sp{IsCompilerDirectiveSentinel(sentinel, j)}) {
-        std::size_t offset = p - start;
-        return {LineClassification{
-            LineClassification::Kind::CompilerDirective, offset, sp}};
-      }
-      break;
+  if (const char *p{SkipWhiteSpace(start)}; p && *p++ == '!') {
+    if (auto maybePair{IsCompilerDirectiveSentinel(p)}) {
+      auto offset{static_cast<std::size_t>(maybePair->second - start)};
+      return {LineClassification{LineClassification::Kind::CompilerDirective,
+          offset, maybePair->first}};
     }
-    sentinel[j] = ToLowerCaseLetter(*p);
   }
   return std::nullopt;
 }
@@ -1415,6 +1395,28 @@ const char *Prescanner::IsCompilerDirectiveSentinel(CharBlock token) const {
   return end > p && IsCompilerDirectiveSentinel(p, end - p) ? p : nullptr;
 }
 
+std::optional<std::pair<const char *, const char *>>
+Prescanner::IsCompilerDirectiveSentinel(const char *p) const {
+  char sentinel[8];
+  for (std::size_t j{0}; j + 1 < sizeof sentinel && *p != '\n'; ++p, ++j) {
+    if (*p == ' ' || *p == '\t' || *p == '&') {
+      if (j > 0) {
+        sentinel[j] = '\0';
+        p = SkipWhiteSpace(p + 1);
+        if (*p != '!') {
+          if (const char *sp{IsCompilerDirectiveSentinel(sentinel, j)}) {
+            return std::make_pair(sp, p);
+          }
+        }
+      }
+      break;
+    } else {
+      sentinel[j] = ToLowerCaseLetter(*p);
+    }
+  }
+  return std::nullopt;
+}
+
 constexpr bool IsDirective(const char *match, const char *dir) {
   for (; *match; ++match) {
     if (*match != ToLowerCaseLetter(*dir++)) {
diff --git a/flang/lib/Parser/prescan.h b/flang/lib/Parser/prescan.h
index 421ba97d324c9..a64df5377e7e0 100644
--- a/flang/lib/Parser/prescan.h
+++ b/flang/lib/Parser/prescan.h
@@ -74,6 +74,9 @@ class Prescanner {
 
   const char *IsCompilerDirectiveSentinel(const char *, std::size_t) const;
   const char *IsCompilerDirectiveSentinel(CharBlock) const;
+  // 'first' is the sentinel, 'second' is beginning of payload
+  std::optional<std::pair<const char *, const char *>>
+  IsCompilerDirectiveSentinel(const char *p) const;
 
   template <typename... A> Message &Say(A &&...a) {
     return messages_.Say(std::forward<A>(a)...);
diff --git a/flang/lib/Parser/token-sequence.cpp b/flang/lib/Parser/token-sequence.cpp
index 133e60ba4f009..394ac79ea3283 100644
--- a/flang/lib/Parser/token-sequence.cpp
+++ b/flang/lib/Parser/token-sequence.cpp
@@ -266,7 +266,7 @@ TokenSequence &TokenSequence::ClipComment(
     if (std::size_t blanks{tok.CountLeadingBlanks()};
         blanks < tok.size() && tok[blanks] == '!') {
       // Retain active compiler directive sentinels (e.g. "!dir$")
-      for (std::size_t k{j + 1}; k < tokens && tok.size() < blanks + 5; ++k) {
+      for (std::size_t k{j + 1}; k < tokens && tok.size() <= blanks + 5; ++k) {
         if (tok.begin() + tok.size() == TokenAt(k).begin()) {
           tok.ExtendToCover(TokenAt(k));
         } else {
@@ -274,12 +274,9 @@ TokenSequence &TokenSequence::ClipComment(
         }
       }
       bool isSentinel{false};
-      if (tok.size() == blanks + 5) {
-        char sentinel[4];
-        for (int k{0}; k < 4; ++k) {
-          sentinel[k] = ToLowerCaseLetter(tok[blanks + k + 1]);
-        }
-        isSentinel = prescanner.IsCompilerDirectiveSentinel(sentinel, 4);
+      if (tok.size() >= blanks + 5) {
+        isSentinel = prescanner.IsCompilerDirectiveSentinel(&tok[blanks + 1])
+                         .has_value();
       }
       if (isSentinel) {
       } else if (skipFirst) {
diff --git a/flang/test/Preprocessing/sentinel-after-semi.F90 b/flang/test/Preprocessing/sentinel-after-semi.F90
new file mode 100644
index 0000000000000..75060ac1db032
--- /dev/null
+++ b/flang/test/Preprocessing/sentinel-after-semi.F90
@@ -0,0 +1,7 @@
+! RUN: %flang_fc1 -fdebug-unparse -fopenacc %s 2>&1 | FileCheck %s
+! CHECK: !$ACC DECLARE COPYIN(var)
+#define ACCDECLARE(var) integer :: var; \
+  !$acc declare copyin(var)
+program main
+  ACCDECLARE(var)
+end program

sentinel[k] = ToLowerCaseLetter(tok[blanks + k + 1]);
}
isSentinel = prescanner.IsCompilerDirectiveSentinel(sentinel, 4);
if (tok.size() >= blanks + 5) {
Copy link
Contributor

Choose a reason for hiding this comment

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

tok.size() is compared with "<= blanks + 5" earlier and here it is compared with ">= blanks + 5". There is overlap due to the equality use. Is there an error here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really, but I will tweak it for clarity.

Copy link
Contributor

@razvanlupusoru razvanlupusoru left a comment

Choose a reason for hiding this comment

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

Thank you!

Don't treat !DIR$ or an active !$ACC, !$OMP, &c. as a comment when they
appear after a semicolon, but instead treat them as a compiler directive
sentinel.
@klausler klausler merged commit 259ce11 into llvm:main Jun 28, 2024
3 of 5 checks passed
@klausler klausler deleted the bug1673 branch June 28, 2024 19:16
lravenclaw pushed a commit to lravenclaw/llvm-project that referenced this pull request Jul 3, 2024
…96966)

Don't treat !DIR$ or an active !$ACC, !$OMP, &c. as a comment when they
appear after a semicolon, but instead treat them as a compiler directive
sentinel.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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