-
Notifications
You must be signed in to change notification settings - Fork 100
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft: initial implementation of implicit_none #506
Conversation
if (model%implicit_none) then | ||
implicit_flag = " -fimplicit-none" | ||
else | ||
implicit_flag = "" | ||
end if |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't seem like the right place to add this option. At this point we are not aware which compiler we are using anymore and cannot be sure that we are actually passing the correct flag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, see the first TODO item.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's not add this option to the top-level in the manifest, but use the build sub-table instead. The top-level currently only contains meta data which does not affect the handling of the package, while all settings which change the behaviour of the build are currently found in the build sub-table.
How about implicit-typing
as name for the key? For consistency with other options, I would also prefer a dash rather than an underscore for multiword keys.
Co-authored-by: Sebastian Ehlert <[email protected]>
Yes, Yes, we couldn't quickly figure out how to do it on the per-package basis instead of global. We have to look into the build subtable. |
@epagone will you have time to finish this PR? |
Yes, I do not intend to let it go stale, sorry for the waiting. This week is extremely busy, I plan on having a look at it next week. |
Awesome, thanks! If you get stuck, please let me know, I can help. |
@epagone just a friendly ping. :) |
The corresponding option for ifort is |
I would suggest implementing this on top of the profiles feature. The logic is already worked out for propagating profiles, this would introduce logic about how different parts of the profiles are propagated. |
I agree with Brad that we should build this on the compiler profiles branch instead. |
Can somebody more familiar with fpm internals create a prototype how to hook this up with an option in fpm toml? I can then try to polish it. |
I can take a look into implementing this feature once we merged #575, which implements the necessary infrastructure to make this possible. But since this is also a straight-forward addition and some of you already started working on it, I'll also offer to only give a few pointers on how this is could be implemented and than hold myself back. Let me know what you preference here is. In any case, this PR is not a good place to discuss this feature. Therefore, I suggest we close this PR since it has significantly diverged from the main branch and continue the discussion at #577. |
Yes, I would like to take a shot at this, please. I had exactly the same plan in mind: wait for the work on the flags to stabilise a minimum and then ask for some pointers. I can't promise that I will be fast but, if you agree, I want to give it a try. |
Sure, that's fine. As long as it gets done. :) |
If that is your order, yessir! :) |
@epagone and I worked on this and implemented the following.
This adds
implicit_none
flag tofpm.toml
, if it is not specified, it will be.false.
by default.Then it is propagated to the model, at the top level (for now). Then in the backend we read this flag from the model and insert
-fimplicit-none
into the command line flag (so it only works for gfortran for now).TODO:
fpm_backend.f90
which is probably too late, it should be already part oftarget%compile_flags
)fpm.toml
and put into compiler flags / profiles and never expose it explicitly in the model?