Re: clang compiler warnings
On 2024-02-27 3:32 am, Werner LEMBERG wrote:
Looking at some MacPorts log files from Mac OS 14.2.1 (which uses the
system's `clang` compiler) I see
```
flower/include/flower-proto.hh:26:30:
warning: implicit conversion changes signedness:
'int' to 'const vsize' (aka 'const unsigned long')
[-Wsign-conversion]
inline constexpr vsize VPOS (-1);
^~
```
for almost all C++ fies, making the log file quite noisy. Dan, any
idea how to fix this gracefully?
Since vsize is just a typedef of size_t, would it make sense to use
SIZE_MAX when defined VPOS instead of a manual cast of negative one?
-- Aaron Hill
Re: clang compiler warnings
I'd just make the cast explicit:
inline constexpr vsize VPOS (vsize(-1));
It's cleaner and requires less updates as the code evolves.
However, I can't reproduce it, what compiler flags are you using?
I'm using -Wall and clang is quiet about this use case
*/tmp %* g++ -Wall -std=c++17 xxx.cc
*/tmp %* cat xxx.cc
#include
inline constexpr size_t x(-1);
int main()
{
std::cout << x << std::endl;
}
*/tmp %* g++ --version
Apple clang version 15.0.0 (clang-1500.1.0.2.5)
Target: x86_64-apple-darwin23.3.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin
*/tmp %* ./a.out
18446744073709551615
*/tmp %*
L
On Tue, 27 Feb 2024, 12:57 Aaron Hill, wrote:
> On 2024-02-27 3:32 am, Werner LEMBERG wrote:
> > Looking at some MacPorts log files from Mac OS 14.2.1 (which uses the
> > system's `clang` compiler) I see
> >
> > ```
> > flower/include/flower-proto.hh:26:30:
> > warning: implicit conversion changes signedness:
> >'int' to 'const vsize' (aka 'const unsigned long')
> >[-Wsign-conversion]
> > inline constexpr vsize VPOS (-1);
> > ^~
> > ```
> >
> > for almost all C++ fies, making the log file quite noisy. Dan, any
> > idea how to fix this gracefully?
>
> Since vsize is just a typedef of size_t, would it make sense to use
> SIZE_MAX when defined VPOS instead of a manual cast of negative one?
>
>
> -- Aaron Hill
>
>
Re: clang compiler warnings
> However, I can't reproduce it, what compiler flags are you using? According to the MacPorts log file it is the clang compiler that comes with XCode 15.0.1 on MaOS 14.2.1. Looking at https://gist.github.com/yamaya/2924292 this corresponds to Apple clang version 15.0.0 (clang-1500.0.40.1) The used compiler flags are ``` CXXFLAGS='-pipe -Os -stdlib=libc++ \ -isysroot/Library/Developer/CommandLineTools/SDKs/MacOSX14.sdk \ -arch arm64' ``` > I'm using -Wall and clang is quiet about this use case [...] > > Apple clang version 15.0.0 (clang-1500.1.0.2.5) Interesting. Maybe this warning annoyed too much people so that it got removed in the more recent clang version you are using. > Since vsize is just a typedef of size_t, would it make sense to use > SIZE_MAX when defined VPOS instead of a manual cast of negative one? This sounds sensible to me, since `size_t` is always unsigned... Werner
Re: clang compiler warnings
On 2024-02-27 07:14, Luca Fascione wrote: inline constexpr vsize VPOS (vsize(-1)); OK, but don't repeat the type. inline constexpr auto VPOS = vsize (-1); Personally, I don't like flower's aliases related to vsize, ssize, and Real -- I'd rather just use standard names -- but I remember that there was some resistance to changing them the last time I suggested it. -- Dan
Re: clang compiler warnings
On Tue, Feb 27, 2024 at 4:49 AM Werner LEMBERG wrote: > According to the MacPorts log file it is the clang compiler that comes > with XCode 15.0.1 on MaOS 14.2.1. [...] this corresponds to > > Apple clang version 15.0.0 (clang-1500.0.40.1) > > The used compiler flags are > > ``` > CXXFLAGS='-pipe -Os -stdlib=libc++ \ > -isysroot/Library/Developer/CommandLineTools/SDKs/MacOSX14.sdk \ > -arch arm64' > ``` > These are not all the compilation flags for sure: at a minimum -std=c++17 (or somesuch) is required to compile inline constexpr, so there's got to be something more going on here. Anyways, I found out that if you use -Weverything, you get what I'd expect from -Wall (gah). And indeed with that I get the warning like you do with the original line while with the cast right at the value it compiles quietly. > Since vsize is just a typedef of size_t, would it make sense to use > > SIZE_MAX when defined VPOS instead of a manual cast of negative one? > > This sounds sensible to me, since `size_t` is always unsigned... > I don't recommend it: if the typedef of vsize changes, SIZE_MAX won't be an appropriate value, as it's not covariant with the definition of vsize. This is just going from one bug today to a bug tomorrow. L -- Luca Fascione
Re: clang compiler warnings
On Tue, Feb 27, 2024 at 4:55 AM Dan Eble wrote: > On 2024-02-27 07:14, Luca Fascione wrote: > > inline constexpr vsize VPOS (vsize(-1)); > > OK, but don't repeat the type. > > inline constexpr auto VPOS = vsize (-1); > Well. To that, yes and no. I read it better in the original way, and repeating the type at a distance of less than 10 characters is ok IMO (in that you obviously see it if/when you're changing the code, so parsing the 'auto' is substantially more cognitive overhead than reasoning about the type being there twice). Besides, depending on how good code intelligence your editor has, I want to see the type in the popup ballon describing VPOS at the usage size, and finding 'auto' there will just make me frustrated. In my school of thought auto should only be used where it's necessary (like inside templates) or in places where the type declaration is so convoluted that reading it is not useful (like std::map::const_reverse_iterator::value_type kinda things). > Personally, I don't like flower's aliases related to vsize, > ssize, and Real -- I'd rather just use standard names -- > but I remember that there was some resistance to changing > them the last time I suggested it. > Redeclaring size_t is a thing of a very distant past, I've worked with APIs that had declarations for int and float (and even void in one, yuck). It's not a modern way of working, but if the thing is external you should use types that match the API definitions, that'll make it easier to keep your code correct when the typedefs move (like when you change platform, the whole long vs long long thing on linux/windows is a complete nightmare, for example) I know that at the moment a native build on windows is not on the cards, but it seems to me that adding friction gratuitously is just an unwise thing to do. But to me, if you have a variable that is intended to be passed as a parameter to a thing that wants a foo_t, even if you know that foo_t is a typedef for bar_t, your variable should be declared as foo_t, you'll find bugs faster that way, because you're pulling the detection of mismatched types closer to code you can act upon. L -- Luca Fascione
Re: clang compiler warnings
>> OK, but don't repeat the type. >> >> inline constexpr auto VPOS = vsize (-1); >> > > Well. To that, yes and no. I read it better in the original way, > [...] I'm on Dan's side here: Using 'auto' tells me that the right-hand side's type is exactly the one on the left-hand side, and I can stop thinking about it immediately. Werner
Re: clang compiler warnings
Forgive me, but I don't see it your way. Reasoning about the type upon seeing the definition or declaration is the unlikely case to occur. The likely case we should be reasoning about is recalling the type upon seeing a usage. In that case either you remember what VPOS is, or you open the file with the declaration, or you use your editor's support to tell you. In cases like this, the _declaration_ is 'auto', not 'vsize'. And now it's not useful to you. IF code intelligence or a header show you more, you can piece it together in our head, if not, you're making poor use of time. And the saving of not typing vsize twice is just not there. That's a one second thing once in a blue moon, whereas every time you see VPOS you'll have to know what type it is. (ok, maybe half the time, but it's still a lot more than the churn expected on the type it's declared as, come on) L (btw, why is VPOS defined as -1? this seems like a weird name for a default value, wouldn't something more descriptive of "default" or "unset" be more appropriate here?) On Tue, Feb 27, 2024 at 5:42 AM Werner LEMBERG wrote: > > >> OK, but don't repeat the type. > >> > >> inline constexpr auto VPOS = vsize (-1); > >> > > > > Well. To that, yes and no. I read it better in the original way, > > [...] > > I'm on Dan's side here: Using 'auto' tells me that the right-hand > side's type is exactly the one on the left-hand side, and I can stop > thinking about it immediately. > > > Werner > -- Luca Fascione
Re: clang compiler warnings
> inline constexpr auto VPOS = vsize (-1); This is now part of https://gitlab.com/lilypond/lilypond/-/merge_requests/2267 Werner
