[flang-commits] [PATCH] D96464: [flang] Don't perform macro replacement unless *.F, *.F90, &c.

[Andrzej Warzynski via Phabricator via flang-commits]

awarzynski added a comment.

Hello @klausler, thank you for working on this!

@PeteSteinfeld  and @klausler, could you please clarify the following before this is merged. I want to make sure that the two Flang compiler drivers don't diverge too much.

**Q1:**
According Parsing.md <https://github.com/llvm/llvm-project/blob/main/flang/docs/Parsing.md>:

  The preprocessor always runs.

Doesn't this patch change this? Or indicate that such change is coming? If that's the case, could you please update `Parsing.md`?

**Q2:**
This patch adds `-cpp/-nocpp` flags. Are these meant to be used to enable/disable preprocessing? (like in e.g. gfortran <https://gcc.gnu.org/onlinedocs/gfortran/Preprocessing-Options.html>). If I understand correctly, currently even with `-nocpp` the preprocessor is still run.

**Q3**:
Could the following condition be extracted into a separate method (e.g. `bool shouldRunPP`):

  if (suffix == "F" || suffix == "F90" || suffix == "F95" ||
      suffix == "CUF" || suffix == "F18") {
    options.predefinitions = predefinitions;

? It would be very helpful if this could be shared between the drivers. Alternatively, could you suggest _where_  such method could be added?

The new driver assumes that the preprocessor always runs and adds standard predefinitions unconditionally here <https://github.com/llvm/llvm-project/blob/0feff71eabd39fc93f3d9ec30ed16e26da6cf883/flang/lib/Frontend/CompilerInstance.cpp#L145>. From what I can see, this patch shouldn't cause any of the driver tests to fail, but it does mean that the currently implemented behavior becomes incorrect. I'm happy to update the new driver in a separate patch, but could you clarify **Q1**, **Q2** and **Q3** first? Your input is much appreciated!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96464