[flang-commits] [PATCH] D155499: [flang] Stricter "implicit continuation" in preprocessing

[Peter Klausler via Phabricator via flang-commits]

klausler added inline comments.


================
Comment at: flang/include/flang/Parser/char-block.h:67
 
+  char OnlyNonBlank() const {
+    char result{' '};
----------------
rogfer01 wrote:
> klausler wrote:
> > rogfer01 wrote:
> > > I'm confused by this function. I've tested it with several inputs and it always seems to return `' '`?
> > > 
> > > My tests here: https://www.godbolt.org/z/af9oc74aE
> > > 
> > > Maybe I'm missing something. Can you add a comment about its purpose?
> > It returns the only non-blank character, if it is the only non-blank character.
> Ah, right a case I forgot to test 😅  .  Thanks.
> 
> Would it make sense not to use a loop? Something like this:
> 
> ```lang=cpp
> char OnlyNonBlank() const {
>     if (size() == 1) {
>         char ch{*begin()};
>         if (ch != ' ' && ch != '\t') {
>             return ch;
>         }
>     }
>     return ' ';
> }
> ```
The entire point of this function is to ignore leading and trailing blanks to expose a single-character token.  Every character needs to be examined.  This requires a loop.


================
Comment at: flang/lib/Parser/preprocessor.cpp:1198
+    }
+    anyMacroWithUnbalancedParentheses_ = nesting != 0;
+  }
----------------
rogfer01 wrote:
> `anyMacroWithUnbalancedParantheses_` is only getting updated when we find a directive and we do not seem to clean it up, so we end carrying wrong state up to the point where we decide we allow an implicit continuation.
> 
> Consider the following testcase:
> 
> ```lang=fortran
> program main
>   implicit none
> #define FOO(x) ((x) + 1)
> #define BAR (
>   integer :: k
> 
>   k = FOO(
>     4)
> end program main
> ```
> 
> This fails with the patch
> 
> ```
> $ flang-new -E -o- t.F90 
> error: Could not scan t.F90
> ./t.F90:8:10: error: Unmatched '('
>     k = FOO(
>            ^
> ./t.F90:9:6: error: Unmatched ')'
>       4)
>        ^
> ```
> 
> but if you remove (or comment) the definition of `BAR` the file is scanned correctly.
> 
> ```
> flang-new -E -o- t.F90 
> #line "./t.F90" 2
>       program main
>       implicit none
> 
> 
>       integer :: k
> 
>       k = (( 4) + 1)
> 
>       end program main
> ```
The whole point of this flag is to disable all implicit continuations if any macro definition has unbalanced parentheses.  I can't figure out how to do it safely and I've spent too much time on this topic already.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155499/new/

https://reviews.llvm.org/D155499