[flang] Handle continuation line edge case #74751

klausler · 2023-12-07T19:21:29Z

For a character literal that is split over more than one source line with free form line continuation using '&'
at the end of one line but missing the standard-required '&' on the continuation line, also handle the case of spaces at the beginning of the continuation line.

For example,

PRINT *, 'don'&
't poke the bear'

now prints "don't poke the bear", like nearly all other Fortran compilers do.

This is not strictly standard conforming behavior, and the compiler emits a portability warning with -pedantic.

Fixes llvm-test-suite/Fortran/gfortran/regression/continuation_1.f90, .../continuation_12.f90, and .../continuation_13.f90.

llvmbot · 2023-12-07T19:22:01Z

@llvm/pr-subscribers-flang-parser

Author: Peter Klausler (klausler)

Changes

For repeated quote marks in a character literal that are separated by free form line continuation without a leading ampersand character on the continuation line, also handle the case of spaces at the beginning of the continuation line.

For example,

PRINT *, 'don'&
't poke the bear'

now prints "don't poke the bear", like nearly all other Fortran compilers do.

This is not strictly standard conforming behavior, and the compiler emits a portability warning with -pedantic.

Fixes llvm-test-suite/Fortran/gfortran/regression/continuation_12.f90 and .../continuation_13.f90.

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

3 Files Affected:

(modified) flang/lib/Parser/prescan.cpp (+11-9)
(modified) flang/lib/Parser/prescan.h (+2-1)
(modified) flang/test/Parser/continuation-before-quote.f90 (+3)

diff --git a/flang/lib/Parser/prescan.cpp b/flang/lib/Parser/prescan.cpp
index 449ea60144424a..ac68b54a50ca18 100644
--- a/flang/lib/Parser/prescan.cpp
+++ b/flang/lib/Parser/prescan.cpp
@@ -706,6 +706,7 @@ void Prescanner::QuotedCharacterLiteral(
   char quote{*at_};
   const char *end{at_ + 1};
   inCharLiteral_ = true;
+  charLiteralQuote_ = quote;
   const auto emit{[&](char ch) { EmitChar(tokens, ch); }};
   const auto insert{[&](char ch) { EmitInsertedChar(tokens, ch); }};
   bool isEscaped{false};
@@ -749,17 +750,10 @@ void Prescanner::QuotedCharacterLiteral(
         break;
       }
       inCharLiteral_ = true;
-      if (insertASpace_) {
-        if (features_.ShouldWarn(
-                common::LanguageFeature::MiscSourceExtensions)) {
-          Say(GetProvenanceRange(at_, end),
-              "Repeated quote mark in character literal continuation line should have been preceded by '&'"_port_en_US);
-        }
-        insertASpace_ = false;
-      }
     }
   }
   inCharLiteral_ = false;
+  charLiteralQuote_.reset();
 }
 
 void Prescanner::Hollerith(
@@ -1122,7 +1116,15 @@ const char *Prescanner::FreeFormContinuationLine(bool ampersand) {
     } else if (*p == '!' || *p == '\n' || *p == '#') {
       return nullptr;
     } else if (ampersand || IsImplicitContinuation()) {
-      if (p > nextLine_) {
+      if (charLiteralQuote_ && *p == *charLiteralQuote_) {
+        // 'a'&            -> 'a''b' == "a'b"
+        //   'b'
+        if (features_.ShouldWarn(
+                common::LanguageFeature::MiscSourceExtensions)) {
+          Say(GetProvenanceRange(p, p + 1),
+              "Repeated quote mark in character literal continuation line should have been preceded by '&'"_port_en_US);
+        }
+      } else if (p > nextLine_) {
         --p;
       } else {
         insertASpace_ = true;
diff --git a/flang/lib/Parser/prescan.h b/flang/lib/Parser/prescan.h
index 16b2c6165f611c..efeda592813ea7 100644
--- a/flang/lib/Parser/prescan.h
+++ b/flang/lib/Parser/prescan.h
@@ -217,7 +217,8 @@ class Prescanner {
   bool tabInCurrentLine_{false};
   bool slashInCurrentStatement_{false};
   bool preventHollerith_{false}; // CHARACTER*4HIMOM not Hollerith
-  bool inCharLiteral_{false};
+  bool inCharLiteral_;
+  std::optional<char> charLiteralQuote_;
   bool inPreprocessorDirective_{false};
 
   // In some edge cases of compiler directive continuation lines, it
diff --git a/flang/test/Parser/continuation-before-quote.f90 b/flang/test/Parser/continuation-before-quote.f90
index 66252010d89c46..1cc44d29c87463 100644
--- a/flang/test/Parser/continuation-before-quote.f90
+++ b/flang/test/Parser/continuation-before-quote.f90
@@ -4,6 +4,9 @@ subroutine test
 !CHECK: portability: Repeated quote mark in character literal continuation line should have been preceded by '&'
   print *, 'needs an '&
 'ampersand'''
+!CHECK: portability: Repeated quote mark in character literal continuation line should have been preceded by '&'
+  print *, 'also needs an '&
+ 'ampersand'''
 !CHECK-NOT: portability: Repeated quote mark in character literal continuation line should have been preceded by '&'
   print *, 'has an '&
 &'ampersand'''

psteinfeld

I'm seeing failing tests with these changes. Here's the list:

Failed Tests (14):
  Flang :: Lower/OpenMP/FIR/unstructured.f90
  Flang :: Lower/OpenMP/unstructured.f90
  Flang :: Lower/array-expression.f90
  Flang :: Lower/assigned-goto.f90
  Flang :: Lower/associate-construct.f90
  Flang :: Lower/block.f90
  Flang :: Lower/computed-goto.f90
  Flang :: Lower/irreducible.f90
  Flang :: Lower/loops.f90
  Flang :: Lower/select-case-statement.f90
  Flang :: Lower/submodule.f90
  Flang :: Lower/transformational-intrinsics.f90
  Flang :: Semantics/allocate09.f90
  Flang :: Semantics/typeinfo01.f90

Here's an excerpt from the log file for the first one:

Command Output (stderr):
--
RUN: at line 3: bbc /local/home/psteinfeld/main/klausler/flang/test/Lower/OpenMP/FIR/unstructured.f90 -fopenmp -emit-fir -hlfir=false -o "-" | /local/home/psteinfeld/main/klausler/build/bin/FileCheck /local/home/psteinfeld/main/klausler/flang/test/Lower/OpenMP/FIR/unstructured.f90
+ bbc /local/home/psteinfeld/main/klausler/flang/test/Lower/OpenMP/FIR/unstructured.f90 -fopenmp -emit-fir -hlfir=false -o -
+ /local/home/psteinfeld/main/klausler/build/bin/FileCheck /local/home/psteinfeld/main/klausler/flang/test/Lower/OpenMP/FIR/unstructured.f90
bbc: could not scan /local/home/psteinfeld/main/klausler/flang/test/Lower/OpenMP/FIR/unstructured.f90
/local/home/psteinfeld/main/klausler/flang/test/Lower/OpenMP/FIR/unstructured.f90:21:19: error: bad character ('!') in Fortran token
  subroutine ss1(n) ! unstructured code followed by a structured OpenMP construct
                    ^
FileCheck error: '<stdin>' is empty.
FileCheck command line:  /local/home/psteinfeld/main/klausler/build/bin/FileCheck /local/home/psteinfeld/main/klausler/flang/test/Lower/OpenMP/FIR/unstructured.f90

klausler · 2023-12-07T21:15:35Z

Everything passes for me -- I always run tests for every build -- and it's basically impossible these tests could have been broken by a prescanner fix.

EDIT: These are tests that my builds may not run. Will investigate further.

psteinfeld · 2023-12-07T21:27:07Z

Everything passes for me -- I always run tests for every build -- and it's basically impossible these tests could have been broken by a prescanner fix.

I was surprised, too. So I did a build from scratch and ran check-flang with just the files from the main branch of llvm-project. Everything looked good. I then copied in just the changed versions prescan.h and prescan.cpp, rebuilt the compiler, and re-ran check-flang. It failed the same way. I then reverted prescan.h and prescan.cpp to the versions in the main branch, rebuilt, and re-ran check-flang. Everything passed again.

You can take a look at my builds on ice4. My build from your repository is in /local/home/psteinfeld/main/klausler. My build with the files from llvm-project/main with just the changed prescan.h and prescan.cpp is in /local/home/psteinfeld/up.

Or, if there's an additional test you'd like me to run, I'd be glad to do that, too.

psteinfeld · 2023-12-07T21:45:57Z

Note that I've done two different builds -- one from a cloned version of your workspace and the other from llvm-project/main with just the two prescan files changed. In both cases, I get several failures from check-flang, but the failures are different.

If it makes a difference, I'm building with GCC 9.3.0.

klausler · 2023-12-07T21:47:41Z

Note that I've done two different builds -- one from a cloned version of your workspace and the other from llvm-project/main with just the two prescan files changed. In both cases, I get several failures from check-flang, but the failures are different.

If it makes a difference, I'm building with GCC 9.3.0.

That gives me an idea. Let me get back to you on this. Thanks for all your time thus far.

For a character literal that is split over more than one source line with free form line continuation using '&' at the end of one line but missing the standard-required '&' on the continuation line, also handle the case of spaces at the beginning of the continuation line. For example, PRINT *, 'don'& 't poke the bear' now prints "don't poke the bear", like nearly all other Fortran compilers do. This is not strictly standard conforming behavior, and the compiler emits a portability warning with -pedantic. Fixes llvm-test-suite/Fortran/gfortran/regression/continuation_1.f90, .../continuation_12.f90, and .../continuation_13.f90.

psteinfeld

This latest version builds and tests correctly.

Thanks for the quick fix!

klausler requested a review from psteinfeld December 7, 2023 19:21

llvmbot added flang Flang issues not falling into any other category flang:parser labels Dec 7, 2023

klausler force-pushed the continuation branch from 5f064bc to ef3acb9 Compare December 7, 2023 19:56

psteinfeld requested changes Dec 7, 2023

View reviewed changes

klausler force-pushed the continuation branch from ef3acb9 to 13ff0e0 Compare December 7, 2023 22:55

klausler force-pushed the continuation branch from 13ff0e0 to c71eb11 Compare December 7, 2023 23:32

psteinfeld approved these changes Dec 8, 2023

View reviewed changes

klausler merged commit ea3a3b2 into llvm:main Dec 11, 2023

klausler deleted the continuation branch December 11, 2023 21:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[flang] Handle continuation line edge case #74751

[flang] Handle continuation line edge case #74751

Uh oh!

klausler commented Dec 7, 2023 •

edited

Loading

Uh oh!

llvmbot commented Dec 7, 2023

Uh oh!

psteinfeld left a comment

Uh oh!

klausler commented Dec 7, 2023 •

edited

Loading

Uh oh!

psteinfeld commented Dec 7, 2023

Uh oh!

psteinfeld commented Dec 7, 2023

Uh oh!

klausler commented Dec 7, 2023

Uh oh!

psteinfeld left a comment

Uh oh!

Uh oh!

[flang] Handle continuation line edge case #74751

[flang] Handle continuation line edge case #74751

Uh oh!

Conversation

klausler commented Dec 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Dec 7, 2023

Uh oh!

psteinfeld left a comment

Choose a reason for hiding this comment

Uh oh!

klausler commented Dec 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

psteinfeld commented Dec 7, 2023

Uh oh!

psteinfeld commented Dec 7, 2023

Uh oh!

klausler commented Dec 7, 2023

Uh oh!

psteinfeld left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

klausler commented Dec 7, 2023 •

edited

Loading

klausler commented Dec 7, 2023 •

edited

Loading