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

reserved identifier violation #46

Closed
elfring opened this issue Dec 2, 2023 · 26 comments
Closed

reserved identifier violation #46

elfring opened this issue Dec 2, 2023 · 26 comments
Assignees
Labels
enhancement Denotes something that will be implemented soon. wontfix We appreciate this issue but decided not to change the current behavior.

Comments

@elfring
Copy link

elfring commented Dec 2, 2023

I would like to point out that an identifier like “__ARM_2D_H__does not fit to the expected naming convention of the C++ language standard.
Would you like to adjust your selection for unique names?

@GorgonMeducer
Copy link
Contributor

GorgonMeducer commented Dec 2, 2023

Thank you for your feedback.

We knew this naming convention, and similar to cmsis and cmsis-dsp, arm-2d is part of the software infrastructure libraries provided by the arm for use with the arm toolchains (for most of the time I guess).

https://github.com/search?q=repo%3AARM-software%2FCMSIS-DSP%20__&type=code

https://github.com/ARM-software/CMSIS-DSP/blob/99e15d3d95084b6868c5579b6791aff5cbeefcd6/Include/dsp/none.h

And you are right. For the application code and example code, we should avoid using __ as the prefix. We will apply the change in the v1.1.6.

Thank you very much.

I hope this answers your questions.

@GorgonMeducer
Copy link
Contributor

By the way, Arm-2D is a C library but not C++, hence by checking.
image

https://wiki.sei.cmu.edu/confluence/display/c/DCL37-C.+Do+not+declare+or+define+a+reserved+identifier

We are open to further discussion.

@elfring
Copy link
Author

elfring commented Dec 3, 2023

💭 Would you become interested to use a development tool like “clang-tidy” for corresponding source code adjustments?

@GorgonMeducer
Copy link
Contributor

GorgonMeducer commented Dec 3, 2023

Thank you. Using which tools isn't a problem. The key is following which software standard. Frankly speaking, this project has no plan to follow the SEI CERT C++ Coding standard in the short term.

The excellence of the SEI CERT C++ Coding standard does not mean it is the only good one that everyone must comply with. At the same time, we are not saying that we will not follow it in the future; we simply don't know about the future, and we have indeed not seen problems caused by failure to comply with this standard yet.
We appreciate your concern about this issue. We have put compliance with some specifications, such as MISRA and SEI CERT C++ Coding standards, into our future plans.

I am looking forward to learning more in this kind of discussion.

@GorgonMeducer GorgonMeducer self-assigned this Dec 3, 2023
@elfring
Copy link
Author

elfring commented Dec 4, 2023

💭 I hope that you would also like to avoid that this software depends on undefined behaviour.

@GorgonMeducer
Copy link
Contributor

💭 I hope that you would also like to avoid that this software depends on undefined behaviour.

For this part, if you could point out detailed problem in a dedicated issue that would help.

Thank you very much.

@elfring
Copy link
Author

elfring commented Dec 4, 2023

👀 I got the impression that some ARM software components tamper with the reserved name space.

@GorgonMeducer
Copy link
Contributor

GorgonMeducer commented Dec 4, 2023

Your impression is correct for arm-2d, nothing to hide, for the other components, it depends.

@elfring
Copy link
Author

elfring commented Dec 5, 2023

Which changes did improve the affected software?

@GorgonMeducer GorgonMeducer reopened this Dec 5, 2023
@GorgonMeducer
Copy link
Contributor

GorgonMeducer commented Dec 5, 2023

Please give a more specific question. Most of the commits are used to improve the arm-2d; otherwise, why are there any commits in the first place?

@elfring
Copy link
Author

elfring commented Dec 5, 2023

🔮 Would any ARM developers like to rename affected include guards (for example)?

@GorgonMeducer
Copy link
Contributor

This is a question for arm developers; I know many who use the same manner, and I am definitely sure you know someone who doesn't like to use reserved identifiers.

I cannot represent any Arm developers, so I am not able to answer this question. Sorry.

@GorgonMeducer
Copy link
Contributor

GorgonMeducer commented Dec 5, 2023

I want to make this clear. You can blame me for using reserved identifiers.
I follow a different naming convention, which I have put it clear in arm-2d documents already, that is:
"__" means private in this library.

Unless I see solid evidence that the existing arm-2d identifier with "__" as their prefix conflicts with the reserved ones in arm c compilers that we claim to support, i.e. IAR, GCC, LLVM and Arm Compiler 5/6, I will not change my mind.

Even if some arm-2d identifiers are addressed to conflicts with some existing reserved ones, I will treat them in a case-by-case manner.

Thank you for your time and suggestions.

@elfring
Copy link
Author

elfring commented Dec 5, 2023

Unless I see solid evidence that the existing arm-2d identifier with "__" as their prefix conflicts with the reserved ones in arm c compilers that we claim to support, i.e. IAR, GCC, LLVM and Arm Compiler 5/6, I will not change my mind.

💭 It seems that some source code reviewers were also too careless according to concerns around undefined behaviour so far.
🔮 Will development interests grow for better compliance with known programming rules?

@GorgonMeducer
Copy link
Contributor

I am open to talking with you about this.

Would you mind providing me with some solid cases that:

existing arm-2d identifier with "__" as their prefix conflicts with the reserved ones in arm c compilers that we claim to support, i.e. IAR, GCC, LLVM and Arm Compiler 5/6,

This will help us to improve the project.
Thank you.

@GorgonMeducer
Copy link
Contributor

It seems that some source code reviewers were also too careless according to concerns around undefined behaviour so far.

We really welcome open-source community to help us with finding the undefined behaviours.

Please raise dedicated issues for each undefined behaviour.

@elfring
Copy link
Author

elfring commented Dec 5, 2023

👀 Did you overlook any warnings about undefined behaviour from linked information sources so far?

@GorgonMeducer
Copy link
Contributor

GorgonMeducer commented Dec 5, 2023

It looks like a personal question for me. Please allow me to keep the answers to myself.

For the arm-2d project, our team reviews all the warnings, evaluates the risks and treats them with different policies.
End users should see 0 warnings when using the arm compilers we claim to support.

if you address any warnings, please feel free to report them.

Thank you.

@elfring
Copy link
Author

elfring commented Dec 5, 2023

End users should see 0 warnings when using the arm compilers we claim to support.

if you address any warnings, please feel free to report them.

💭 I find such a feedback questionable according to discussed programming language compliance concerns.
Further source code analysis tools can help to reconsider remaining open issues, can't they? 🤔

@GorgonMeducer
Copy link
Contributor

We have yet to claim to follow the standard you mentioned.
So I would put your suggestion as a new feature recommendation.

Thank you for your time.

@GorgonMeducer GorgonMeducer added enhancement Denotes something that will be implemented soon. wontfix We appreciate this issue but decided not to change the current behavior. labels Dec 5, 2023
@elfring
Copy link
Author

elfring commented Dec 5, 2023

We have yet to claim to follow the standard you mentioned.

Does this software belong to the implementation for the programming language “C” (or “C++”)?

@GorgonMeducer
Copy link
Contributor

GorgonMeducer commented Dec 5, 2023

Arm-2D is under Apache 2.0.
It is an open-source project.
Anyone can check the source code and evaluate the risks based on their own judgements.
We have clearly mentioned in our document that "__" is used as an indicator of private symbols.

Those who use arm-2d should follow the Apache 2.0 license and manage their own risk.

@elfring
Copy link
Author

elfring commented Dec 5, 2023

We have clearly mentioned in our document that "__" is used as an indicator of private symbols.

I interpret such information as a recurring programming mistake.

@GorgonMeducer
Copy link
Contributor

I respect your judgement. Thank you.

@elfring
Copy link
Author

elfring commented Dec 5, 2023

I respect your judgement.

🔮 What does hinder you then to adhere to known programming language standard specifications?

@GorgonMeducer
Copy link
Contributor

Please allow me to keep this personal question to myself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Denotes something that will be implemented soon. wontfix We appreciate this issue but decided not to change the current behavior.
Projects
None yet
Development

No branches or pull requests

2 participants