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 #439

Closed
elfring opened this issue Jul 26, 2018 · 10 comments
Closed

reserved identifier violation #439

elfring opened this issue Jul 26, 2018 · 10 comments

Comments

@elfring
Copy link

elfring commented Jul 26, 2018

I would like to point out that identifiers like “__math_compat_h” and “__STRINGdo eventually not fit to the expected naming convention of the C++ language standard.
Would you like to adjust your selection for unique names?

@hawicz
Copy link
Member

hawicz commented Jul 27, 2018

Meh, if it's causing an actual problem then of course let's change it, but otherwise it seems like needless work. However, if you want to submit a pull request, go ahead and I'll merge it.

@hawicz hawicz closed this as completed Jul 27, 2018
@elfring
Copy link
Author

elfring commented Jul 27, 2018

How do you think about to avoid that this software depends on undefined behaviour?

@hawicz
Copy link
Member

hawicz commented Jul 27, 2018

What are you talking about? If you are seeing an actual concrete problem, then please explain. If you have some change that you're proposing, please submit a pull request.

@elfring
Copy link
Author

elfring commented Jul 27, 2018

Would you like to achieve standard compliance also for this software library?

@hawicz
Copy link
Member

hawicz commented Jul 27, 2018

Blind adherence to some standard, without consideration for the practical impacts would be a waste of time for me, but if you want to submit a PR to change the header guards even, then be my guest.

@hawicz
Copy link
Member

hawicz commented Jul 27, 2018

Also note that json-c is a C library, not a C++ one, so it's even less useful to try to apply C++ guidelines to its header files, especially those meant only for internal use.

@elfring
Copy link
Author

elfring commented Jul 27, 2018

Should this software library be safely usable not only for implementations of compilers for these programming languages?

@ploxiln
Copy link
Contributor

ploxiln commented Jul 27, 2018

That page has both "critical undefined behavior" and "bounded undefined behavior". When people talk about undefined behavior in C, they talk about the "critical" kind. But some conflict over identifiers is the "bounded" kind which just means that some future compiler+libc may theoretically fail to compile json-c due to an identifier name conflict. At that time we will notice.

I suggest not trying to dig up and find theoretical problems about which you simply do not understand. It's ok, you don't have to understand it all in order to be a decent programmer, it takes a lot of time and experience, this is cryptic archaic systems stuff ...

@elfring
Copy link
Author

elfring commented Jul 27, 2018

How does the probability look like to notice anything (besides the application of strict static source code analysis) when undefined behaviour could be involved because other programmers are using this software library not only for compiler implementations?

@hawicz
Copy link
Member

hawicz commented Jul 28, 2018

Who said anything about using this within compiler implementations? I certainly did not.

IMO, the probability of noticing a problem in this specific concrete case of header guards is quite high, as either you'll be missing definitions from a system header and the code won't compile, or you'll be missing definitions from the json-c header, and again the code won't compile.
If it happens to compile anyway, the chance that the behavior of the defined symbols is different than expected is nearly zero (isnan, isinf, etc... will be pretty consistent), and even if it is different then the automated tests will catch the discrepancies.

"undefined behavior" doesn't mean it's some sneaky, impossible to notice case. It just means that the behavior isn't defined by the standard that you happen to be reading, so to determine what actually happens you need to look beyond the standard.

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

No branches or pull requests

3 participants