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

Add C compiler base options guidelines #63

Open
wants to merge 3 commits into
base: wip/coding_guidelines
Choose a base branch
from

Conversation

josecm
Copy link
Member

@josecm josecm commented Jun 2, 2023

This PR introduces a set of guidelines on what options to use in the C compiler to be compliant with standard C11 and enable the base recommended warnings that will enforce a lot of the C guidelines that will be proposed next.

I'm wondering if this is the best way to indicate the compiler options, misra rules and checkers for each rule. Both structurally and formatting-wise. Please provide some feedback.

This PR is dependent on #62.

Checklist:

  • The changes follows the documentation guidelines described in here.
  • The changes generate no new warnings when building the project. If so, I have justified above.
  • I have run the CI checkers before submitting the PR to avoid unnecessary runs of the workflow.

@josecm josecm force-pushed the feat/coding_guidelines branch from 6709d42 to ec00050 Compare June 17, 2023 15:11
Base automatically changed from feat/coding_guidelines to wip/coding_guidelines July 17, 2023 14:02
@josecm
Copy link
Member Author

josecm commented Jul 27, 2023

ping @danielRep @miguelafsilva5

Comment on lines +33 to +49
- The C11 version shall be used.

Compiler options: `-std=c11`.
MISRA C:2012 : R1.1

- No language extensions shall be allowed.

Compiler options: `-pedantic -pedantic-errors`.
MISRA C:2012 : R1.1

- Treat all warnings as errors so no warnings can be ignored.

Compiler options: `-Werrors`.

- Enable all recommended warnings.

Compiler options: `-Wall -Wextra`.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- The C11 version shall be used.
Compiler options: `-std=c11`.
MISRA C:2012 : R1.1
- No language extensions shall be allowed.
Compiler options: `-pedantic -pedantic-errors`.
MISRA C:2012 : R1.1
- Treat all warnings as errors so no warnings can be ignored.
Compiler options: `-Werrors`.
- Enable all recommended warnings.
Compiler options: `-Wall -Wextra`.
R1. **The C11 version shall be used.**
- Compiler options: `-std=c11`. MISRA C:2012 : R1.1
R2. **No language extensions shall be allowed.**
- Compiler options: `-pedantic -pedantic-errors`. MISRA C:2012 : R1.1
R3. **Treat all warnings as errors so no warnings can be ignored.**
- Compiler options: `-Werrors`.
R4. **Enable all recommended warnings.**
- Compiler options: `-Wall -Wextra`.

Copy link
Member

Choose a reason for hiding this comment

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

IMHO, i'd prefer this type of format so if we want to reference one rule we can use the enumeration (e.g., R1, etc)

image

Copy link
Member

Choose a reason for hiding this comment

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

I agree

Comment on lines +23 to +24
both. If one compiler is supported for given project, it must be so for all
supported target architectures.
Copy link
Member

Choose a reason for hiding this comment

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

Might be too strict, for example for tricore

both. If one compiler is supported for given project, it must be so for all
supported target architectures.

For reach guideline we try to provide a set of compiler options which enforce
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
For reach guideline we try to provide a set of compiler options which enforce
For each guideline we try to provide a set of compiler options which enforce

Comment on lines +33 to +49
- The C11 version shall be used.

Compiler options: `-std=c11`.
MISRA C:2012 : R1.1

- No language extensions shall be allowed.

Compiler options: `-pedantic -pedantic-errors`.
MISRA C:2012 : R1.1

- Treat all warnings as errors so no warnings can be ignored.

Compiler options: `-Werrors`.

- Enable all recommended warnings.

Compiler options: `-Wall -Wextra`.
Copy link
Member

Choose a reason for hiding this comment

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

I agree

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants