[flang] Support correct continuations for compiler directives

[unterumarmung]

Summary

If a line is over 72 characters long, flang's preprocessor cuts it there
and continues on the next line.
For this purpose it uses the standard way of continuing line with & on each line.
However, it doesn't work with long compiler directives, like OpenMP or OpenACC ones.
The line that continues the directive also has to
contain the corresponding sentinel at the beginning.

This change implements the described functionality.
Also, some code was refactored in order to simplify and reuse existing code.

[klausler]

Reviewers take note: this patch changes how some directive lines are *emitted* in the fake preprocessed output, *not* how they are processed in the prescanner.

flang/lib/Parser/parsing.cpp

126

Why?

197

Always use braced initialization in the parts of f18 that use modern C++.

[unterumarmung]

flang/lib/Parser/parsing.cpp

126

I extracted this code to a lambda from the existing code you've written a while ago.
Why I did this?

1. I thought it'd be nice to have the same-cased sentinel for the continuation as the original directive's sentinel, so I used this function for the implementation of this functionality. IIRC, it is not strictly necessary, though.
2. If we ignore the first point, IMHO extracting this code to a lambda increases readability of the code below.

Does this answer your question or there are things left to clarify?

197

Yes, sure.. It's hard for me to get used to always using the braced initialization. Will fix.

[unterumarmung]

Address review comment: use brace initialization

[klausler]

flang/lib/Parser/parsing.cpp

99

Why not use IsLetter()?

110

Don't use unsigned ints when regular ints will work just fine. Also, please use braces in the parts of f18 written in modern C++ -- braced initialization provides stronger safety checking. And this is a case where auto probably isn't the most readable choice.

130

Why not use ToUpperCaseLetter()?

197

Braced initialization is safer; thanks.

[unterumarmung]

Address review comments, fix encountered bug, add test case for the bug

flang/lib/Parser/parsing.cpp

99

Didn't know about this function, will fix

110

Forgot to change this one to the braced initialization, sorry
IDK why I chose unsigned int here - agree that simple int here is fine
Will fix!

130

This was your code initially, so I didn't change anything here intentionally. Also, I didn't know such function exists in the codebase.
The suggestion is nice, will fix!

[unterumarmung]

@klausler, could you please take another look to the revision?
I tested the change on an internal codebase and found a bug: exclamation marks in string literals were treated as compiler directive beginnings. So, this lead to wrong behavior when a string literal "exceeded" 72 character limit.
It wasn't the problem before because there were no special logic for compiler directives.
The error was not detected with the test suite, so I added a test case for such occasion.

[klausler]

I think that you need a test case with long CHARACTER literals and Hollerith literals that contain the comment marker ('!').

Or (as a suggestion) treat the comment marker ('!') as such only when it appears as the first non-blank character of an output line.

flang/lib/Parser/parsing.cpp

139

You also need to handle Hollerith literals.

[unterumarmung]

Changed the logic for identifying compiler directives, added Hollerith literals to the test

[unterumarmung]

In D126301#3534770, @klausler wrote:

I think that you need a test case with long CHARACTER literals and Hollerith literals that contain the comment marker ('!').

Yeah, totally forgot about it, thanks!

In D126301#3534770, @klausler wrote:

Or (as a suggestion) treat the comment marker ('!') as such only when it appears as the first non-blank character of an output line.

It is actually a nice suggestion! I thought about it too, but for some reason thought that it'd be harder to implement than the solution with quotation marks.
Adding to this handling Hollerith literals would be such a mess. So, I implemented your suggestion and it is as simple as it possibly can be.

[unterumarmung]

@klausler, I'm sorry, could you please take a look?

flang/lib/Parser/parsing.cpp

const SourceFile *Parsing::Prescan(const std::string &path, Options options) {
   }
   currentCooked_->Marshal(allCooked_);
   if (options.needProvenanceRangeToCharBlockMappings) {
     currentCooked_->CompileProvenanceRangeToOffsetMappings(allSources);
   }
   return sourceFile;
 }
 
 void Parsing::EmitPreprocessedSource(
     llvm::raw_ostream &out, bool lineDirectives) const {
   const SourceFile *sourceFile{nullptr};
   int sourceLine{0};
   int column{1};
   bool inDirective{false};
   bool inContinuation{false};
+  bool inCharacterLiteral{false};
+  char characterLiteralStart{};
   const AllSources &allSources{allCooked().allSources()};
+  // All directives that flang support are known to have a length of 3 chars
+  constexpr int directiveNameLength{3};
+  // We need to know the current directive in order to provide correct
+  // continuation for the directive
+  std::string directive;
   for (const char &atChar : cooked().AsCharBlock()) {
     char ch{atChar};
     if (ch == '\n') {
       out << '\n'; // TODO: DOS CR-LF line ending if necessary
       column = 1;
       inDirective = false;
       inContinuation = false;
       ++sourceLine;
+      directive.clear();
     } else {
-      if (ch == '!') {
+      auto provenance{cooked().GetProvenanceRange(CharBlock{&atChar, 1})};
+
+      // Preserves original case of the character
+      const auto getOriginalChar{[&](char ch) {
+        if (IsLetter(ch) && provenance && provenance->size() == 1) {
+          if (const char *orig{allSources.GetSource(*provenance)}) {
+            const char upper{ToUpperCaseLetter(ch)};
+            if (*orig == upper) {
+              return upper;
+            }
+          }
+        }
+        return ch;
+      }};
+
+      if (ch == '\'' || ch == '"') {
+        // We need to know, if the current character is belongs to a character
+        // literal, to be able to correctly identify if a character belongs to a
+        // compiler directive later. This is needed because an exclamation mark
+        // can be used in both string literals and compiler directives.
+        if (!inCharacterLiteral) {
+          inCharacterLiteral = true;
+          characterLiteralStart = ch;
+        } else if (ch == characterLiteralStart) {
+          inCharacterLiteral = false;
+        }
+      }
+
+      if (ch == '!' && !inCharacterLiteral) {
         // Other comment markers (C, *, D) in original fixed form source
         // input card column 1 will have been deleted or normalized to !,
         // which signifies a comment (directive) in both source forms.
         inDirective = true;
       }
-      auto provenance{cooked().GetProvenanceRange(CharBlock{&atChar, 1})};
+      if (inDirective && directive.size() < directiveNameLength &&
+          IsLetter(ch)) {
+        directive += getOriginalChar(ch);
+      }
+
       std::optional<SourcePosition> position{provenance
               ? allSources.GetSourcePosition(provenance->start())
               : std::nullopt};
       if (lineDirectives && column == 1 && position) {
         if (&position->file != sourceFile) {
           out << "#line \"" << position->file.path() << "\" " << position->line
               << '\n';
         } else if (position->line != sourceLine) {
if (ch == '\n') {
             out << "#line " << position->line << '\n';
           }
         }
         sourceFile = &position->file;
         sourceLine = position->line;
       }
       if (column > 72) {
         // Wrap long lines in a portable fashion that works in both
-        // of the Fortran source forms.  The first free-form continuation
+        // of the Fortran source forms. The first free-form continuation
         // marker ("&") lands in column 73, which begins the card commentary
         // field of fixed form, and the second one is put in column 6,
         // where it signifies fixed form line continuation.
         // The standard Fortran fixed form column limit (72) is used
         // for output, even if the input was parsed with a nonstandard
         // column limit override option.
-        out << "&\n     &";
+        // OpenMP and OpenACC directives' continuations should have the
+        // corresponding sentinel at the next line.
+        const auto continuation{
+            inDirective ? "&\n!$" + directive + "&" : "&\n     &"s};
+        out << continuation;
         column = 7; // start of fixed form source field
         ++sourceLine;
         inContinuation = true;
       } else if (!inDirective && ch != ' ' && (ch < '0' || ch > '9')) {
         // Put anything other than a label or directive into the
         // Fortran fixed form source field (columns [7:72]).
         for (; column < 7; ++column) {
           out << ' ';
         }
       }
       if (!inContinuation && position && position->column <= 72 && ch != ' ') {
         // Preserve original indentation
         for (; column < position->column; ++column) {
           out << ' ';
         }
       }
-      if (ch >= 'a' && ch <= 'z' && provenance && provenance->size() == 1) {
-        // Preserve original case
-        if (const char *orig{allSources.GetSource(*provenance)}) {
-          auto upper{static_cast<char>(ch + 'A' - 'a')};
-          if (*orig == upper) {
-            ch = upper;
-          }
-        }
-      }
-      out << ch;
+      out << getOriginalChar(ch);
       ++column;
     }
   }
 }
 
 void Parsing::DumpCookedChars(llvm::raw_ostream &out) const {
   UserState userState{allCooked_, common::LanguageFeatureControl{}};
   ParseState parseState{cooked()};

flang/test/Preprocessing/pp132.f90

+! RUN: %flang -E -fopenmp -fopenacc %s 2>&1 | FileCheck %s
+! CHECK:  !$OMP parallel default(shared) private(super_very_long_name_for_the_va&
+! CHECK:  !$OMP&riable)
+! CHECK:  !$acc data copyin(super_very_long_name_for_the_variable, another_super&
+! CHECK:  !$acc&_wordy_variable_to_test)
+! Test correct continuations in compiler directives
+subroutine foo
+  integer :: super_very_long_name_for_the_variable
+  integer :: another_super_wordy_variable_to_test
+
+  super_very_long_name_for_the_variable = 42
+  another_super_wordy_variable_to_test = super_very_long_name_for_the_variable * 2
+  !$OMP parallel default(shared) private(super_very_long_name_for_the_variable)
+  !$omp end parallel
+
+  !$acc data copyin(super_very_long_name_for_the_variable, another_super_wordy_variable_to_test)
+  !$acc end data
+end subroutine foo

flang/test/Preprocessing/pp133.f90

+! RUN: %flang -E %s 2>&1 | FileCheck %s
+! CHECK: character(*), parameter :: message = "This is going to be a really&
+! CHECK: & long message to the whole world that should be trimmed into 72 ch&
+! CHECK: &aracters! Also, it must contain single quotation mark - ""'"" - ju&
+! CHECK: &st to make sure it works correctly"
+! Test correct continuations in string literals
+subroutine foo
+  character(*), parameter :: message = "This is going to be a really long message to the whole world that should be trimmed into 72 characters! Also, it must contain single quotation mark - ""'"" - just to make sure it works correctly"
+end subroutine foo