-
Notifications
You must be signed in to change notification settings - Fork 589
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
base: blead
Are you sure you want to change the base?
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
No underscore is needed, as this is an internal macro.
6295e86
to
9163e9f
Compare
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