search

Cytnx-dev / Cytnx

Project Cytnx, A Cross-section of Python & C++,Tensor network library
Apache License 2.0
35 stars 13 forks source link

Using identifiers reserved for the implementation #460

Open ianmccul opened 2 weeks ago

ianmccul commented 2 weeks ago

I see that Cytnx uses a lot of identifiers of the form underscore followed by a capital letter. Eg all of the header file include guards _H...., and also as other identifiers (function names, variables etc).

This isn't a good idea: identifiers starting with an underscore then a capital letter are reserved for the implementation and should not appear in user code. Eg, the standard library might use them as global variables or macros. This actually happened once, I was debugging some student's code that was producing some weird error and we couldn't figure out what was going on, until I noticed that one of the variables was _P, and it turned out that that version of gcc had defined it as a macro. Note that the implementation of the standard library must always use such reserved identifiers in the implementation, because there is no way for the implementation to guard against users #define etc for non-reserved identifiers.

From https://en.cppreference.com/w/cpp/language/identifiers :

The following forms are reserved:

“Reserved” here means that the standard library headers #define or declare such identifiers for their internal needs, the compiler may predefine non-standard identifiers of that kind, and that name mangling algorithm may assume that some of these identifiers are not in use. If the programmer uses such identifiers, the program is ill-formed, no diagnostic required.

In addition, it is undefined behavior to #define or #undef certain names in a translation unit, see reserved macro names for more details.

Basically, avoid any identifier that starts with an underscore. If you really want to start with an underscore, it must be followed by a lowercase letter or a digit, and it should be in some namespace/class etc and not in the global namespace.

kaihsin commented 2 weeks ago

Yes. In fact. The best way to deal with this guard is just use #pragma once

kaihsin commented 2 weeks ago

We need to keep an eye on this when doing refactor. But for now I would'nt bother much about this.

ianmccul commented 2 weeks ago

We need to keep an eye on this when doing refactor. But for now I would'nt bother much about this.

Yes I agree, it obviously isn't causing problems now and the chances of a problem in the future are small, BUT it might lead to some crazy hard-to-diagnose problem, so it is good to avoid such identifiers in new code, and take the opportunity to remove existing uses when refactoring code.