Skip to content

Conversation

@soerengrunewald
Copy link
Collaborator

Why

There are some really annoying warnings which are spammed if compiling with clang.
Since we use gul includes all over, every include will emit these warnings.

[Why]
If compiled with clang and -Wconversion warnings enable, the
compiler will complain:
> gul14/span.h:218:42: warning: implicit conversion changes signedness: 'long' to 'std::size_t' (aka 'unsigned long') [-Wsign-conversion]
>   218 |         : storage_(first_elem, last_elem - first_elem)
>       |           ~~~~~~~~             ~~~~~~~~~~^~~~~~~~~~~~

Signed-off-by: Soeren Grunewald <soeren.grunewald@desy.de>
Signed-off-by: Soeren Grunewald <soeren.grunewald@desy.de>
[Why]
If compiled with clang, the compiler complains:
> include/gul14/date.h:912:41: warning: identifier '_d' preceded by whitespace in a literal operator declaration is deprecated [-Wdeprecated-literal-operator]
>   912 | constexpr gul14::date::day  operator "" _d(unsigned long long d) noexcept;
>       |                             ~~~~~~~~~~~~^~
>       |                             operator""_d
> include/gul14/date.h:913:41: warning: identifier '_y' preceded by whitespace in a literal operator declaration is deprecated [-Wdeprecated-literal-operator]
>   913 | constexpr gul14::date::year operator "" _y(unsigned long long y) noexcept;
>       |                             ~~~~~~~~~~~~^~
>       |                             operator""_y
> include/gul14/date.h:1813:13: warning: identifier '_d' preceded by whitespace in a literal operator declaration is deprecated [-Wdeprecated-literal-operator]
>  1813 | operator "" _d(unsigned long long d) noexcept
>       | ~~~~~~~~~~~~^~
>       | operator""_d
> include/gul14/date.h:1820:13: warning: identifier '_y' preceded by whitespace in a literal operator declaration is deprecated [-Wdeprecated-literal-operator]
>  1820 | operator "" _y(unsigned long long y) noexcept
>       | ~~~~~~~~~~~~^~
>       | operator""_y

Signed-off-by: Soeren Grunewald <soeren.grunewald@desy.de>
@Finii
Copy link
Member

Finii commented Jun 11, 2025

My understanding is that GUL14 has been sunsetted and will be removed soon.
I'm not sure any minute spent on GUL14 is justified.
Maybe I'm wrong?

What is the timeline, @alt-graph

@Finii
Copy link
Member

Finii commented Jun 11, 2025

I mean, I see changes in 'date' and 'sliding buffer', maybe the fixes should be applied to GUL17.

forward-porting the fixes from GUL14 you did here to GUL17 is kind of ... unneeded work, if you could fix GUL17 directly instead.

@alt-graph
Copy link
Member

My understanding is that GUL14 has been sunsetted and will be removed soon. I'm not sure any minute spent on GUL14 is justified. Maybe I'm wrong?

What is the timeline, @alt-graph

I guess we'll keep GUL14 around for compatibility for a long time, so I would give its maintenance a low (but nonzero) priority. New features should primarily go into GUL17.

If you ask for a timeline, we can probably declare GUL14 as unmaintained in a year or two, meaning we leave it on Github but leave a "you're on your own" sign.

[Why]
If compiled with clang, the compiler complains:
> include/gul14/string_view.h:576:30: warning: implicit conversion changes signedness: 'std::size_t' (aka 'unsigned long') to 'streamsize' (aka 'long') [-Wsign-conversion]
>   576 |         os.write(fill_chars, n);
>       |            ~~~~~             ^

Signed-off-by: Soeren Grunewald <soeren.grunewald@desy.de>
[Why]
If compiled with clang, which complains about:
> include/gul14/string_util.h:154:27: warning: implicit conversion changes signedness: 'typename iterator_traits<unsigned char *>::difference_type' (aka 'long') to 'const std::size_t' (aka 'const unsigned long') [-Wsign-conversion]
>   154 |     const std::size_t n = std::distance(begin, end);
>       |                       ~   ^~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Soeren Grunewald <soeren.grunewald@desy.de>
Signed-off-by: Soeren Grunewald <soeren.grunewald@desy.de>
Signed-off-by: Soeren Grunewald <soeren.grunewald@desy.de>
@soerengrunewald soerengrunewald force-pushed the bugfix/resolve-clang-compiler-warnings branch from 56588c2 to 7c20d26 Compare June 11, 2025 09:20
@alt-graph
Copy link
Member

@soerengrunewald: I guess there is a higher chance that someone will review this if you break it apart into smaller pieces. Everybody can find five minutes for reviewing, but >15 for a library that we are slowly phasing out is a hard sell.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants