Skip to content

Allow for header protection using defined operator#550

Draft
MFelida wants to merge 2 commits into42school:masterfrom
MFelida:header_prot_using_defined_operator
Draft

Allow for header protection using defined operator#550
MFelida wants to merge 2 commits into42school:masterfrom
MFelida:header_prot_using_defined_operator

Conversation

@MFelida
Copy link
Contributor

@MFelida MFelida commented Apr 13, 2025

Issues Fixed

fixes #544

Verification Steps

  • Added new tests to cover the changes.
  • Fixed all broken tests.
  • Ran poetry run flake8 to check for linting issues.
  • Verified that all unit tests are passing:
    • Ran poetry run pytest to ensure unit tests pass.
    • Ran poetry run tox to validate compatibility across Python versions.jk

Additional Notes

The norm doesn't specifically mandate the use if #ifndef for header protections, so I think using #if !defined should be allowed

@NiumXp
Copy link
Contributor

NiumXp commented Apr 13, 2025

Hello, @MFelida!

I don't think this is a good idea. Using if requires a runtime-style check, which isn't appropriate for preprocessor directives. For example, your approach doesn’t properly handle cases like:

// Just added parenthesis
#if !defined(file_h)
#define file_h
// ...
#endif

// Omit `!` and use `==` instead
#if defined(file_h) == 0
#define file_h
// ...
#endif

// What about a lot of `!`?
#if !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!                    defined file_h
#define file_h
// ...
#endif

To be honest, #ifndef is the most common, clear, and widely accepted way to do this (and it's an example in the norm PDF). Doing it your way seems like it's just for the sake of being different from everyone else.

@MFelida
Copy link
Contributor Author

MFelida commented Apr 13, 2025

Thanks for your input @NiumXp.

I don't think this is a good idea. Using if requires a runtime-style check, which isn't appropriate for preprocessor directives.

...

To be honest, #ifndef is the most common, clear, and widely accepted way to do this (and it's an example in the norm PDF). Doing it your way seems like it's just for the sake of being different from everyone else.

Personally, I don't think any of that matters. Using !defined as a header protection is not explicitly against the norm so norminette should support it, especially since norminette prevails over any evaluator's interpretation of the norm.
I don't think examples are valid or rigorous way of setting a precedent.

Preferably the norm would be amended to only allow ifndef, but I don't have that authority. I can however submit pull requests, so here we are.

Thanks for pointing out the potential errors. I'll see what I can do about those.

Copy link
Contributor

@NiumXp NiumXp left a comment

Choose a reason for hiding this comment

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

It doesn't resolve issue #544.

@MFelida MFelida marked this pull request as draft April 13, 2025 19:14
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.

if !defined() is considered a norm error

2 participants

Comments