Genivia / ugrep

NEW ugrep 7.0: a more powerful, ultra fast, user-friendly, compatible grep. Includes a TUI, Google-like Boolean search with AND/OR/NOT, fuzzy search, hexdumps, searches (nested) archives (zip, 7z, tar, pax, cpio), compressed files (gz, Z, bz2, lzma, xz, lz4, zstd, brotli), pdfs, docs, and more
https://ugrep.com
BSD 3-Clause "New" or "Revised" License
2.64k stars 112 forks source link

Build: missing type / include `ptrdiff_t` on older systems #277

Closed barsnick closed 1 year ago

barsnick commented 1 year ago

Environment: ugrep version 4.0.0 - the issue was introduced with this version. RHEL7 / EPEL7 in a Fedora COPR mock environment.

Hi, when building ugrep-4.0.0 on EPEL7 / RHEL7 with g++-4.8.5 (yeah ;-)) and glibc-2.17, the build fails due to a missing type / include:

g++ -std=gnu++11 -DHAVE_CONFIG_H -I. -I..  -I../include -msse2 -DHAVE_AVX2 -I/usr/include -I/usr/include -I/usr/include -I/usr/include -I/usr/include -I/usr/include -I/usr/include -I/usr/include -I/usr/include -I/usr/include -I/usr/include -I/usr/include  -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches   -m64 -mtune=generic -mavx2 -c -o libreflex_a-simd_avx2.o `test -f 'simd_avx2.cpp' || echo './'`simd_avx2.cpp
simd_avx2.cpp: In function 'size_t reflex::simd_nlcount_avx2(const char*&, const char*)':
simd_avx2.cpp:51:28: error: expected type-specifier before 'ptrdiff_t'
   while ((reinterpret_cast<ptrdiff_t>(s) & 0x1f) != 0)
                            ^
simd_avx2.cpp:51:28: error: expected '>' before 'ptrdiff_t'
simd_avx2.cpp:51:28: error: expected '(' before 'ptrdiff_t'
simd_avx2.cpp:51:28: error: 'ptrdiff_t' was not declared in this scope
simd_avx2.cpp:51:28: note: suggested alternatives:
In file included from /usr/include/c++/4.8.2/cstdio:41:0,
                 from ../include/reflex/error.h:40,
                 from ../include/reflex/convert.h:40,
                 from ../include/reflex/absmatcher.h:55,
                 from simd_avx2.cpp:37:
/usr/include/c++/4.8.2/x86_64-redhat-linux/bits/c++config.h:1858:28: note:   'std::ptrdiff_t'
   typedef __PTRDIFF_TYPE__ ptrdiff_t;
                            ^
/usr/include/c++/4.8.2/x86_64-redhat-linux/bits/c++config.h:1858:28: note:   'std::ptrdiff_t'
simd_avx2.cpp:52:5: error: expected ')' before 'n'
     n += (*s++ == '\n');
     ^

It works fine on all newer systems.

ptrdiff_t was introduced to gcc earlier, so this may be a compiler-specific thing (saying: perhaps gcc wasn't doing it correctly back then).

Relevant packages:

 gcc                           x86_64   4.8.5-44.el7            base       16 M
 gcc-c++                       x86_64   4.8.5-44.el7            base      7.2 M
 glibc                         x86_64   2.17-326.el7_9          updates   3.6 M
 glibc-common                  x86_64   2.17-326.el7_9          updates    12 M
 glibc-devel                   x86_64   2.17-326.el7_9          updates   1.1 M
 glibc-headers                 x86_64   2.17-326.el7_9          updates   691 k
 libstdc++                     x86_64   4.8.5-44.el7            base      306 k
 libstdc++-devel               x86_64   4.8.5-44.el7            base      1.5 M
genivia-inc commented 1 year ago

I've tested on various systems before release. Forgot ptrdiff_t is relatively new. It can be replaced with a different type. We just need to cast a pointer to a (small) integer to test memory address alignment. Like so:

while ((reinterpret_cast<uint64_t>(s) & 0x3f) != 0)

I'll prepare an update.

barsnick commented 1 year ago

I've tested on various systems before release. Forgot ptrdiff_t is relatively new. It can be replaced with a different type. We just need to cast a pointer to a (small) integer to test memory address alignment. Like so:

while ((reinterpret_cast<uint64_t>(s) & 0x3f) != 0)

Ah, I see, but even uint64_t would be in stddef.h.

I'll prepare an update.

I have already hacked at the problem and managed to get it to build, so I made a PR. You were faster in replying. ;-)

genivia-inc commented 1 year ago

Committed an update. Do you consider this update of the two files satisfactory?

barsnick commented 1 year ago

Committed an update. Do you consider this update of the two files satisfactory?

No, as mentioned, that should expose the same problem. But I can test anyway if you wish.

genivia-inc commented 1 year ago

Approved.