reserved identifier violation

[elfring]

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

[xaizek]

Thanks for reminding me. I know they are non-compliant in C++ for a while, but not for the reason outlined in the link. Current C++ draft says in [lex.name]:

In addition, some identifiers are reserved for use by C++ implementations and shall not be used otherwise; no diagnostic is required.
(3.1) Each identifier that contains a double underscore __ or begins with an underscore followed by an uppercase letter is reserved to the implementation for any use.
(3.2) Each identifier that begins with an underscore is reserved to the implementation for use as a name in the global namespace.

So I would change ids to look like ZOGRASCOPE_HIGHLIGHTER_HPP_ and not ZOGRASCOPE_HIGHLIGHTER_HPP.

[elfring]

…, but not for the reason outlined in the link.

🤔 Some identifiers will need further development considerations for this issue because of the usage of double underscores.

[elfring]

Thanks for your adjustment of some affected include guards.

💭 Can further implementation details be reconsidered accordingly?
Examples:

• zograscope/third-party/pmr/pmr_string.hpp

  Line 8 in 3ad3027

  #ifndef PMR__PMR_STRING_HPP__

• zograscope/third-party/tree-sitter/src/unicode/umachine.h

  Line 25 in 3ad3027

  #ifndef __UMACHINE_H__

[xaizek]

That's not my code. I can update third-party/pmr/ as it's unlikely to be updated, but other changes will be undone on importing corresponding projects, so I wouldn't bother. This isn't a serious issue as indicated by no diagnostic is required in the standard and it's unlikely to cause trouble.

[elfring]

That's not my code.

• Do you use these components also for your software?
• How much do you care for the C++ standard compliance according to referenced software components?

This isn't a serious issue as indicated by no diagnostic is required in the standard

💭 I suggest to reconsider your interpretation of provided information.

and it's unlikely to cause trouble.

Would you like to take descriptions for undefined behavior better into account? 🤔

[xaizek]

Do you use these components also for your software?

Yes.

How much do you care for the C++ standard compliance according to referenced software components?

Proportionally to the negative effects non-compliance can have. Also some of third-party code is auto-generated and it's better to fix the generator (output of tree-sitter and also headers in its runtime).

Would you like to take descriptions for undefined behavior better into account?

Don't know how they derived that. From "shall not"? Maybe there is some additional clause somewhere.

The behaviour is quite clear here: code may not compile if header guard will clash with an id defined by a toolchain.

[elfring]

code may not compile if header guard will clash with an id defined by a toolchain.

💭 I suggest to reconsider also such an expectation.

I got the impression that it can be challenging to avoid the tampering with the reserved name space.

🔮 Will any more code reviewers and developers get into the mood to take another look at information according to the standard wording “…, or defines a reserved identifier as a macro name, the behavior is undefined.”?

[xaizek]

I just don't want to fix this in my copy of other projects, then forget and undo it. These fixes should be done upstream in Catch and tree-sitter. Otherwise if I decide to switch to, say, xmake build system and pull these dependencies as packages, those packages will contain broken ids anyway. Fixing it locally won't achieve much.

Such ids generally don't bother me, but since you're into reporting issues in various projects, I thought you'd want to raise the issue with corresponding upstreams.

Will any more code reviewers and developers get into the mood to take another look at information according to the standard wording “…, or defines a reserved identifier as a macro name, the behavior is undefined.”?

It's probably derived from definition of "undefined behaviour" at http://eel.is/c++draft/defns.undefined.

[elfring]

Such ids generally don't bother me, …

🔮 Will software development attention grow for the avoidance of undefined behaviour?

… raise the issue with corresponding upstreams.

💭 Can the referenced software components be clearly identified (and the corresponding issue trackers)?

[xaizek]

Will software development attention grow for the avoidance of undefined behaviour?

Not all UBs are equal. Those cases which the standard explicitly calls out as undefined are the most severe ones, non-compliance is a different kind of UB, which doesn't need the same level of attention.

Can the referenced software components be clearly identified (and the corresponding issue trackers)?

Looks like it was fixed a year ago in Catch2, so I just need to update it.

tree-sitter's part is here: https://github.com/tree-sitter/tree-sitter/tree/master/lib/src/unicode. I confused something for being autogenerated, looks like only these files borrowed from ICU need to be updated.

[elfring]

…, looks like only these files borrowed from ICU need to be updated.

🔮 How will the chances evolve to convince involved developers for further adjustments of affected source code?

Can any more double underscores be omitted from a software component like “polymorphic_allocator”?
🤔 How do you think about to use the programming language standard “C++17”?

[xaizek]

How will the chances evolve to convince involved developers for further adjustments of affected source code?

Not sure what you're asking here, please rephrase.

Can any more double underscores be omitted from a software component like “polymorphic_allocator”?

Thanks, was looking only at defines.

How do you think about to use the programming language standard “C++17”?

Isn't worth it to get rid only of third-party/pmr, and dropping Boost dependency also needs dealing with parameter parsing and stream buffers differently.

[elfring]

Not sure what you're asking here, please rephrase.

A fraction of developers cares for discussed implementation details to some degree.
Other contributors present different development priorities or even understanding difficulties.

🤔 Further clarification can help for the influence of Unicode parts on the software like “Tree-sitter”, can't it?

[xaizek]

So you worry they will also be reluctant to adjust headers because those are borrowed from ICU? Might be, ICU also haven't updated their guards, so pulling new version won't help. However, tree-sitter's runtime is meant for direct inclusion into other projects, in this case updating guards in place is quite justified. Also names like __UTF16_H__ can quite easily appear in /usr/include of some systems.

[elfring]

🤔 I am unsure how much “ICU” contributors will care for remaining C/C++ compliance concerns.

[xaizek]

Me neither, but given the size of the change they might be fine doing an update. There are grounds for avoiding such generic names for sure, I know one project using COMPILER_H_ which conflicts with system headers on some BSD systems.