[PATCH] support ptrdiff_t if it has the same storage size as int, but does not alias it

[Robin Haberkorn]

Hello everybody!

As promised, this patch fixes NetBSD 10 builds on ARMv6.
But it's not NetBSD-specific.

Scintilla assumed that ptrdiff_t and int alias (are the same time) if
their storage size is equal. With this patch we no longer make such
assumptions which should improve code portability.

It also removes a hack for 32-bit Haiku which was apparently
causing the same issues [1] I was experiencing and would probably
be broken by now due to the same ptrdiff_t-assumptions in CellBuffer.cxx.

See the patch header for more details.

Due to my CI systems I can say that this at least does not break Linux
x86_64 GCC and Clang, i586 GCC and MinGW MSVCRT (Win64) GCC builds.

Best regards,
Robin

PS: Scintilla v5.5.7 is still building on NetBSD 10 ARMv6 with the default
GCC. It's a GCC v10.5.0 and not a GCC v7 as I was erronously writing
earlier.

PPS: Musing about C++'s std::is_same<X,Y> construct, I realized that we
can do the same in C11 now as well - at least with some ubiquitous GCC
extensions:

#define TYPE_EQUALS(X, Y) ({ \
X __dummy; \
_Generic(__dummy, Y : true, default : false); \
})

With some minor modifications, you could use it for static assertions as
well. That's actually an useful application of _Generic() for once...

[1] https://groups.google.com/g/scintilla-interest/c/xPXquJUIXo8/m/BLXBpTTgBwAJ

[Neil Hodgson]

I’ll look at this when I return from vacation. Trip was extended a bit but should return to coding in October.

Neil

[Neil Hodgson]

Robin Haberkorn:

> ... their storage size is equal. With this patch we no longer make such

> assumptions which should improve code portability.
>
> It also removes a hack for 32-bit Haiku which was apparently
> causing the same issues [1] I was experiencing and would probably
> be broken by now due to the same ptrdiff_t-assumptions in CellBuffer.cxx.

This produces a large number of warnings in RunStyles.cxx from MSVC
2022 17.14.122 for the x86 target.

1>G:\u\hg\scintilla\src\RunStyles.cxx(51,22): warning C4244:
'argument': conversion from 'DISTANCE' to 'ptrdiff_t', possible loss
of data
1>G:\u\hg\scintilla\src\RunStyles.cxx(51,22): warning C4244: with
1>G:\u\hg\scintilla\src\RunStyles.cxx(51,22): warning C4244: [
1>G:\u\hg\scintilla\src\RunStyles.cxx(51,22): warning C4244:
DISTANCE=__int64
1>G:\u\hg\scintilla\src\RunStyles.cxx(51,22): warning C4244: ]
1> G:\u\hg\scintilla\src\RunStyles.cxx(51,22):
1> the template instantiation context (the oldest one first) is
1> G:\u\hg\scintilla\src\RunStyles.cxx(327,62):
1> see reference to class template instantiation
'Scintilla::Internal::RunStyles<__int64,int>' being compiled
1> G:\u\hg\scintilla\src\RunStyles.cxx(44,66):
1> while compiling class template member function 'DISTANCE
Scintilla::Internal::RunStyles<DISTANCE,int>::SplitRun(DISTANCE)'
1> with
1> [
1> DISTANCE=__int64
1> ]

Neil

[Robin Haberkorn]

> This produces a large number of warnings in RunStyles.cxx from MSVC
> 2022 17.14.122 for the x86 target.
>
> 1>G:\u\hg\scintilla\src\RunStyles.cxx(51,22): warning C4244:
> 'argument': conversion from 'DISTANCE' to 'ptrdiff_t', possible loss
> of data
> 1>G:\u\hg\scintilla\src\RunStyles.cxx(51,22): warning C4244: with
> 1>G:\u\hg\scintilla\src\RunStyles.cxx(51,22): warning C4244: [
> 1>G:\u\hg\scintilla\src\RunStyles.cxx(51,22): warning C4244:
> DISTANCE=__int64
> 1>G:\u\hg\scintilla\src\RunStyles.cxx(51,22): warning C4244: ]
> 1> G:\u\hg\scintilla\src\RunStyles.cxx(51,22):
> 1> the template instantiation context (the oldest one first) is
> 1> G:\u\hg\scintilla\src\RunStyles.cxx(327,62):
> 1> see reference to class template instantiation
> 'Scintilla::Internal::RunStyles<__int64,int>' being compiled
> 1> G:\u\hg\scintilla\src\RunStyles.cxx(44,66):
> 1> while compiling class template member function 'DISTANCE
> Scintilla::Internal::RunStyles<DISTANCE,int>::SplitRun(DISTANCE)'
> 1> with
> 1> [
> 1> DISTANCE=__int64
> 1> ]
>

Is the log truncated? Perhaps you could attach it.
Can you find out what's the definition of ptrdiff_t on this platform?

[Neil Hodgson]

Robin Haberkorn:

> ...

> Is the log truncated? Perhaps you could attach it.

Yes, the log repeated the above for multiple methods. Attached complete log.

> Can you find out what's the definition of ptrdiff_t on this platform?

It's complicated by being in multiple headers and configs but "Go To
Definition" in the IDE shows this (elided), where __int64 is a
built-in.

_WIN64 is defined for x64 target and not for x86 target.

#ifdef _WIN64
typedef __int64 ptrdiff_t;
#else
typedef int ptrdiff_t;
#endif

Unfortunately, it is difficult to develop portable code without using
the platform standard tools - MSVC on Win32 and Xcode on macOS.

Neil

[Robin Haberkorn]

On Thu Oct 2, 2025 at 03:05:04 GMT +03, Neil Hodgson wrote:
> It's complicated by being in multiple headers and configs but "Go To
> Definition" in the IDE shows this (elided), where __int64 is a
> built-in.
>
> _WIN64 is defined for x64 target and not for x86 target.
>
> #ifdef _WIN64
> typedef __int64 ptrdiff_t;
> #else
> typedef int ptrdiff_t;
> #endif
>

So the warnings are generated for the x86 target (_WIN64 is not defined).
Actually not really surprising since long long could be degraded into int.
I wonder why this doesn't cause warnings on Clang/GCC.

But I don't see how we could work around this, except by adding static
casts in all relevant places to silence the warnings.
Preprocessor #ifs cannot really be used to take out the unnecessary
template instantiations.

What if we just switched to dynamic instantiation for RunStyles?

Best regards,
Robin

[Neil]

Robin Haberkorn:

 

What if we just switched to dynamic instantiation for RunStyles?

Without the explicit instantiations at the bottom of RunStyles.cxx, MSVC builds fail with many 'unresolved external symbol's.

My recollection is that builds for other platforms also failed.

Neil

[Robin Haberkorn]

On Sun Oct 5, 2025 at 08:42:33 GMT +03, Neil wrote:
> Without the explicit instantiations at the bottom of RunStyles.cxx, MSVC
> builds fail with many 'unresolved external symbol's.
>
> My recollection is that builds for other platforms also failed.

I will give it a try. All methods would have to be moved into the header
of course.

What if we used int64_t consistently instead of ptrdiff_t?
Sci::Position would be 64-bit even on 32-bit platforms.
I guess this wouldn't be tolerable?
Perhaps we could also detect 64-bit and conditionally typedef to
int32_t or int64_t. We then would at least know what it aliases to,
therefore fixing the #if (INT64_MAX != INT_MAX) approach.

[Neil]

Robin Haberkorn:

 

What if we used int64_t consistently instead of ptrdiff_t?
Sci::Position would be 64-bit even on 32-bit platforms.
I guess this wouldn't be tolerable?

That would be excessive.

Maybe invert the conditions and prioritise ptrdiff_t before int, then avoiding int with added compile time tests where there are currently only runtime checks of largeDocument.

Neil

[Robin Haberkorn]

On Sun Oct 5, 2025 at 08:42:33 GMT +03, Neil wrote:

> Without the explicit instantiations at the bottom of RunStyles.cxx, MSVC
> builds fail with many 'unresolved external symbol's.
>
> My recollection is that builds for other platforms also failed.
>

Another thing to consider: What if we got rid of the RunStyles<int, X>
instantiation, leaving only RunStyles<ptrdiff_t, X>?
In other words, what if we fixed DISTANCE to ptrdiff_t?
Would we loose much on those platforms where int != ptrdiff_t,
ie. on 64-bit architectures?

[Neil]

Robin Haberkorn:

 

Would we loose much on those platforms where int != ptrdiff_t,
ie. on 64-bit architectures?

That wastes more memory. There is insufficient benefit in supporting *BSD for anything like this.

Neil 

[Robin Haberkorn]

On Sun Oct 5, 2025 at 13:01:03 GMT +03, Neil wrote:
> That wastes more memory. There is insufficient benefit in supporting *BSD
> for anything like this.

I very much doubt that the problem is NetBSD-specific.
It never happend on x86_64 FreeBSD btw.
On the other hand, it has already occurred on Haiku as well.
The code as it is makes non-portable assumptions.
Unless of course these platforms somehow violate the
C standard with their ptrdiff_t definitions, which
we should check as well.

But if you don't want to waste any more memory,
we will find a better solution.

Best regards,
Robin

[Neil]

Robin Haberkorn:

But if you don't want to waste any more memory,
we will find a better solution.

This is a tiny problem and some of the proposals here, like forcing 32-bit systems to store and manipulate 64-bit positions, have an unreasonably large impact.

It would be simpler to -D__HAIKU__ on systems that need it.

Neil

[Robin Haberkorn]

On Sun Oct 5, 2025 at 12:28:36 GMT +03, Robin Haberkorn wrote:
> On Sun Oct 5, 2025 at 08:42:33 GMT +03, Neil wrote:
>> Without the explicit instantiations at the bottom of RunStyles.cxx, MSVC
>> builds fail with many 'unresolved external symbol's.
>>
>> My recollection is that builds for other platforms also failed.
>
> I will give it a try. All methods would have to be moved into the header
> of course.

Here's a version of the patch that uses implicit instantiation of RunStyles.
It applies on top of rel-5-5-7.
Seems to work as well.
Does it compile on MSVC?

What's you policy on includes in headers? You don't seem to transitively
pull in all dependencies in the headers. For instance if class A in a.h is derived
from class B in header b.h, you don't seem to #include "b.h" in a.h.
Therefore I didn't move any of the includes from RunStyles.cxx into
RunStyles.h.

Best regards,
Robin

[Robin Haberkorn]

On Sun Oct 5, 2025 at 17:35:04 GMT +03, Robin Haberkorn wrote:
> ...

> The code as it is makes non-portable assumptions.
> Unless of course these platforms somehow violate the
> C standard with their ptrdiff_t definitions, which
> we should check as well.
>

I checked the C23 standard - at least the last free draft (ISO/IEC 9899:2024
(en) — N3220 working draft). ptrdiff_t is just some signed integer large
enough to hold the difference of two pointers. So it's basically a black box.
On 32-bit architectures it could be both `int` and `long` (or `long int`, `signed int`,
`signed long int` etc).
It doesn't seem to be a very wide-spread typedef, though.
It could perhaps also be worked around on the affected platforms by passing
`-fno-strict-aliasing` via CXXFLAGS. But that may come with a performance
penalty.

[Robin Haberkorn]

On Wed Oct 8, 2025 at 19:07:12 GMT +03, Robin Haberkorn wrote:
> Here's a version of the patch that uses implicit instantiation of RunStyles.
> It applies on top of rel-5-5-7.
> Seems to work as well.
> Does it compile on MSVC?

I've taken the patch into SciTECO's CI.
It builds on amd64 FreeBSD and cross compiles to Windows and OS X.
All of these are Clang.

Furthermore it builds on all sorts of Debian and
RPM-based Linux distributions on a variety of architectures
including armv7, s390x, aarch64, i586 and ppc64le. [1]
Of those only i586 is a 32-bit architecture, though.
All of these were built with GCC.

[1]: https://build.opensuse.org/package/show/home:rhaberkorn:sciteco:UNSTABLE/sciteco

[Neil]

Robin Haberkorn:

 

Here's a version of the patch that uses implicit instantiation of RunStyles.
It applies on top of rel-5-5-7.
Seems to work as well.

Still looks to be too much of a change when a simple -D__HAIKU__ can avoid most of it. It increases the proportion of code in headers leading to worse build times and is a bad direction to move in.

 

Does it compile on MSVC?

It compiles on MSVC 2022 with the .vcproj inside the IDE and with nmake. I haven't tried older MSVC.

What's you policy on includes in headers?

Avoid include statements in headers. There's scripts/HeaderCheck.py to ensure consistent header inclusion. I would like to move to C++20 modules.

Neil

[Robin Haberkorn]

On Fri Oct 10, 2025 at 00:08:54 GMT +03, Neil wrote:
> Still looks to be too much of a change when a simple -D__HAIKU__ can avoid
> most of it. It increases the proportion of code in headers leading to worse
> build times and is a bad direction to move in.

Even if we could assume that ptrdiff_t is always long on 32-bit Haiku
and therefore doesn't alias int that would be a lot trickier on NetBSD where
apparently some ports use long and others int.
Also, do we really know that on Haiku? They support armv7 as well.

Am I supposed to write some kind of custom Autoconf compile test to check
for the aliasability of ptrdiff_t with int if sizeof(ptrdiff_t) == sizeof(int)
and pass in -D__HAIKU__ on all platforms that do?
Ugly, but possible. Then we should at the very least give it a more
neutral name like RUNSTYLES_NEEDS_PTRDIFF or something.

Robin

[doyle]

I know this message isn't remotely about the topic at hand, but didn't
know how else to handle this. My name is Jennifer Whisenant. I am the
daughter of Doyle Whisenant, who was a member of this group. He passed
away on August 30, 2025. I accessed his email and found these group
messages. I just wanted to inform everyone of his passing. Thank you for
your time.

[Neil Hodgson]

Thank you Jennifer, for letting us know about your father. Doyle was a
member of our community for around 20 years.

I am saddened to hear this news.

Neil

[Neil Hodgson]

Robin Haberkorn:

> ... on NetBSD where

> apparently some ports use long and others int.

If a platform isn't interested in consistency then it will lose in the
market. It is unreasonable to expect other projects to adapt to this.

> Also, do we really know that on Haiku? They support armv7 as well.

That will be up to the Haiku people.

> Am I supposed to write some kind of custom Autoconf compile test to check
> for the aliasability of ptrdiff_t with int if sizeof(ptrdiff_t) == sizeof(int)
> and pass in -D__HAIKU__ on all platforms that do?

That's one approach. You could loop over the options in a shell build
script and drop out on success.

> Then we should at the very least give it a more
> neutral name like RUNSTYLES_NEEDS_PTRDIFF or something.

A generically named check can be added if done in a way that preserves Haiku.

I suspect that it is possible to templatize the instantiations over
is_convertible_v, possibly using enable_if, replacing the #if but that
needs someone more motivated than me.

Neil

[Robin Haberkorn]

On Mon Oct 13, 2025 at 02:50:03 GMT +02, Neil Hodgson wrote:
> If a platform isn't interested in consistency then it will lose in the
> market. It is unreasonable to expect other projects to adapt to this.

I disagree. NetBSD and Haiku aren't violating the C/C++ standards. Also,
authors of future libcs/platforms might well not be aware that you think that
ptrdiff_t should alias int under certain conditions and cause the same issue.

But it's your project and your decision.

> I suspect that it is possible to templatize the instantiations over
> is_convertible_v, possibly using enable_if, replacing the #if but that
> needs someone more motivated than me.

Unfortunately you can't. It doesn't work in explicit instantiations.
Perhaps declaring RunStyles<int> __attribute__((weak)) might have
worked - but it's neither compiler-, nor platform-agnostic.

So I prepared another version of my patch that introduces
PTRDIFF_DOESNT_ALIAS_INT, which you may have to pass in via CXXFLAGS.

For SciTECO I wrote an Autoconf macro AX_PTRDIFF_ALIASES_INT [1]
to check for this condition.

Yours sincerely,
Robin

[1]: https://git.fmsbw.de/sciteco/tree/m4/ax_ptrdiff_aliases_int.m4

[Neil Hodgson]

Robin:

> I disagree. NetBSD and Haiku aren't violating the C/C++ standards. Also,
> authors of future libcs/platforms might well not be aware that you think that
> ptrdiff_t should alias int under certain conditions and cause the same issue.

The standards allow lots of weird choices like defining NULL to not
be all 0 bits. Platforms that behave differently to the big 3 get to
live with the consequences of their choices.

32-bit systems are becoming quite rare. On macOS, Scintilla went
64-bit only in 2014 and support effort would be decreased by only
supporting 64-bit platforms.

> So I prepared another version of my patch that introduces
> PTRDIFF_DOESNT_ALIAS_INT, which you may have to pass in via CXXFLAGS.

Compile-time options should be listed in the table in the "Building
Scintilla" section at the end of doc/ScintillaDoc.html.

Neil