[flang-commits] [flang] 50e1ad6 - [flang][Preprocessor] Constrain a bit more implicit continuations

Roger Ferrer Ibanez via flang-commits flang-commits at lists.llvm.org
Wed Aug 23 03:49:20 PDT 2023


Author: Roger Ferrer Ibanez
Date: 2023-08-23T10:48:40Z
New Revision: 50e1ad6ed23682d3a992a2e8c8b7db64baaccb66

URL: https://github.com/llvm/llvm-project/commit/50e1ad6ed23682d3a992a2e8c8b7db64baaccb66
DIFF: https://github.com/llvm/llvm-project/commit/50e1ad6ed23682d3a992a2e8c8b7db64baaccb66.diff

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

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

   ! 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/

Differential Revision: https://reviews.llvm.org/D157414

Added: 
    flang/test/Preprocessing/implicit-contin4.F90

Modified: 
    flang/lib/Parser/preprocessor.cpp
    flang/lib/Parser/preprocessor.h
    flang/lib/Parser/prescan.cpp
    flang/test/Preprocessing/implicit-contin1.F90

Removed: 
    


################################################################################
diff  --git a/flang/lib/Parser/preprocessor.cpp b/flang/lib/Parser/preprocessor.cpp
index d755605c12221a..c78a9e1f4d9f48 100644
--- a/flang/lib/Parser/preprocessor.cpp
+++ b/flang/lib/Parser/preprocessor.cpp
@@ -504,12 +504,10 @@ void Preprocessor::Directive(const TokenSequence &dir, Prescanner &prescanner) {
         }
       }
       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}));
     }
@@ -681,6 +679,11 @@ 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()};
@@ -1181,23 +1184,4 @@ void Preprocessor::LineDirective(
   }
 }
 
-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

diff  --git a/flang/lib/Parser/preprocessor.h b/flang/lib/Parser/preprocessor.h
index cea84839c88483..e0617a49095757 100644
--- a/flang/lib/Parser/preprocessor.h
+++ b/flang/lib/Parser/preprocessor.h
@@ -73,6 +73,7 @@ class Preprocessor {
   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 &);
@@ -80,10 +81,6 @@ class Preprocessor {
   // 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 };
@@ -95,14 +92,11 @@ class Preprocessor {
   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_

diff  --git a/flang/lib/Parser/prescan.cpp b/flang/lib/Parser/prescan.cpp
index 37ac1e5f4ecec8..6ed1581209da68 100644
--- a/flang/lib/Parser/prescan.cpp
+++ b/flang/lib/Parser/prescan.cpp
@@ -634,7 +634,7 @@ bool Prescanner::NextToken(TokenSequence &tokens) {
     if (ch == '(') {
       if (parenthesisNesting_++ == 0) {
         isPossibleMacroCall_ = tokens.SizeInTokens() > 0 &&
-            preprocessor_.IsNameDefined(
+            preprocessor_.IsFunctionLikeDefinition(
                 tokens.TokenAt(tokens.SizeInTokens() - 1));
       }
     } else if (ch == ')' && parenthesisNesting_ > 0) {
@@ -1148,8 +1148,7 @@ bool Prescanner::FreeFormContinuation() {
 // 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;
 }
 

diff  --git a/flang/test/Preprocessing/implicit-contin1.F90 b/flang/test/Preprocessing/implicit-contin1.F90
index 45fd568235da69..1c5ec8869bf4c2 100644
--- a/flang/test/Preprocessing/implicit-contin1.F90
+++ b/flang/test/Preprocessing/implicit-contin1.F90
@@ -1,6 +1,5 @@
 ! 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

diff  --git a/flang/test/Preprocessing/implicit-contin4.F90 b/flang/test/Preprocessing/implicit-contin4.F90
new file mode 100644
index 00000000000000..893be974f6d5ce
--- /dev/null
+++ b/flang/test/Preprocessing/implicit-contin4.F90
@@ -0,0 +1,17 @@
+! 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


        


More information about the flang-commits mailing list