Skip to content

Specify C standard #1532

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.

Sign up for GitHub

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

Jump to bottom
Merged
maxaehle merged 2 commits into from
Feb 2, 2022
Merged

Specify C standard #1532

maxaehle merged 2 commits into from
Feb 2, 2022

Conversation

@maxaehle
Copy link
Contributor

@maxaehle maxaehle commented Feb 2, 2022 •
edited
Loading

Proposed Changes

In SU2/meson.build, specify the version of the C standard used when compiling SU2, as C99. It must be at least C99 because of the for loop initial declarations in externals/cgns/cgnslib.c. (One could probably use a newer C standard as well.)

This change was necessary to compile SU2 on a system where cc is the GCC 4.8.5 (from 2015), which used -std=gnu89 by default.

Related Work

The change was not necessary in v7.1.0. I suppose it has something to do with the recent CGNS updates (#1500, #1507, ...).

PR Checklist

Put an X by all that apply. You can fill this out after submitting the PR. If you have any questions, don't hesitate to ask! We want to help. These are a guide for you to know what the reviewers will be looking for in your contribution.

  • I am submitting my contribution to the develop branch.
  • My contribution generates no new compiler warnings (try with the '-Wall -Wextra -Wno-unused-parameter -Wno-empty-body' compiler flags, or simply --warnlevel=2 when using meson).
  • My contribution is commented and consistent with SU2 style.
  • I have added a test case that demonstrates my contribution, if necessary.
  • I have updated appropriate documentation (Tutorials, Docs Page, config_template.cpp) , if necessary.

@maxaehle
Use C99.
36d7542 
Otherwise a pre-5.1 version of GCC, using std=gnu89 by default,
complains about the for loop initial declaration in
externals/cgns/cgnslib.c.
TobiKattmann
TobiKattmann approved these changes Feb 2, 2022
View reviewed changes
Copy link
Contributor

@TobiKattmann TobiKattmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Elwetritsch I guess... I didn't notice because I build with -Denable-cgns=false as I never really deal with CGNS meshes
💐

@maxaehle
Copy link
Contributor Author

maxaehle commented Feb 2, 2022

On Elwetritsch I guess...

Yes ;-)

@thomasdick
Copy link
Contributor

thomasdick commented Feb 2, 2022

Yes, it is an issue on the Elwetritsch cluster. I tested this and the solution works.

@TobiKattmann
Copy link
Contributor

TobiKattmann commented Feb 2, 2022 •
edited
Loading

Don't mind the failing parallel_regression.py, that is due to su2code/TestCases#91 . The correct residuals will be pushed with #1530. It is the 3D streamwise periodic case where I changed a wrong NDIME in the mesh

But I guess merging is blocked as parallel_regression.py is required 😬

@maxaehle maxaehle merged commit 2b79590 into develop Feb 2, 2022
@maxaehle maxaehle deleted the fix_c_version branch February 2, 2022 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MicK7 MicK7 MicK7 approved these changes

@WallyMaier WallyMaier WallyMaier approved these changes

@TobiKattmann TobiKattmann TobiKattmann approved these changes

@pcarruscag pcarruscag pcarruscag approved these changes

Assignees

No one assigned

Labels

changelog:chore PR: merged

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@maxaehle @thomasdick @TobiKattmann @MicK7 @WallyMaier @pcarruscag