[flang][Preprocessor] Constrain a bit more implicit continuations

[rogfer01]

Summary

D155499 fixed an issue with implicit continuations. The fixes included a nested parenthesis check during definition of a macro which is then carried over in the scanner state.

This leads to the following corner case to fail:

subroutine foo(a, d)
  implicit none
  integer :: a
  integer :: d

#define sub(x, y) foo2(x, y)
#define bar )

   ! An implicit continuation won't be considered unless
   ! the definition of "bar" above is removed/commented
   call sub(1,
     2)
end subroutine foo

The definition of bar is indeed unbalanced but it is not even used in the code, so it should not impact whether we apply implicit continuation in the expansion of sub.

This change aims at addressing this issue by removing the balance check and constraining a bit more when we consider implicit continuations: only when we see a left parenthesis after a function-like macro, not a object-like macro. In this case I think it is OK to (unconditionally) implicitly continue to the next line in search of the corresponding right parenthesis. This is, to my understanding, similar to what the C preprocessor would do according to the description in [1].

[1] https://www.spinellis.gr/blog/20060626/

[klausler]

This patch would cause the original motivating test case to fail again. In the new test, call sub(1, 2 bar would fail.

[rogfer01]

The original testcase was not expanding a function-like macro.

When expanding a function-like macro we need to find an unexpanded ).

subroutine foo()
  implicit none

#define sub(x, y) foo2(x, y)
#define bar )

   call sub(1, 2 bar
   print *, "hello"
end subroutine foo

This will be preprocessed as

#line "./t.F90" 1
      subroutine foo()
      implicit none




      call sub(1, 2 ) print *, "hello" end subroutine foo

Which is nonsensical. I'll see if we can emit a diagnostic in this case because we didn't find any `)' when expanding a function-like macro.

Note that something odd like

subroutine foo()
  implicit none

#define sub(x, y) foo2(x, y)
#define bar )

   call sub(1, 2 bar
   ) print *, "hello"
end subroutine foo

gives a different diagnostic

error: Could not scan t.F90
./t.F90:7:9: error: Unmatched ')'
     call sub(1, 2 bar
          ^^^^^^^^^^^^
./t.F90:4:19: in a macro defined here
  #define sub(x, y) foo2(x, y)
                    ^^^^^
that expanded to:
  foo2(1,  2 ))
              ^

because we do find the ) and then we expand the arguments (including the bar).

Does this seem reasonable to you?

flang/lib/Parser/preprocessor.h

public:
 
   const AllSources &allSources() const { return allSources_; }
   AllSources &allSources() { return allSources_; }
 
   void DefineStandardMacros();
   void Define(std::string macro, std::string value);
   void Undefine(std::string macro);
   bool IsNameDefined(const CharBlock &);
+  bool IsFunctionLikeDefinition(const CharBlock &);
 
   std::optional<TokenSequence> MacroReplacement(
       const TokenSequence &, Prescanner &);
 
   // Implements a preprocessor directive.
   void Directive(const TokenSequence &, Prescanner &);
 
-  bool anyMacroWithUnbalancedParentheses() const {
-    return anyMacroWithUnbalancedParentheses_;
-  }
-
 private:
   enum class IsElseActive { No, Yes };
   enum class CanDeadElseAppear { No, Yes };
 
   CharBlock SaveTokenAsName(const CharBlock &);
   TokenSequence ReplaceMacros(const TokenSequence &, Prescanner &);
   void SkipDisabledConditionalCode(
       const std::string &, IsElseActive, Prescanner &, ProvenanceRange);
   bool IsIfPredicateTrue(const TokenSequence &expr, std::size_t first,
       std::size_t exprTokens, Prescanner &);
   void LineDirective(const TokenSequence &, std::size_t, Prescanner &);
-  void CheckForUnbalancedParentheses(
-      const TokenSequence &, std::size_t first, std::size_t tokens);
 
   AllSources &allSources_;
   std::list<std::string> names_;
   std::unordered_map<CharBlock, Definition> definitions_;
   std::stack<CanDeadElseAppear> ifStack_;
-  bool anyMacroWithUnbalancedParentheses_{false};
 };
 } // namespace Fortran::parser
 #endif // FORTRAN_PARSER_PREPROCESSOR_H_

flang/lib/Parser/preprocessor.cpp

if (++j < tokens && dir.TokenAt(j).OnlyNonBlank() == '(') {
         if (std::set<std::string>(argName.begin(), argName.end()).size() !=
             argName.size()) {
           prescanner.Say(dir.GetTokenProvenance(dirOffset),
               "#define: argument names are not distinct"_err_en_US);
           return;
         }
       }
       j = dir.SkipBlanks(j + 1);
-      CheckForUnbalancedParentheses(dir, j, tokens - j);
       definitions_.emplace(std::make_pair(
           nameToken, Definition{argName, dir, j, tokens - j, isVariadic}));
     } else {
       j = dir.SkipBlanks(j + 1);
-      CheckForUnbalancedParentheses(dir, j, tokens - j);
       definitions_.emplace(
           std::make_pair(nameToken, Definition{dir, j, tokens - j}));
     }
   } else if (dirName == "undef") {
     if (nameToken.empty()) {
       prescanner.Say(
           dir.GetIntervalProvenanceRange(dirOffset, tokens - dirOffset),
           "# missing or invalid name"_err_en_US);
CharBlock Preprocessor::SaveTokenAsName(const CharBlock &t) {
   names_.push_back(t.ToString());
   return {names_.back().data(), names_.back().size()};
 }
 
 bool Preprocessor::IsNameDefined(const CharBlock &token) {
   return definitions_.find(token) != definitions_.end();
 }
 
+bool Preprocessor::IsFunctionLikeDefinition(const CharBlock &token) {
+  auto it{definitions_.find(token)};
+  return it != definitions_.end() && it->second.isFunctionLike();
+}
+
 static std::string GetDirectiveName(
     const TokenSequence &line, std::size_t *rest) {
   std::size_t tokens{line.SizeInTokens()};
   std::size_t j{line.SkipBlanks(0)};
   if (j == tokens || line.TokenAt(j).ToString() != "#") {
     *rest = tokens;
     return "";
   }
if (lineNumber && sourceFile) {
     CHECK(pos);
     if (!linePath) {
       linePath = &*pos->path;
     }
     sourceFile->LineDirective(pos->trueLineNumber + 1, *linePath, *lineNumber);
   }
 }
 
-void Preprocessor::CheckForUnbalancedParentheses(
-    const TokenSequence &tokens, std::size_t j, std::size_t n) {
-  if (!anyMacroWithUnbalancedParentheses_) {
-    int nesting{0};
-    for (; n-- > 0; ++j) {
-      char ch{tokens.TokenAt(j).OnlyNonBlank()};
-      if (ch == '(') {
-        ++nesting;
-      } else if (ch == ')') {
-        if (nesting-- == 0) {
-          break;
-        }
-      }
-    }
-    if (nesting != 0) {
-      anyMacroWithUnbalancedParentheses_ = true;
-    }
-  }
-}
 } // namespace Fortran::parser

flang/lib/Parser/prescan.cpp

if (EmitCharAndAdvance(tokens, '*') == '*') {
       //  DATAC/N*2H  /    is repeated Hollerith
       preventHollerith_ = !slashInCurrentStatement_;
     }
   } else {
     char ch{*at_};
     if (ch == '(') {
       if (parenthesisNesting_++ == 0) {
         isPossibleMacroCall_ = tokens.SizeInTokens() > 0 &&
-            preprocessor_.IsNameDefined(
+            preprocessor_.IsFunctionLikeDefinition(
                 tokens.TokenAt(tokens.SizeInTokens() - 1));
       }
     } else if (ch == ')' && parenthesisNesting_ > 0) {
       --parenthesisNesting_;
     }
     char nch{EmitCharAndAdvance(tokens, ch)};
     preventHollerith_ = false;
     if ((nch == '=' &&
bool Prescanner::FreeFormContinuation() {
   } while (SkipCommentLine(ampersand));
   return false;
 }
 
 // Implicit line continuation allows a preprocessor macro call with
 // arguments to span multiple lines.
 bool Prescanner::IsImplicitContinuation() const {
   return !inPreprocessorDirective_ && !inCharLiteral_ && isPossibleMacroCall_ &&
-      parenthesisNesting_ > 0 &&
-      !preprocessor_.anyMacroWithUnbalancedParentheses() && !IsAtEnd() &&
+      parenthesisNesting_ > 0 && !IsAtEnd() &&
       ClassifyLine(nextLine_).kind == LineClassification::Kind::Source;
 }
 
 bool Prescanner::Continuation(bool mightNeedFixedFormSpace) {
   if (*at_ == '\n' || *at_ == '&') {
     if (inFixedForm_) {
       return FixedFormContinuation(mightNeedFixedFormSpace);
     } else {

flang/test/Preprocessing/implicit-contin1.F90

 ! RUN: %flang -E %s | FileCheck %s
-! When there's a macro definition with unbalanced parentheses,
-! don't apply implicit continuation.
+! When there's an object-like macro don't apply implicit continuation.
 #define M )
 call foo (1 M
 end
 
 !CHECK:      call foo(1 )
 !CHECK:      end

flang/test/Preprocessing/implicit-contin4.F90

+! RUN: %flang -E %s | FileCheck %s
+! Macro definitions with unbalanced parentheses should not affect
+! implicit continuations.
+subroutine foo(a, d)
+  implicit none
+  integer :: a
+  integer :: d
+
+#define sub(x, y) foo2(x, y)
+#define bar )
+
+   call sub(1,
+     2)
+end subroutine foo
+
+!CHECK: call foo2(1, 2)
+!CHECK: end subroutine foo