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

[Roger Ferrer Ibanez via Phabricator via flang-commits]

rogfer01 added a comment.

Thanks for the update, @klausler

I added some more comments.

(Pretty please 🙂 , add the full context <https://www.llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface> otherwise reviewing the change gets unnecessarily harder)



================
Comment at: flang/include/flang/Parser/char-block.h:67
 
+  char OnlyNonBlank() const {
+    char result{' '};
----------------
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 ' ';
}
```


================
Comment at: flang/lib/Parser/preprocessor.cpp:1198
+    }
+    anyMacroWithUnbalancedParentheses_ = nesting != 0;
+  }
----------------
`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
```


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

https://reviews.llvm.org/D155499