MLXXXp/Arduboy2

Consider removing uses of keyword 'register'

Pharap opened this issue · 4 comments

As far as I'm aware, register has only ever existed in C++ for reasons of backwards compatibility with C and has no actual meaning.

It's not likely to cause a problem any time soon, but it has the potential to cause an issue if anyone needed to compile the library with a C++17 compiler because register was removed in C++17.

That aside, it might cause confusion for readers of the source code who aren't familiar with it, considering that it's outdated and fairly esoteric.

In case it's relevant, the reason this suddenly occurred to me was because drawFastHLine was recently mentioned on the forum and I decided to have a peek at its implementation, which is how I noticed one such occurance of register:

Arduboy2/src/Arduboy2.cpp

Lines 583 to 587 in 7dc88be

// buffer pointer plus row offset + x offset
register uint8_t *pBuf = sBuffer + ((y / 8) * WIDTH) + x;
// pixel mask
register uint8_t mask = 1 << (y & 7);

I wasn't aware that the register keyword was used anywhere. I could only find it in the two places mentioned above. I'll remove them in the next release but I don't think a new release specifically to change this is warranted, unless it affects code size or speed.

It's a patch change at best (since not compiling for C++17 could be considered a bug) but it's not likely to cause an issue any time soon, it's just something I noticed in passing. Chances are this is probably the only place it's being used.

I've committed a change removing the two known register keywords. It will be picked up in the next official release. Anyone forking or otherwise using the master head with also get the change.

Thanks @Pharap