Extend the C++ module scanner to handle Fortran, too. #8095
Conversation
mesonbuild/backend/ninjabackend.py
Outdated
| @@ -872,7 +872,11 @@ def generate_target(self, target): | |||
| self.generate_shlib_aliases(target, self.get_target_dir(target)) | |||
| self.add_build(elem) | |||
|
|
|||
| def should_scan_target(self, target): | |||
| def should_use_dyndeps_for_target(self, target): | |||
There was a problem hiding this comment.
| def should_use_dyndeps_for_target(self, target): | |
| def should_use_dyndeps_for_target(self, target: 'build.BuildTarget') -> bool: |
mesonbuild/scripts/depscan.py
Outdated
| FORTRAN_USE_PAT = r"^\s*use,?\s*(?:non_intrinsic)?\s*(?:::)?\s*(\w+)" | ||
|
|
||
| fortran_module_re = re.compile(FORTRAN_MODULE_PAT) | ||
| fortran_use_re = re.compile(FORTRAN_USE_PAT) |
There was a problem hiding this comment.
These should really all be all caps per PEP8, as they're module constants.
mesonbuild/backend/ninjabackend.py
Outdated
| all_suffixes = set() | ||
| for s in compilers.lang_suffixes['cpp']: | ||
| all_suffixes.add(s) | ||
| for s in compilers.lang_suffixes['fortran']: | ||
| all_suffixes.add(s) |
There was a problem hiding this comment.
| all_suffixes = set() | |
| for s in compilers.lang_suffixes['cpp']: | |
| all_suffixes.add(s) | |
| for s in compilers.lang_suffixes['fortran']: | |
| all_suffixes.add(s) | |
| all_suffixes = set(compilers.lang_suffixes['cpp']) | set(compilers.lang_suffixes['fortran']) |
| elif suffix in lang_suffixes['cpp']: | ||
| self.scan_fortran_file(fname) | ||
| else: | ||
| sys.exit('Can not scan files with suffix .{}.'.format(suffix)) |
There was a problem hiding this comment.
I think raise SystemExit is preferred to sys.exit in general.
However, in this case should it rather be raise RuntimeError?
mesonbuild/scripts/depscan.py
Outdated
| cpp_import_re = re.compile('\w*import ([a-zA-Z0-9]+);') | ||
| cpp_export_re = re.compile('\w*export module ([a-zA-Z0-9]+);') | ||
|
|
||
| FORTRAN_INCLUDE_PAT = r"^\s*#?include\s*['\"](\w+\.\w+)['\"]" |
There was a problem hiding this comment.
This could be in a separate PR, but to speed this up, the #include syntax isn't used by any compiler I've found. I added this when I was overhauling the regex for Fortran, but since realized no current compiler allows this syntax.
Suggest:
FORTRAN_INCLUDE_PAT = r"^\s*include\s*['\"](\w+\.\w+)['\"]"
|
Most of the Fortran project tests python .\run_project_tests.py --only fortranfail with this setup (only one I tried):
Typical build-time errorstest 3 test 4 and so on. |
jpakkane
force-pushed
the
fortranmodscan
branch
from
d91ba1f to
3cd6586
Compare
|
3cd6586 fails at a list | list I think maybe it's meant to have those variables be sets? |
jpakkane
force-pushed
the
fortranmodscan
branch
4 times, most recently
from
a0aa0c7 to
2aa674a
Compare
|
Finally it passes all project tests. I'll fix the mypy issues later. |
|
This pull request introduces 1 alert when merging 2aa674a into cb10ba7 - view on LGTM.com new alerts:
|
jpakkane
force-pushed
the
fortranmodscan
branch
from
2aa674a to
553e1fe
Compare
|
Yes this works for me with Gfortran 10 as well. I can't effectively test with the Intel Windows compiler yet, as the Intel oneAPI "gold" official release a few weeks ago actually broke a bunch of things that previously worked throughout the beta period, including independently breaking Meson and CMake. Intel has confirmed the issues and referred to internal team. Once Intel fixes oneAPI, this commit is necessary to recognize the new compilers (which replace the old) scivision@c6c2c53 |
jpakkane
force-pushed
the
fortranmodscan
branch
from
553e1fe to
62b04ed
Compare
|
That is unrelated to this issue, so merging. |
|
yes that's correct, thanks |
This will probably fail to the tests, but let's just get this out there for people to test and comment on. So ping all Fortran users, such as @scivision.
The main advantage of this approach is that we can drop all the prescan hacks we have and instead do Fortran automatically and reliably out of the box. I.e. moving module definitions from one file to another should just work without needing to run Meson reconfiguration.