perl5 icon indicating copy to clipboard operation
perl5 copied to clipboard

Encode the misc SV_* flags using bitshifts rather than decimal integers

Open leonerd opened this issue 4 years ago • 5 comments

The bitwise pattern, and the gap at (1<<3), are more visible this way.

leonerd avatar Sep 12 '21 21:09 leonerd

This is better than the decimals, but I'd find it easier to read if they were just hex constants (as most of the other flags are, even in sv.h).

Generally the only reason I look at these values is because I'm sitting in gdb trying to check what set of flags a given number corresponds to. It's not a huge effort to convert the shifts into hex mentally, but still more than none.

hvds avatar Sep 12 '21 23:09 hvds

On 9/12/21 5:10 PM, Hugo van der Sanden wrote:

This is better than the decimals, but I'd find it easier to read if they were just hex constants (as most of the other flags are, even in sv.h).

Generally the only reason I look at these values is because I'm sitting in gdb trying to check what set of flags a given number corresponds to. It's not a huge effort to convert the shifts into hex mentally, but still more than none.

I prefer the way the P.R. is currently written.

I use these to see which bit is set, and to see how many bits are in use to see if anything is available. This gives that info at a glance. I don't find the mental conversion into hex problematic

khwilliamson avatar Sep 13 '21 15:09 khwilliamson

The goal was:

The bitwise pattern, and the gap at (1<<3), are more visible this way.

more visible than decimal...

So write them as hex?

0x0001
0x0002
0x0004
0x0010
0x0020
0x0040
0x0080
0x0100
0x0200
0x0400
0x0800
0x1000
0x2000
0x4000
0x8000

This missing one is more obvious than decimal?

nwc10 avatar Sep 28 '21 07:09 nwc10

On Mon, 13 Sept 2021 at 01:10, Hugo van der Sanden @.***> wrote:

This is better than the decimals, but I'd find it easier to read if they were just hex constants (as most of the other flags are, even in sv.h).

Generally the only reason I look at these values is because I'm sitting in gdb trying to check what set of flags a given number corresponds to. It's not a huge effort to convert the shifts into hex mentally, but still more than none.

Cant you just put the hex value next to the bitshift form in a comment? Then we can have our cake and eat it too.

Yves

-- perl -Mre=debug -e "/just|another|perl|hacker/"

demerphq avatar Sep 28 '21 07:09 demerphq

Is there a reason now to not merge this?

khwilliamson avatar Jun 08 '22 16:06 khwilliamson

I have rebased this and modified it to show the hex and decimal values as a comment, and force pushed it. I also decided to "document" the unused bit with a define SV_FLAG_BIT3_UNUSED.

I will merge this once it passes CI.

demerphq avatar Feb 08 '23 06:02 demerphq

On Tue, Feb 07, 2023 at 11:00:14PM -0800, Yves Orton wrote:

I have rebased this and modified it to show the hex and decimal values as a comment, and force pushed it. I also decided to "document" the unused bit with a define SV_FLAG_BIT3_UNUSED.

Personally I'm not a fan of this approach for unused bits.

Leaving a blank line holding just a comment like /* unused */ stands out (to me, anyway), while a solid block of defines looks like they're all in use, and you'd have to look carefully (and know to look) to spot that SV_FLAG_BIT3_UNUSED is actually an unused bit. Then you'd have to worry whether such a define is indicating an unused bit, or whether it's actually used (or was used) as a flag to some sv_foo() function to indicate something about skipping a bit when processing the SV.

-- A major Starfleet emergency breaks out near the Enterprise, but fortunately some other ships in the area are able to deal with it to everyone's satisfaction. -- Things That Never Happen in "Star Trek" #13

iabyn avatar Feb 19 '23 19:02 iabyn

and you'd have to look carefully (and know to look) to spot that SV_FLAG_BIT3_UNUSED is actually an unused bit.

Its not a hill I want to die on, so I don't mind removing it. But the contrary view is the it is much easier to manage in terms of ensuring things are well formed, and automating things like showing the value different ways. With a "gap" its harder for a tool to see that the defines are all related, and things like that. I think if we have a well defined convention, then it should be pretty obvious its not a real bit flag. Maybe SV_FLAG_BIT3_UNUSED isnt perfect. It could be: 'XXXXXXX_SV_B3' or something like that. Its easier to teach humans a convention than it is to teach a program about human irregularity.

I would like us to automate more and more of our maintenance. I see lists of defines where the first K have N characters, then the next K have N+1, etc, which IMO makes it hard to read. The more regular we are about these lists the more reasonable it is to write a script that keeps them well formed or to detect when they are not.

demerphq avatar Feb 19 '23 23:02 demerphq