The behaviour triggered with this flag is consistent with -fparse-only in flang (i.e. the throwaway driver). This new spelling is consistent with Clang and gfortran, and was proposed and agreed on for the new driver in .
A few comments.
|43–44 ↗||(On Diff #310234)|
Nit: Semaantics -> Semantics
What is the coding style of this file?
Is Evaluate needed?
What is the use of output stream here?
Will the Prescan step have happened before?
Nit: semantincs -> semantics
should there be a test without fc1?
Thanks for patch, a few queries below.
Are the tests in clang which are related to flang failing at https://reviews.llvm.org/B81474?
Do you think following a pattern here for mapping with enum ActionKind would be better?
What happens in case one of the stages in the pipeline fails?
Thank you for your reviews! I'll submit an updated patch shortly.
|43–44 ↗||(On Diff #310234)|
These 2 lines mean that flang-new -fsyntax-only is translated to flang-new -fc1 -fsyntax-only -triple <triple>. However,
$ bin/flang-new -fc1 -triple error: unknown argument: '-triple'
i.e. the compiler frontend driver, flang-new -fc1, doesn't support this flag.
Yes, that would be better, thanks!
Don't think so. I built with -DBUILD_SHARED_LIBS=On, which usually is pretty good at catching missing CMake dependencies.
In set_debugOutput: https://github.com/llvm/llvm-project/blob/1365718778b4ce05587afa22835282c5d3f835b7/flang/lib/Parser/parsing.cpp#L113. Ideally this should go via some diagnostics engine.
Prescan happens here: https://github.com/llvm/llvm-project/blob/1365718778b4ce05587afa22835282c5d3f835b7/flang/lib/Frontend/FrontendAction.cpp#L53. ExecuteAction (i.e. this method) is the next step.
That's a great catch, thank you!
This is a tricky question :) The answer depends on what we want to test here:
In general flang-new -fsyntax-only != flang-new -fc1 -fsyntax-only. Admittedly, currently these are actually equal. However, if you compare clang -### -fsyntax-only and clang -cc1 -fsyntax-only then you will see a rather huge difference. flang-new is also heading in that direction.
To me -fsyntax-only is actually a frontend driver flag. If we also test flang-new -fsyntax-only then that triggers quite a few code-paths within libclangDriver. But this patch doesn't really touch that logic. Instead, it adds some logic in the frontend driver and hence IMO it should be testing the frontend driver only.
But one can argue that from user's perspective:
should be identical. But then again, are we verifying that the user gets what they expect or the internal implementation?
Separately, the fact that libclangDriver _forwards_ -fsyntax-only to flang-new -fc1 is tested here: https://github.com/llvm/llvm-project/blob/1365718778b4ce05587afa22835282c5d3f835b7/clang/test/Driver/flang/flang.f90#L16. (it's a bit more nuanced, that option is not really forwarded, but that's an implementation detail).
Just a quick question:
Hard-coded values are bad, thanks for pointing that out!
We can't use CompilerInvocation::SetDefaultFortranOpts here, as that's for Fortran::parser::Options, and here we are dealing with the internal state of Fortran::semantics::SemanticsContext.
However, moduleDirectory_ is in fact initialised to "."s by default: https://github.com/llvm/llvm-project/blob/8acb5f2723ecaf0f1904a085ad79d0623cec38f6/flang/include/flang/Semantics/semantics.h#L185.
So, it's safe to delete this line for now. A proper solution/approach will be implemented once we add support for -module-dir: https://docs.google.com/spreadsheets/d/1JRe39lP_KhtkYxFEIvwrCFlE5v1Ofa_krOHI-XXXWPY/edit#gid=0.