-
Notifications
You must be signed in to change notification settings - Fork 599
Change names with leading underscores to be legal #23592
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
Conversation
54bcb5b
to
3fe7128
Compare
On a simple unthreaded build on Linux, this p.r. is failing for me here:
|
3fe7128
to
f13df7f
Compare
This commit should be immediately rejected. It is breaking over thirty years of C source code compatibility of Perl 5. this commit is as much vandalism as attempting a commit changing the P5P repo to the Raku 6 .tar.gz. KHW needs to find a commercial C compiler and it's .h files on some on Linux BSD Unix Windows OS first that shows how the underscores in P5 C Token names are harmful or break the building the interp first. |
Perhaps you are entirely correct in your assessment. But you make this claim without any concrete evidence that would allow someone else to evaluate its accuracy. |
bulk88, I think your suggestion here is that somebody downstream might be using the Also, calling it vandalism -- the deliberate destruction or defacement of property -- is not helpful. Nobody is going to believe that Karl is showing up here to vandalize perl. |
Unless I made a mistake, no code outside core control should be using these names. That is in fact the point of the underscores, to mark these as special, internal. |
f13df7f
to
6295e86
Compare
6295e86
to
9163e9f
Compare
No underscore is needed, as this is an internal macro.
9163e9f
to
8b2de44
Compare
These were merged via 4bb3572 |
This continues the process started in Perl#23592 to change names with leading underscores to be legal C. See that p.r. or 4bb3572 for extensive discussion. This commit simply moves the leading underscore to be trailing
This continues the process started in Perl#23592 to change names with leading underscores to be legal C. See that p.r. or 4bb3572 for extensive discussion. This commit simply moves the leading underscore to be trailing Some of these may actually be legal, and don't need to be moved, as leading underscores are only illegal in symbols with file scope. But it was easier to just do a global substitute, and does no harm to move them all to be trailing.
It is undefined behavior in C for a symbol name to begin with an underscore followed by a capital letter or a second
underscore. It is also undefined behavior for a symbol at file scope to begin with an underscore followed by a lowercase letter. C++ further restricts any leading underscore in file scope. Our headers need to be able to compile with C++, so the restriction for headers is never begin a symbol with an underscore. Some people compile core perl using C++, and sometimes we move symbols into headers. Therefore a reasonable rule is to not begin file-scoped symbols with an underscore.
There are a hundred-ish symbols in the perl core that do begin with an underscore, not all of them currently in file scope. This series of commits renames almost all of them to instead have a single trailing underscore (thus retaining a visual clue that these are special in some way). It doesn't do the ones that already have a pull request in progress for (submitted, or a WIP on my box), nor for the ones that are generated by scripts, as those are a bit more complicated. The symbols changed here are the ones that are simple to do.
Some of the symbols changed here are ones I introduced, out of ignorance of the C standard's wording on these.
Each commit changes a single symbol
The consequences of them being the way they are now are minimal, Only if a C implementation changed to use one of our symbols would there be a symbol clash, or we got ported to a new C compiler. The odds of these being problesm are fairly low. Yet they are non-zero, and we do have existing symbol clashes with other software that they have had to work around.
I got tired of running into these symbols and being reminded that these aren't strictly legal, and that I was responsible for some of them. So I ended up with this p.r., removing nearly all of them at once.
The commits here change even symbols that aren't currently file-level, hence legal. I did this for several reasons