ARM-software / Arm-2D

2D Graphic Library optimized for Cortex-M processors
Apache License 2.0
298 stars 66 forks source link

reserved identifier violation #46

Closed elfring closed 11 months ago

elfring commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

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

GorgonMeducer commented 11 months ago

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.

elfring commented 11 months ago

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

GorgonMeducer commented 11 months ago

:thought_balloon: 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 commented 11 months ago

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

GorgonMeducer commented 11 months ago

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

elfring commented 11 months ago

Which changes did improve the affected software?

GorgonMeducer commented 11 months ago

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 commented 11 months ago

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

GorgonMeducer commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

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.

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

GorgonMeducer commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

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

GorgonMeducer commented 11 months ago

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 commented 11 months ago

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.

:thought_balloon: 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? :thinking:

GorgonMeducer commented 11 months ago

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.

elfring commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

I respect your judgement. Thank you.

elfring commented 11 months ago

I respect your judgement.

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

GorgonMeducer commented 11 months ago

Please allow me to keep this personal question to myself.