Skip to content

Conversation

@klausler
Copy link
Contributor

@klausler klausler commented Jun 30, 2025

…ion line

An obsolete flag ("insertASpace_") is being used to signal some cases in the prescanner's implementation of continuation lines when a token should be broken when it straddles a line break. It turns out that it's sufficient to simply note these cases without ever actually inserting a space, so don't do that (fixing the motivating bug). This leaves some variables with obsolete names, so change them as well.

This patch handles the third of the three bugs reported in #146362 .

@llvmbot llvmbot added flang Flang issues not falling into any other category flang:parser labels Jun 30, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 30, 2025

@llvm/pr-subscribers-flang-parser

Author: Peter Klausler (klausler)

Changes

…ion line

A single flag ("insertASpace_") is being used to signal multiple conditions of interest in the prescanner's implementation of continuation lines. But sometimes we need to know that a single token can't straddle a line continuation without also inserting a space into it, so add a second flag to the prescanner for these cases.

This patch handles the third of the three bugs reported in #146362 .


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

7 Files Affected:

  • (modified) flang/lib/Parser/prescan.cpp (+9-3)
  • (modified) flang/lib/Parser/prescan.h (+11-1)
  • (added) flang/test/Preprocessing/bug1077.F90 (+7)
  • (modified) flang/test/Preprocessing/pp111.F90 (+1-1)
  • (modified) flang/test/Preprocessing/pp112.F90 (+1-1)
  • (modified) flang/test/Preprocessing/pp115.F90 (+1-1)
  • (modified) flang/test/Preprocessing/pp116.F90 (+1-1)
diff --git a/flang/lib/Parser/prescan.cpp b/flang/lib/Parser/prescan.cpp
index ed5184b0aa13d..979a29b6c7309 100644
--- a/flang/lib/Parser/prescan.cpp
+++ b/flang/lib/Parser/prescan.cpp
@@ -642,6 +642,7 @@ void Prescanner::SkipSpaces() {
     NextChar();
   }
   insertASpace_ = false;
+  brokenToken_ = false;
 }
 
 const char *Prescanner::SkipWhiteSpace(const char *p) {
@@ -747,8 +748,10 @@ bool Prescanner::NextToken(TokenSequence &tokens) {
   }
   if (insertASpace_) {
     tokens.PutNextTokenChar(' ', spaceProvenance_);
+    tokens.CloseToken();
     insertASpace_ = false;
   }
+  brokenToken_ = false;
   if (*at_ == '\n') {
     return false;
   }
@@ -808,7 +811,7 @@ bool Prescanner::NextToken(TokenSequence &tokens) {
     bool anyDefined{false};
     bool hadContinuation{false};
     // Subtlety: When an identifier is split across continuation lines,
-    // its parts are kept as distinct pp-tokens if that macro replacement
+    // its parts are kept as distinct pp-tokens if macro replacement
     // should operate on them independently.  This trick accommodates the
     // historic practice of using line continuation for token pasting after
     // replacement.
@@ -822,6 +825,9 @@ bool Prescanner::NextToken(TokenSequence &tokens) {
       ++at_, ++column_;
       hadContinuation = SkipToNextSignificantCharacter();
       if (hadContinuation && IsLegalIdentifierStart(*at_)) {
+        if (insertASpace_ || brokenToken_) {
+          break;
+        }
         // Continued identifier
         tokens.CloseToken();
         ++parts;
@@ -1464,7 +1470,7 @@ const char *Prescanner::FreeFormContinuationLine(bool ampersand) {
     p = SkipWhiteSpace(p);
     if (*p == '&') {
       if (!ampersand) {
-        insertASpace_ = true;
+        brokenToken_ = true;
       }
       return p + 1;
     } else if (ampersand) {
@@ -1494,7 +1500,7 @@ const char *Prescanner::FreeFormContinuationLine(bool ampersand) {
     } else if (p > lineStart && IsSpaceOrTab(p - 1)) {
       --p;
     } else {
-      insertASpace_ = true;
+      brokenToken_ = true;
     }
     return p;
   } else {
diff --git a/flang/lib/Parser/prescan.h b/flang/lib/Parser/prescan.h
index ec4c53cf3e0f2..79c943eb75828 100644
--- a/flang/lib/Parser/prescan.h
+++ b/flang/lib/Parser/prescan.h
@@ -258,9 +258,19 @@ class Prescanner {
 
   // In some edge cases of compiler directive continuation lines, it
   // is necessary to treat the line break as a space character by
-  // setting this flag, which is cleared by EmitChar().
+  // setting this flag, which is cleared by NextToken().
   bool insertASpace_{false};
 
+  // True after processing a free form continuation that can't be allowed
+  // to appear in the middle of an identifier token, but doesn't have a space
+  // character handy to use as a separator, and can't have a space inserted
+  // via "insertASpace_":
+  // a) (standard) doesn't begin with a leading '&' on the continuation
+  //     line, but has a non-blank in column 1, or
+  // b) (extension) does have a leading '&', but didn't have one
+  //    on the continued line.
+  bool brokenToken_{false};
+
   // When a free form continuation marker (&) appears at the end of a line
   // before a INCLUDE or #include, we delete it and omit the newline, so
   // that the first line of the included file is truly a continuation of
diff --git a/flang/test/Preprocessing/bug1077.F90 b/flang/test/Preprocessing/bug1077.F90
new file mode 100644
index 0000000000000..dd7391813a357
--- /dev/null
+++ b/flang/test/Preprocessing/bug1077.F90
@@ -0,0 +1,7 @@
+!RUN: %flang -E %s 2>&1 | FileCheck %s
+!CHECK: print *,((1)+(2)),4
+#define foo(x,y) ((x)+(y))
+print *,&
+foo(1,2)&
+,4
+end
diff --git a/flang/test/Preprocessing/pp111.F90 b/flang/test/Preprocessing/pp111.F90
index 4da45ef35f5c0..bbf8709c3ab15 100644
--- a/flang/test/Preprocessing/pp111.F90
+++ b/flang/test/Preprocessing/pp111.F90
@@ -1,5 +1,5 @@
 ! RUN: %flang -E %s 2>&1 | FileCheck %s
-! CHECK: res = IFLM (666)
+! CHECK: res = IFLM(666)
 ! FLM call name split across continuation, no leading &, with & ! comment
       integer function IFLM(x)
         integer :: x
diff --git a/flang/test/Preprocessing/pp112.F90 b/flang/test/Preprocessing/pp112.F90
index 16705527f68c3..a5244410f31af 100644
--- a/flang/test/Preprocessing/pp112.F90
+++ b/flang/test/Preprocessing/pp112.F90
@@ -1,5 +1,5 @@
 ! RUN: %flang -E %s 2>&1 | FileCheck %s
-! CHECK: res = IFLM (666)
+! CHECK: res = IFLM(666)
 ! ditto, but without & ! comment
       integer function IFLM(x)
         integer :: x
diff --git a/flang/test/Preprocessing/pp115.F90 b/flang/test/Preprocessing/pp115.F90
index 4e4c621110ed8..eea42c53b936d 100644
--- a/flang/test/Preprocessing/pp115.F90
+++ b/flang/test/Preprocessing/pp115.F90
@@ -1,5 +1,5 @@
 ! RUN: %flang -E %s 2>&1 | FileCheck %s
-! CHECK: res = IFLM (666)
+! CHECK: res = ((666)+111)
 ! ditto, with & ! comment, no leading &
       integer function IFLM(x)
         integer :: x
diff --git a/flang/test/Preprocessing/pp116.F90 b/flang/test/Preprocessing/pp116.F90
index e35a13cbf6489..39edf95763eab 100644
--- a/flang/test/Preprocessing/pp116.F90
+++ b/flang/test/Preprocessing/pp116.F90
@@ -1,5 +1,5 @@
 ! RUN: %flang -E %s 2>&1 | FileCheck %s
-! CHECK: res = IFLM (666)
+! CHECK: res = ((666)+111)
 ! FLM call split between name and (, no leading &
       integer function IFLM(x)
         integer :: x

@klausler klausler force-pushed the bug1077 branch 2 times, most recently from e58e9e6 to 15a86d3 Compare June 30, 2025 23:46
…ion line

An obsolete flag ("insertASpace_") is being used to signal some
cases in the prescanner's implementation of continuation lines
when a token should be broken when it straddles a line break.
It turns out that it's sufficient to simply note these cases
without ever actually inserting a space, so don't do that
(fixing the motivating bug).  This leaves some variables with
obsolete names, so change them as well.

This patch handles the third of the three bugs reported in
llvm#146362 .
@klausler klausler merged commit dd3214d into llvm:main Jul 3, 2025
7 checks passed
@klausler klausler deleted the bug1077 branch July 3, 2025 21:32
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.

4 participants