Skip to content
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

Style suggestions cannot be disable #2984

Closed
kargl opened this issue Dec 15, 2023 · 6 comments · Fixed by #3910
Closed

Style suggestions cannot be disable #2984

kargl opened this issue Dec 15, 2023 · 6 comments · Fixed by #3910
Labels
bug Something isn't working easy to fix Easy to fix issue (good first issue for a new contributor)

Comments

@kargl
Copy link

kargl commented Dec 15, 2023

It seems that there is no option to disable style suggestions. Please add a --no-style option!

These suggestions vastly swamp actually errors with lfortran.

Edited: In fact, style suggestions should not be the default. The option should be --enable-style.

Edited**2: Just realized that the style suggestion was completely and utterly wrong. It seems to be complaining about the C preprocessor directive "#endif". Whoops.

@kargl kargl changed the title Style suggestins cannot be disable Style suggestions cannot be disable Dec 15, 2023
@certik certik added bug Something isn't working easy to fix Easy to fix issue (good first issue for a new contributor) labels Dec 16, 2023
@certik
Copy link
Contributor

certik commented Dec 16, 2023

I am sorry we didn't add the option to turn it off yet. This is a nice first issue for somebody to fix, just a compile time option needs to be added, and the warnings should only be emitted when the option is on.

Regarding default options, that's a separate discussion. But at the very least we also need to add an option --std=f23 or something like that, that will NOT have these style warnings on.

@kargl
Copy link
Author

kargl commented Apr 18, 2024

Any progress on this?

Style warnings are being produced for #endif C preprocessors macros when a error occurs with lfortran --cpp file.F90. #end if is not a valid macro.

@certik
Copy link
Contributor

certik commented Apr 18, 2024

@gxyd, @HarshitaKalani, @parth121101 does anyone of you have time to fix this? It should be pretty simple, just a new compiler option to add.

The #endif issue I think is a separate issue, that we need to investigate and fix. I think it gets tokenized improperly.

@HarshitaKalani
Copy link
Contributor

Can we use --no-warnings to disable the warnings?

$ lfortran a.f90
style suggestion: Use 'end do' instead of 'enddo'
 --> a.f90:5:5
  |
5 |     enddo
  |     ^^^^^ help: write this as 'end do'

style suggestion: Use 'end if' instead of 'endif'
 --> a.f90:8:5
  |
8 |     endif
  |     ^^^^^ help: write this as 'end if'


Note: Please report unclear, confusing or incorrect messages as bugs at
https://github.com/lfortran/lfortran/issues.
i is 10
$ lfortran a.f90 --no-warnings
i is 10

@kargl
Copy link
Author

kargl commented Apr 19, 2024

Can we use --no-warnings to disable the warnings?

That will disable all warnings, including possibly valid warnings. For example, real(4) :: x = sin(1._8) will convert a double precision value to single precision with a lose of precision. gfortran will give you

% gfcx -Wall -o z a.f90
  a.f90:1:14:

 1 | real(4) :: x = sin(1._8)
   |              1
Warning: Change of value in conversion from ‘REAL(8)’ to ‘REAL(4)’ at (1) [-Wconversion]

Recommendations for a change in style is based on someone's personal opinion. That opinion may conflict with a user's opinion and, more importantly, a style guide that one must follow. There is no compelling reason to issue style recommendations without a user specifically asking for them with, say, --style.

@HarshitaKalani
Copy link
Contributor

Got it. Thanks @kargl . Will work on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working easy to fix Easy to fix issue (good first issue for a new contributor)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants