[flang][preprocessing] Mix preprocessing directives with free form li… #96244

klausler · 2024-06-20T22:32:08Z

…ne continuation

Allow preprocessing directives to appear between a source line and its continuation, including conditional compilation directives (#if, #ifdef, &c.).

Fixes #95476.

llvmbot · 2024-06-20T22:32:46Z

@llvm/pr-subscribers-flang-driver

@llvm/pr-subscribers-flang-parser

Author: Peter Klausler (klausler)

Changes

…ne continuation

Allow preprocessing directives to appear between a source line and its continuation, including conditional compilation directives (#if, #ifdef, &c.).

Fixes #95476.

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

4 Files Affected:

(modified) flang/include/flang/Parser/preprocessor.h (+1)
(modified) flang/lib/Parser/prescan.cpp (+23-17)
(modified) flang/lib/Parser/prescan.h (+1-1)
(added) flang/test/Preprocessing/cond-contin.F90 (+21)

diff --git a/flang/include/flang/Parser/preprocessor.h b/flang/include/flang/Parser/preprocessor.h
index c3076435be5f0..57690dd226f62 100644
--- a/flang/include/flang/Parser/preprocessor.h
+++ b/flang/include/flang/Parser/preprocessor.h
@@ -82,6 +82,7 @@ class Preprocessor {
   bool IsNameDefined(const CharBlock &);
   bool IsFunctionLikeDefinition(const CharBlock &);
   bool AnyDefinitions() const { return !definitions_.empty(); }
+  bool InConditional() const { return !ifStack_.empty(); }
 
   // When called with partialFunctionLikeMacro not null, MacroReplacement()
   // and ReplaceMacros() handle an unclosed function-like macro reference
diff --git a/flang/lib/Parser/prescan.cpp b/flang/lib/Parser/prescan.cpp
index 2a6ecfbb0830e..452752cc89962 100644
--- a/flang/lib/Parser/prescan.cpp
+++ b/flang/lib/Parser/prescan.cpp
@@ -105,13 +105,14 @@ void Prescanner::Statement() {
     NextLine();
     return;
   case LineClassification::Kind::ConditionalCompilationDirective:
-  case LineClassification::Kind::DefinitionDirective:
-  case LineClassification::Kind::PreprocessorDirective:
+  case LineClassification::Kind::IncludeDirective:
     preprocessor_.Directive(TokenizePreprocessorDirective(), *this);
+    afterPreprocessingDirective_ = true;
     return;
-  case LineClassification::Kind::IncludeDirective:
+  case LineClassification::Kind::PreprocessorDirective:
+  case LineClassification::Kind::DefinitionDirective:
     preprocessor_.Directive(TokenizePreprocessorDirective(), *this);
-    afterIncludeDirective_ = true;
+    // Don't set afterPreprocessingDirective_
     return;
   case LineClassification::Kind::CompilerDirective: {
     directiveSentinel_ = line.sentinel;
@@ -289,13 +290,14 @@ void Prescanner::CheckAndEmitLine(
   tokens.CheckBadFortranCharacters(
       messages_, *this, disableSourceContinuation_);
   // Parenthesis nesting check does not apply while any #include is
-  // active, nor on the lines before and after a top-level #include.
+  // active, nor on the lines before and after a top-level #include,
+  // nor before or after conditional source.
   // Applications play shenanigans with line continuation before and
-  // after #include'd subprogram argument lists.
+  // after #include'd subprogram argument lists and conditional source.
   if (!isNestedInIncludeDirective_ && !omitNewline_ &&
-      !afterIncludeDirective_ && tokens.BadlyNestedParentheses()) {
-    if (inFixedForm_ && nextLine_ < limit_ &&
-        IsPreprocessorDirectiveLine(nextLine_)) {
+      !afterPreprocessingDirective_ && tokens.BadlyNestedParentheses() &&
+      !preprocessor_.InConditional()) {
+    if (nextLine_ < limit_ && IsPreprocessorDirectiveLine(nextLine_)) {
       // don't complain
     } else {
       tokens.CheckBadParentheses(messages_);
@@ -306,7 +308,7 @@ void Prescanner::CheckAndEmitLine(
     omitNewline_ = false;
   } else {
     cooked_.Put('\n', newlineProvenance);
-    afterIncludeDirective_ = false;
+    afterPreprocessingDirective_ = false;
   }
 }
 
@@ -1069,6 +1071,17 @@ bool Prescanner::SkipCommentLine(bool afterAmpersand) {
     return true;
   } else if (inPreprocessorDirective_) {
     return false;
+  } else if (afterAmpersand &&
+      (lineClass.kind ==
+              LineClassification::Kind::ConditionalCompilationDirective ||
+          lineClass.kind == LineClassification::Kind::DefinitionDirective ||
+          lineClass.kind == LineClassification::Kind::PreprocessorDirective ||
+          lineClass.kind == LineClassification::Kind::IncludeDirective ||
+          lineClass.kind == LineClassification::Kind::IncludeLine)) {
+    SkipToEndOfLine();
+    omitNewline_ = true;
+    skipLeadingAmpersand_ = true;
+    return false;
   } else if (lineClass.kind ==
           LineClassification::Kind::ConditionalCompilationDirective ||
       lineClass.kind == LineClassification::Kind::PreprocessorDirective) {
@@ -1080,13 +1093,6 @@ bool Prescanner::SkipCommentLine(bool afterAmpersand) {
     // continued line).
     preprocessor_.Directive(TokenizePreprocessorDirective(), *this);
     return true;
-  } else if (afterAmpersand &&
-      (lineClass.kind == LineClassification::Kind::IncludeDirective ||
-          lineClass.kind == LineClassification::Kind::IncludeLine)) {
-    SkipToEndOfLine();
-    omitNewline_ = true;
-    skipLeadingAmpersand_ = true;
-    return false;
   } else {
     return false;
   }
diff --git a/flang/lib/Parser/prescan.h b/flang/lib/Parser/prescan.h
index b6f6d2ca439ee..421ba97d324c9 100644
--- a/flang/lib/Parser/prescan.h
+++ b/flang/lib/Parser/prescan.h
@@ -214,7 +214,7 @@ class Prescanner {
   int prescannerNesting_{0};
   int continuationLines_{0};
   bool isPossibleMacroCall_{false};
-  bool afterIncludeDirective_{false};
+  bool afterPreprocessingDirective_{false};
   bool disableSourceContinuation_{false};
 
   Provenance startProvenance_;
diff --git a/flang/test/Preprocessing/cond-contin.F90 b/flang/test/Preprocessing/cond-contin.F90
new file mode 100644
index 0000000000000..919ea5b127d60
--- /dev/null
+++ b/flang/test/Preprocessing/cond-contin.F90
@@ -0,0 +1,21 @@
+! RUN: %flang -fc1 -E %s 2>&1 | FileCheck %s
+! CHECK: subroutine test(ARG1,FA, FB,ARG2)
+! CHECK: end
+
+subroutine test( &
+ARG1, &
+! test
+#ifndef SWAP
+#define ARG1 FA
+#define ARG2 FB
+#else
+#define ARG1 FB
+#define ARG2 FA
+#endif
+ARG1, ARG2, &
+! test
+#undef ARG1
+#undef ARG2
+&ARG2)
+! comment
+end

psteinfeld

All builds and tests correctly and looks good.

foxtran · 2024-06-21T17:27:36Z

I still have some issues with preprocessor, even with commit 0511614.

klausler · 2024-06-21T17:29:28Z

Examples would be useful.

foxtran · 2024-06-21T19:20:30Z

The test is the following:
test.f:

      program main
#include "inc.f"
     $,.false.)
      end program main

inc.f:

      call t(

P.S. Ooops. It is invalid program, but the error messages did not say this :)

For valid program,
inc.f:

      call t(5.0

foxtran · 2024-06-21T19:40:55Z

Another example with .f90:
test.f90:

        program main
#include "inc.f"
     &.44
      end program main

inc.f:

print *, 5.&

After running flang tests:

$ flang-new -fc1 -E test.f90
#line "./test.f90" 1
      program main
#line "./inc.f" 1
      print 5.end program main

while the expected output is:

$ flang-new -fc1 -E test.f90
#line "./test.f90" 1
      program main
#line "./inc.f" 1
      print *, 5.44
#line ??? ! I have no idea :)
      end program main

foxtran · 2024-06-21T21:10:02Z

One more funny example with Fortran include statement:
test.f90:

      program main
#define ERROR 1
      include "inc.f"
     &.44
      end program main

inc.f:

#ifndef ERROR
print 5.&
#else
#error "Hi from LLVM"
#endif

gfortran -cpp -E produces:

# 1 "test.f90"
# 1 "<built-in>"
# 1 "<command-line>"
# 1 "test.f90"
      program main

      include "inc.f"
     &.44
      end program main

ifx -cpp -E produces (but, in general, it fails):

# 1 "test.f90"
      program main

      include "inc.f"
     &.44
      end program main

However, flang passes ERROR into Fortran-include statement that gives:

error: Could not scan test.f90
./inc.f:4:2: error: #error "Hi from LLVM"
  #error "Hi from LLVM"
   ^^^^^^^^^^^^^^^^^^^^
./test.f90:3:1: error: included here
        include "inc.f"
  ^^^^^^^^^^^^^^^^^^^^^

foxtran · 2024-06-21T21:13:58Z

BTW, I would prefer to see preprocessed file and only after that error messages, if they are, when -E flag is passed.

klausler · 2024-06-22T19:19:26Z

BTW, I would prefer to see preprocessed file and only after that error messages, if they are, when -E flag is passed.

That would be best filed as a separate bug against the driver; I don't work on that.

klausler · 2024-06-22T19:27:31Z

Fortran's INCLUDE lines are handled by f18 in pretty much the same way as #include directives are, so that the included text has access to macro definitions in effect at the time of the inclusion. In other words, this is intentional behavior.

klausler · 2024-06-22T20:36:45Z

Thanks for the additional two cases; the patch has been updated to cover them as well.

foxtran · 2024-06-22T21:08:35Z

flang/test/Preprocessing/inc-contin-2.F90

@@ -0,0 +1,6 @@
+! RUN: %flang_fc1 -E %s 2>&1 | FileCheck %s
+! CHECK: print *, 3.14159


It is improper result, since test has whitespace between . and & in flang/test/Preprocessing/inc-contin-2.h.
It should be:

! CHECK: print *, 3. 14159

And then, it must fail since whitespaces are not allowed in constants as it was in Fortran-77.

There is an example:
https://godbolt.org/z/Gx1brb7h6

Thanks; patch has been updated to test cases with and without the space.

…ne continuation Allow preprocessing directives to appear between a source line and its continuation, including conditional compilation directives (#if, #ifdef, &c.). Fixes llvm#95476.

foxtran · 2024-06-24T14:25:47Z

Thanks! Parser can read our source code :)

llvm#96244) …ne continuation Allow preprocessing directives to appear between a source line and its continuation, including conditional compilation directives (#if, #ifdef, &c.). Fixes llvm#95476.

klausler requested a review from psteinfeld June 20, 2024 22:32

llvmbot added flang Flang issues not falling into any other category flang:parser labels Jun 20, 2024

klausler mentioned this pull request Jun 20, 2024

[flang][preprocessor] C preprocessor can not be inside of statement #95476

Closed

psteinfeld approved these changes Jun 21, 2024

View reviewed changes

klausler force-pushed the bug95476 branch from 0511614 to 5a56db1 Compare June 22, 2024 20:36

llvmbot added the flang:driver label Jun 22, 2024

foxtran reviewed Jun 22, 2024

View reviewed changes

[flang][preprocessing] Mix preprocessing directives with free form li…

e4ef433

…ne continuation Allow preprocessing directives to appear between a source line and its continuation, including conditional compilation directives (#if, #ifdef, &c.). Fixes llvm#95476.

klausler force-pushed the bug95476 branch from 5a56db1 to e4ef433 Compare June 23, 2024 17:19

klausler merged commit 5d15f60 into llvm:main Jun 24, 2024
7 checks passed

klausler deleted the bug95476 branch June 24, 2024 17:18

		@@ -0,0 +1,6 @@
		! RUN: %flang_fc1 -E %s 2>&1 \| FileCheck %s
		! CHECK: print *, 3.14159

[flang][preprocessing] Mix preprocessing directives with free form li… #96244

[flang][preprocessing] Mix preprocessing directives with free form li… #96244

Uh oh!

Conversation

klausler commented Jun 20, 2024

Uh oh!

llvmbot commented Jun 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

psteinfeld left a comment

Choose a reason for hiding this comment

Uh oh!

foxtran commented Jun 21, 2024

Uh oh!

klausler commented Jun 21, 2024

Uh oh!

foxtran commented Jun 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

foxtran commented Jun 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

foxtran commented Jun 21, 2024

Uh oh!

foxtran commented Jun 21, 2024

Uh oh!

klausler commented Jun 22, 2024

Uh oh!

klausler commented Jun 22, 2024

Uh oh!

klausler commented Jun 22, 2024

Uh oh!

foxtran Jun 22, 2024

Choose a reason for hiding this comment

Uh oh!

foxtran Jun 22, 2024

Choose a reason for hiding this comment

Uh oh!

klausler Jun 23, 2024

Choose a reason for hiding this comment

Uh oh!

foxtran commented Jun 24, 2024

Uh oh!

Uh oh!

Uh oh!

llvmbot commented Jun 20, 2024 •

edited

Loading

foxtran commented Jun 21, 2024 •

edited

Loading

foxtran commented Jun 21, 2024 •

edited

Loading