[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

128

Why?

186

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

[unterumarmung]

flang/lib/Parser/parsing.cpp

128

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?

186

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()?

112

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.

132

Why not use ToUpperCaseLetter()?

186

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

112

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!

132

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

156

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;
 }
 
+constexpr bool IsAlpha(const char ch) {
+  return (ch >= 'a' && ch <= 'z') || (ch >= 'A' && ch <= 'Z');
+}
+
 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};
   const AllSources &allSources{allCooked().allSources()};
+  // All directives that flang support are known to have a length of 3 chars
+  constexpr auto directiveNameLength = 3u;
+  // 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 {
+      auto provenance{cooked().GetProvenanceRange(CharBlock{&atChar, 1})};
+
+      // Preserves original case of the character
+      const auto getOriginalChar{[&](char ch) {
+        if (IsAlpha(ch) && provenance && provenance->size() == 1) {
+          if (const char *orig{allSources.GetSource(*provenance)}) {
+            auto upper{static_cast<char>(ch + 'A' - 'a')};
+            if (*orig == upper) {
+              return upper;
+            }
+          }
+        }
+        return ch;
+      }};
+
       if (ch == '!') {
         // 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 &&
+          IsAlpha(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 (sourceLine < position->line &&
               sourceLine + 10 >= position->line) {
             // Emit a few newlines to catch up when they'll likely
             // require fewer bytes than a #line directive would have
             // occupied.
             while (sourceLine++ < position->line) {
               out << '\n';
             }
           } else {
             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