[Mingw-w64-public] [PATCH] stralign: cast ua_wcschr and ua_wcsrchr returns to wchar

Ping, looks like it slipped unnoticed.

------ Original Message ------
Subject: [PATCH] stralign: cast ua_wcschr and ua_wcsrchr returns to
wchar_t *
Date: Mon, 3 Apr 2017 14:01:00 +0200
To: Mingw-w64-public
From: Mateusz MikuÅa

Post by Mateusz MikuÅa
I made another attempt to build libc++ with mingw-w64 and stumbled
cannot initialize return object of type 'PUWSTR_C' (aka 'wchar_t *')
with an rvalue of type 'const wchar_t *' if(WSTR_ALIGNED(String))
return wcschr((PCWSTR)String,Character);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cannot initialize return object of type 'PUWSTR_C' (aka 'wchar_t *')
with an rvalue of type 'const wchar_t *' if(WSTR_ALIGNED(String))
return wcsrchr((PCWSTR)String,Character); You can see here function
that should return PUWSTR_C (aka ||'wchar_t *') returns ||'const wchar_t *|' instead.
This patch solved error for Clang and caused no errors or warning for GCC.

Kai Tietz

2017-04-05 12:26:50 UTC

Hello Mateusz,

could you please re-attach, or simply inline patch to this mail?

Thanks in advance,
Kai

Post by Mateusz MikuÅa
Ping, looks like it slipped unnoticed.
------ Original Message ------
Subject: [PATCH] stralign: cast ua_wcschr and ua_wcsrchr returns to
wchar_t *
Date: Mon, 3 Apr 2017 14:01:00 +0200
To: Mingw-w64-public
From: Mateusz Mikuła

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Mingw-w64-public mailing list
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public

Mateusz Mikuła

2017-04-05 13:06:41 UTC

Hmm, somehow Sourcefore has lost track on this thread (you can see
patch on the mailing list website).

This time sending it as both attachment and text.

From f3febf9542a6664eedc384b0d2bf63411477e141 Mon Sep 17 00:00:00 2001
From: Mateusz Mikula <***@gmail.com>
Date: Mon, 3 Apr 2017 13:36:36 +0200
Subject: [PATCH] stralign: cast ua_wcschr and ua_wcsrchr returns to
wchar_t *

Clang doesn't allow implicit conversion form 'const wchar_t *' to
'wchar_t *'

Signed-off-by: Mateusz Mikula <***@gmail.com>
---
mingw-w64-headers/include/stralign.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mingw-w64-headers/include/stralign.h
b/mingw-w64-headers/include/stralign.h
index 9b5637d6..cdb81438 100644
--- a/mingw-w64-headers/include/stralign.h
+++ b/mingw-w64-headers/include/stralign.h
@@ -118,11 +118,11 @@ extern "C" {

#ifndef __CRT__NO_INLINE
__CRT_INLINE PUWSTR_C ua_wcschr(PCUWSTR String,WCHAR Character) {
- if(WSTR_ALIGNED(String)) return wcschr((PCWSTR)String,Character);
+ if(WSTR_ALIGNED(String)) return (wchar_t
*)wcschr((PCWSTR)String,Character);
return (PUWSTR_C)uaw_wcschr(String,Character);
}
__CRT_INLINE PUWSTR_C ua_wcsrchr(PCUWSTR String,WCHAR Character) {
- if(WSTR_ALIGNED(String)) return wcsrchr((PCWSTR)String,Character);
+ if(WSTR_ALIGNED(String)) return (wchar_t
*)wcsrchr((PCWSTR)String,Character);
return (PUWSTR_C)uaw_wcsrchr(String,Character);
}
#if defined(__cplusplus) && defined(_WConst_Return)
--
2.12.1

------ Original Message ------
Subject: Re: [Mingw-w64-public] [PATCH] stralign: cast ua_wcschr and
ua_wcsrchr returns to wchar_t *
Date: Wed, 5 Apr 2017 14:26:50 +0200
To: Mingw-w64-public
From: Kai Tietz

Post by Kai Tietz
Hello Mateusz,
could you please re-attach, or simply inline patch to this mail?
Thanks in advance,
Kai

Mateusz Mikuła

2017-04-05 13:35:51 UTC

On second though casting to "PUWSTR_C" which is could be more appealing.
Attaching improved patch.

------ Original Message ------
Subject: Re: [Mingw-w64-public] [PATCH] stralign: cast ua_wcschr and
ua_wcsrchr returns to wchar_t *
Date: Wed, 5 Apr 2017 15:06:41 +0200
To: Mingw-w64-public
From: Mateusz MikuÅa

Post by Mateusz MikuÅa
Hmm, somehow Sourcefore has lost track on this thread (you can see
patch on the mailing list website).
This time sending it as both attachment and text.
From f3febf9542a6664eedc384b0d2bf63411477e141 Mon Sep 17 00:00:00 2001
Date: Mon, 3 Apr 2017 13:36:36 +0200
Subject: [PATCH] stralign: cast ua_wcschr and ua_wcsrchr returns to
wchar_t *
Clang doesn't allow implicit conversion form 'const wchar_t *' to
'wchar_t *'
---
mingw-w64-headers/include/stralign.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/mingw-w64-headers/include/stralign.h
b/mingw-w64-headers/include/stralign.h
index 9b5637d6..cdb81438 100644
--- a/mingw-w64-headers/include/stralign.h
+++ b/mingw-w64-headers/include/stralign.h
@@ -118,11 +118,11 @@ extern "C" {
#ifndef __CRT__NO_INLINE
__CRT_INLINE PUWSTR_C ua_wcschr(PCUWSTR String,WCHAR Character) {
- if(WSTR_ALIGNED(String)) return wcschr((PCWSTR)String,Character);
+ if(WSTR_ALIGNED(String)) return (wchar_t
*)wcschr((PCWSTR)String,Character);
return (PUWSTR_C)uaw_wcschr(String,Character);
}
__CRT_INLINE PUWSTR_C ua_wcsrchr(PCUWSTR String,WCHAR Character) {
- if(WSTR_ALIGNED(String)) return wcsrchr((PCWSTR)String,Character);
+ if(WSTR_ALIGNED(String)) return (wchar_t
*)wcsrchr((PCWSTR)String,Character);
return (PUWSTR_C)uaw_wcsrchr(String,Character);
}
#if defined(__cplusplus) && defined(_WConst_Return)
--
2.12.1
------ Original Message ------
Subject: Re: [Mingw-w64-public] [PATCH] stralign: cast ua_wcschr and
ua_wcsrchr returns to wchar_t *
Date: Wed, 5 Apr 2017 14:26:50 +0200
To: Mingw-w64-public
From: Kai Tietz

Post by Kai Tietz
Hello Mateusz,
could you please re-attach, or simply inline patch to this mail?
Thanks in advance,
Kai

Kai Tietz

2017-04-05 13:36:53 UTC

Hmm, using here "wchar_t *" as cast looks wrong. Actually we should
use anyway PUWSTR_C instead.
Nevertheless we can have here a const/none-const conversion (means
const specifiers for C-runtime function isn't regarded right?). I
would suggest to introduce a union-cast instead to avoid further
warnings instead.

Regards,
Kai

Post by Kai Tietz
Hello Mateusz,
could you please re-attach, or simply inline patch to this mail?
Thanks in advance,
Kai

Mateusz Mikuła

2017-04-05 13:51:48 UTC

Post by Kai Tietz
Hmm, using here "wchar_t *" as cast looks wrong. Actually we should
use anyway PUWSTR_C instead.

I noticed it a bit too late and sent another patch casting to PUWSTR_C.

Post by Kai Tietz
Nevertheless we can have here a const/none-const conversion (means
const specifiers for C-runtime function isn't regarded right?). I
would suggest to introduce a union-cast instead to avoid further
warnings instead.

Conversion from const pointer to normal pointer is definitely unsafe but
that's probably what GCC just did.
I'm unsure what you mean by "union-cast" but you can commit your fix.

Liu Hao

2017-04-05 17:09:21 UTC

Post by Mateusz MikuÅa
Conversion from const pointer to normal pointer is definitely unsafe but
that's probably what GCC just did.
I'm unsure what you mean by "union-cast" but you can commit your fix.

Take a look at the prototype of the `strchr` function in the C standard
library. Its first parameter is of type `const char *`, but its return
value is of type `char *`.

Only by having such a declaration can the following two function calls
compile, since C does not allow overloading:

char s1[] = "hello world!";
const char *s2 = s2;
char * p1 = strchr(s1, '!'); // [1]
const char *p2 = strchr(s2, '!'); // [2]

--
Best regards,
LH_Mouse

Kai Tietz

2017-04-06 09:48:24 UTC

A cast via union looks like this:
type1 *foo(const type1 *my_const_ptr)
{
union {
type1 *t1;
const type1 *ct1;
} v;
v.ct1 = my_const_ptr;
return v.t1;
}

The advantage of such a pattern is that no type conversion
errors/warnings are shown. So for casting from const to none-const,
this variant is to be preferred. (and it works for C, and C++!!!)

Cheers,
Kai

Post by Kai Tietz
Hmm, using here "wchar_t *" as cast looks wrong. Actually we should
use anyway PUWSTR_C instead.

I noticed it a bit too late and sent another patch casting to PUWSTR_C.

Conversion from const pointer to normal pointer is definitely unsafe but
that's probably what GCC just did.
I'm unsure what you mean by "union-cast" but you can commit your fix.
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Mingw-w64-public mailing list
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public

Norbert Pfeiler

2017-04-06 11:38:48 UTC

I’m pretty sure that cast via union is UB in C++ whereas C-style casting
away constness isn’t (only writing to the resulting object would be) but it
may result in compilers issuing warnings about style (because C++ has
const_cast).

Best, Norbert.

Post by Kai Tietz
type1 *foo(const type1 *my_const_ptr)
{
union {
type1 *t1;
const type1 *ct1;
} v;
v.ct1 = my_const_ptr;
return v.t1;
}
The advantage of such a pattern is that no type conversion
errors/warnings are shown. So for casting from const to none-const,
this variant is to be preferred. (and it works for C, and C++!!!)
Cheers,
Kai

Post by Kai Tietz
Hmm, using here "wchar_t *" as cast looks wrong. Actually we should
use anyway PUWSTR_C instead.

I noticed it a bit too late and sent another patch casting to PUWSTR_C.

Conversion from const pointer to normal pointer is definitely unsafe but
that's probably what GCC just did.
I'm unsure what you mean by "union-cast" but you can commit your fix.

------------------------------------------------------------------------------

Post by Mateusz MikuÅa
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Mingw-w64-public mailing list
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public

Kai Tietz

2017-04-06 12:47:54 UTC

2017-04-06 13:38 GMT+02:00 Norbert Pfeiler

Post by Norbert Pfeiler
I’m pretty sure that cast via union is UB in C++ whereas C-style casting
away constness isn’t (only writing to the resulting object would be) but it
may result in compilers issuing warnings about style (because C++ has
const_cast).
Best, Norbert.

True. The reason why we prefer such patter is that it works in both
languages as desired. Otherwise we would be in need to write
different variants for the languages C/C++. This looks backward, and
it still doesn't make sure that there might be warnings emitted for C,
or C++ doing const casts.

Cheers,
Kai

Post by Norbert Pfeiler

Post by Kai Tietz
Hmm, using here "wchar_t *" as cast looks wrong. Actually we should
use anyway PUWSTR_C instead.

I noticed it a bit too late and sent another patch casting to PUWSTR_C.

Conversion from const pointer to normal pointer is definitely unsafe but
that's probably what GCC just did.
I'm unsure what you mean by "union-cast" but you can commit your fix.

------------------------------------------------------------------------------

Liu Hao

2017-04-06 13:06:03 UTC

Post by Kai Tietz
True. The reason why we prefer such patter is that it works in both
languages as desired. Otherwise we would be in need to write
different variants for the languages C/C++. This looks backward, and
it still doesn't make sure that there might be warnings emitted for C,
or C++ doing const casts.

The warning about casting away const-ness can be enabled on GCC6 using
`-Wcast-qual` (I am not sure about other versions). But as long as the
header file is in a system directory GCC is smart enough to not warn
about it even if our user enable that warning, unless `-Wsystem-headers`
is also given.

--
Best regards,
LH_Mouse

Kai Tietz

2017-04-06 13:13:03 UTC

Norbert Pfeiler

2017-04-07 00:11:25 UTC

Wasn’t LH_Mouse’s point that even if the warning is explicitly turned on it
wouldn’t be shown to the user?

The situation for unions is different in C and C++: (I don’t think that
it’s just about constness changes anything)
http://stackoverflow.com/questions/11373203

Best, Norbert.

True, that there are flags to turn such warnings off. Each compiler
has its own variants for this. But well, why we should rely on such
things. As you already mentioned, are our headers system ones, and
therefore as silent as possible. Nothing is worse in experience to
users, if they suddenly see warnings, they don't want to see. And
that all because they enabled some warning flags for "their" code.
Cheers,
Kai
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Mingw-w64-public mailing list
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public

Liu Hao

2017-04-07 03:46:47 UTC

Post by Norbert Pfeiler
Wasn’t LH_Mouse’s point that even if the warning is explicitly turned on it
wouldn’t be shown to the user?
The situation for unions is different in C and C++: (I don’t think that
it’s just about constness changes anything)
http://stackoverflow.com/questions/11373203

Nice link. Nevertheless, I don't think most people care about UB unless
UB bites them (that is, unless UB refuses to work silently). XD

--
Best regards,
LH_Mouse

Norbert Pfeiler

2017-04-07 11:33:17 UTC

That’s a kinda sad stance for a compiler support project, don’t you think?

Best, Norbert.

Post by Norbert Pfeiler
Wasn’t LH_Mouse’s point that even if the warning is explicitly turned on

Post by Norbert Pfeiler
wouldn’t be shown to the user?
The situation for unions is different in C and C++: (I don’t think that
it’s just about constness changes anything)
http://stackoverflow.com/questions/11373203

Nice link. Nevertheless, I don't think most people care about UB unless
UB bites them (that is, unless UB refuses to work silently). XD
--
Best regards,
LH_Mouse
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Mingw-w64-public mailing list
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public

David Grayson

2017-04-07 15:21:00 UTC

Post by Kai Tietz
type1 *foo(const type1 *my_const_ptr)
{
union {
type1 *t1;
const type1 *ct1;
} v;
v.ct1 = my_const_ptr;
return v.t1;
}

Yes, that is sad, and it seems like just a matter of time before a C++
compiler looks at the code above and reasons that the write to ct1 can be
optimized away because there is no code that ever reads from ct1. Thus the
read from t1 will be recognized as undefined behavior (probably called a
poison value in LLVM) and anything could happen.

What was the problem with Mateusz's patch? I think people are implying it
results in compiler warnings, but I thought casting a pointer to a pointer
usually doesn't give any warnings.

--David

On Fri, Apr 7, 2017 at 4:33 AM, Norbert Pfeiler <

Post by Kai Tietz
That’s a kinda sad stance for a compiler support project, don’t you think?
Best, Norbert.

Post by Norbert Pfeiler
Wasn’t LH_Mouse’s point that even if the warning is explicitly turned

Post by Liu Hao
it

------------------

Post by Liu Hao
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Mingw-w64-public mailing list
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public

------------------------------------------------------------
------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Mingw-w64-public mailing list
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public

David Grayson

2017-04-07 15:22:31 UTC

Oh, it would be a warning with -Wcast-qual, ok, got it. --David

Post by Kai Tietz
type1 *foo(const type1 *my_const_ptr)
{
union {
type1 *t1;
const type1 *ct1;
} v;
v.ct1 = my_const_ptr;
return v.t1;
}

Yes, that is sad, and it seems like just a matter of time before a C++
compiler looks at the code above and reasons that the write to ct1 can be
optimized away because there is no code that ever reads from ct1. Thus the
read from t1 will be recognized as undefined behavior (probably called a
poison value in LLVM) and anything could happen.
What was the problem with Mateusz's patch? I think people are implying it
results in compiler warnings, but I thought casting a pointer to a pointer
usually doesn't give any warnings.
--David
On Fri, Apr 7, 2017 at 4:33 AM, Norbert Pfeiler <

Post by Kai Tietz
That’s a kinda sad stance for a compiler support project, don’t you think?
Best, Norbert.

Post by Norbert Pfeiler
Wasn’t LH_Mouse’s point that even if the warning is explicitly turned

Post by Liu Hao
it

Post by Norbert Pfeiler
wouldn’t be shown to the user?
The situation for unions is different in C and C++: (I don’t think

that

Post by Norbert Pfeiler
it’s just about constness changes anything)
http://stackoverflow.com/questions/11373203

------------------

------------------------------------------------------------
------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Mingw-w64-public mailing list
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public

Kai Tietz

2017-04-07 16:31:30 UTC

Post by Kai Tietz
type1 *foo(const type1 *my_const_ptr)
{
union {
type1 *t1;
const type1 *ct1;
} v;
v.ct1 = my_const_ptr;
return v.t1;
}

LOL ... if a compiler modifies a technical valid language construct,
and produces by this optimization something it needs to warn about,
then the compiler has a bug. And well, you should be sad to use such
a compiler, as it is broken in many aspects.

Post by David Grayson
What was the problem with Mateusz's patch? I think people are implying it
results in compiler warnings, but I thought casting a pointer to a pointer
usually doesn't give any warnings.

David Grayson

2017-04-07 17:00:26 UTC

I did read the top two answers in the link that Norbert posted:

http://stackoverflow.com/questions/11373203/accessing-inactive-union-member-and-undefined-behavior

The first answer (from ecatmur) indicates that this kind of conversion with
a union would be undefined behavior in C++, but not C. I'm not sure what
else to read, except the latest C++ standard, which was quoted heavily in
the answer.

The second answer (from Bo Persson) provides a useful link to the GCC
documentation which makes me think that GCC provides additional guarantees
beyond what the standard says, and so this kind of conversion would
actually be safe, in both C and C++. Since GCC behaves that way, clang
probably would too. So yeah, the union conversion is probably fine because
of the design of the compilers we care about.

I think casting would work too. When LH_Mouse's reasoned about why the
warnings would not be a problem, Kai said: "Each compiler has its own
variants for this. But well, why we should rely on such things." Are you
more OK with relying on the details of compiler behavior for union
conversions than the details of compiler behavior for warnings?

--David

Post by Kai Tietz
type1 *foo(const type1 *my_const_ptr)
{
union {
type1 *t1;
const type1 *ct1;
} v;
v.ct1 = my_const_ptr;
return v.t1;
}

the

Post by David Grayson
read from t1 will be recognized as undefined behavior (probably called a
poison value in LLVM) and anything could happen.

Post by David Grayson
What was the problem with Mateusz's patch? I think people are implying

Post by David Grayson
results in compiler warnings, but I thought casting a pointer to a

pointer

Post by David Grayson
usually doesn't give any warnings.

The problem is that you seems to see an UB, where actually none is.
Please try read a bit about C++, compatibility to C, and what, and how
a union on pointer types are treated in a union (especially the part
about integer scalars & pointers).
~Kai
PS: As you already recognized, yes the issue is to avoid warnings OP
doesn't expect them.
------------------------------------------------------------
------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Mingw-w64-public mailing list
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public

Mateusz Mikuła

2017-04-07 18:35:54 UTC

I tested these _out of box_ cases:

No patch:
GCC: silent | Clang: 32bit silent, 64bit error

Cast to "PUWSTR_C":
GCC: silent | Clang: silent

Union cast:
GCC: silent | Clang: silent

If you want to use Union cast You should create fix because I have no
idea about MinGW-w64 coding standards in case like this.
Testing this is very tricky since I could reproduce it only when
building 64bit libc++ but to get to this stage enormous amount of hacks
have been created (workaround missing mingw-w64 defines, wrong code for
mingw, etc.)
Basically both compilers tell it comes from "#include<Windows.h>" from
few cpp files:

|In file included from
D:/projekty/msys2/MINGW-packages/mingw-w64-clang/src/llvm-3.9.0.src/projects/libcxx/src/chrono.cpp:40:
In file included from
D:\projekty\msys2\clang\msys64\mingw64\x86_64-w64-mingw32\include\windows.h:114:
D:\projekty\msys2\clang\msys64\mingw64\x86_64-w64-mingw32\include\stralign.h:121:37:
error: cannot initialize return object of type 'PUWSTR_C' (aka 'wchar_t
*') with an rvalue of type 'const wchar_t *' ... |

Regards,
Mateusz

------ Original Message ------
Subject: Re: [Mingw-w64-public] [PATCH] stralign: cast ua_wcschr and
ua_wcsrchr returns to wchar_t *
Date: Fri, 7 Apr 2017 10:00:26 -0700
To: Mingw-w64-public
From: David Grayson

Post by David Grayson
http://stackoverflow.com/questions/11373203/accessing-inactive-union-member-and-undefined-behavior
The first answer (from ecatmur) indicates that this kind of conversion with
a union would be undefined behavior in C++, but not C. I'm not sure what
else to read, except the latest C++ standard, which was quoted heavily in
the answer.
The second answer (from Bo Persson) provides a useful link to the GCC
documentation which makes me think that GCC provides additional guarantees
beyond what the standard says, and so this kind of conversion would
actually be safe, in both C and C++. Since GCC behaves that way, clang
probably would too. So yeah, the union conversion is probably fine because
of the design of the compilers we care about.
I think casting would work too. When LH_Mouse's reasoned about why the
warnings would not be a problem, Kai said: "Each compiler has its own
variants for this. But well, why we should rely on such things." Are you
more OK with relying on the details of compiler behavior for union
conversions than the details of compiler behavior for warnings?
--David

Post by Kai Tietz
type1 *foo(const type1 *my_const_ptr)
{
union {
type1 *t1;
const type1 *ct1;
} v;
v.ct1 = my_const_ptr;
return v.t1;
}

the

Post by David Grayson
read from t1 will be recognized as undefined behavior (probably called a
poison value in LLVM) and anything could happen.

Post by David Grayson
What was the problem with Mateusz's patch? I think people are implying

Post by David Grayson
results in compiler warnings, but I thought casting a pointer to a

pointer

Post by David Grayson
usually doesn't give any warnings.

Liu Hao

2017-04-08 05:39:02 UTC

That is not exactly the case here.

At the moment `const T *` and `T *` are not layout-compatible types
([basic.types]/11). However, they shall
have the same value representation and alignment requirements
([basic.compound]/3). If they were layout-compatible, it would be
perfectly defined to write one and read the other, because both are part
of the common initial sequence ([class.mem]/22, 23).

After all, we generally don't care about this problem because there is a
huge amount of code that relies on such UB (`LARGE_INTEGER` for example,
and `sockaddr`, `sockaddr_in`, `sockaddr_in6` etc). There is no reason
for a compiler to break such code.

--
Best regards,
LH_Mouse

Kai Tietz

2017-04-06 13:09:33 UTC

Post by Kai Tietz
2017-04-06 13:38 GMT+02:00 Norbert Pfeiler

Hmm, undefined behavior??? no, this is pretty well defined behavior.
In C++ world a union cast is pretty much the same as a
reinterpret_cast (or a C style cast without conversion operator). And
a const_cast is just a smaller set out of the realm of
reinterpret_cast.

Cheers,
Kai

Mateusz Mikuła

2017-06-04 10:15:40 UTC

Anything to improve?

From 05bc4cbc93f5f9942fc28a578dc1afa68d69daa2 Mon Sep 17 00:00:00 2001
From: Mateusz Mikula <***@gmail.com>
Date: Sun, 4 Jun 2017 11:33:22 +0200
Subject: [PATCH] cast ua_wcschr and ua_wcsrchr returns when WSTR_ALIGNED is
true

Clang doesn't allow implicit conversion form "const wchar_t *" to
"PUWSTR_C" (aka wchar_t *)

Signed-off-by: Mateusz Mikula <***@gmail.com>
---
mingw-w64-headers/include/stralign.h | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/mingw-w64-headers/include/stralign.h
b/mingw-w64-headers/include/stralign.h
index 9b5637d6..4b157c27 100644
--- a/mingw-w64-headers/include/stralign.h
+++ b/mingw-w64-headers/include/stralign.h
@@ -117,12 +117,23 @@ extern "C" {
size_t ua_wcslen(PCUWSTR String);

#ifndef __CRT__NO_INLINE
+ union {
+ wchar_t *wcharPointer;
+ const wchar_t *constWcharPointer;
+ } cast;
+
__CRT_INLINE PUWSTR_C ua_wcschr(PCUWSTR String,WCHAR Character) {
- if(WSTR_ALIGNED(String)) return wcschr((PCWSTR)String,Character);
+ if(WSTR_ALIGNED(String)) {
+ cast.constWcharPointer = wcschr((PCWSTR)String,Character);
+ return cast.wcharPointer;
+ }
return (PUWSTR_C)uaw_wcschr(String,Character);
}
__CRT_INLINE PUWSTR_C ua_wcsrchr(PCUWSTR String,WCHAR Character) {
- if(WSTR_ALIGNED(String)) return wcsrchr((PCWSTR)String,Character);
+ if(WSTR_ALIGNED(String)) {
+ cast.constWcharPointer = wcsrchr((PCWSTR)String,Character);
+ return cast.wcharPointer;
+ }
return (PUWSTR_C)uaw_wcsrchr(String,Character);
}
#if defined(__cplusplus) && defined(_WConst_Return)
--
2.12.1

------ Original Message ------
Subject: Re: [Mingw-w64-public] [PATCH] stralign: cast ua_wcschr and
ua_wcsrchr returns to wchar_t *
Date: Thu, 6 Apr 2017 11:48:24 +0200
To: Mingw-w64-public
From: Kai Tietz

Post by Kai Tietz
Hmm, using here "wchar_t *" as cast looks wrong. Actually we should
use anyway PUWSTR_C instead.

I noticed it a bit too late and sent another patch casting to PUWSTR_C.

Norbert Pfeiler

2017-06-04 12:40:05 UTC

1st point was to use PUWSTR_C instead of wchar_t * for the cast, still
stands.
2nd point was to »to avoid further warnings« and i would like to – once
again – vote to avoid union casts.

As far as i know there is no reason to believe that -Wcast-qual (as well as
-Wsystem-headers) will be a default warning anytime soon. And even then it
would be just a warning but still standards-conforming code (which is also
a reason for it not to become a default warning).

A C-style cast is perfectly appropriate here, no union needed.

FTR, it’s not only Clang which doesn’t accept the pre-patch code, GCC
errors too.

Best, Norbert.

Post by Mateusz MikuÅa
Anything to improve?
From 05bc4cbc93f5f9942fc28a578dc1afa68d69daa2 Mon Sep 17 00:00:00 2001
Date: Sun, 4 Jun 2017 11:33:22 +0200
Subject: [PATCH] cast ua_wcschr and ua_wcsrchr returns when WSTR_ALIGNED is
true
Clang doesn't allow implicit conversion form "const wchar_t *" to
"PUWSTR_C" (aka wchar_t *)
---
mingw-w64-headers/include/stralign.h | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/mingw-w64-headers/include/stralign.h
b/mingw-w64-headers/include/stralign.h
index 9b5637d6..4b157c27 100644
--- a/mingw-w64-headers/include/stralign.h
+++ b/mingw-w64-headers/include/stralign.h
@@ -117,12 +117,23 @@ extern "C" {
size_t ua_wcslen(PCUWSTR String);
#ifndef __CRT__NO_INLINE
+ union {
+ wchar_t *wcharPointer;
+ const wchar_t *constWcharPointer;
+ } cast;
+
__CRT_INLINE PUWSTR_C ua_wcschr(PCUWSTR String,WCHAR Character) {
- if(WSTR_ALIGNED(String)) return wcschr((PCWSTR)String,Character);
+ if(WSTR_ALIGNED(String)) {
+ cast.constWcharPointer = wcschr((PCWSTR)String,Character);
+ return cast.wcharPointer;
+ }
return (PUWSTR_C)uaw_wcschr(String,Character);
}
__CRT_INLINE PUWSTR_C ua_wcsrchr(PCUWSTR String,WCHAR Character) {
- if(WSTR_ALIGNED(String)) return wcsrchr((PCWSTR)String,Character);
+ if(WSTR_ALIGNED(String)) {
+ cast.constWcharPointer = wcsrchr((PCWSTR)String,Character);
+ return cast.wcharPointer;
+ }
return (PUWSTR_C)uaw_wcsrchr(String,Character);
}
#if defined(__cplusplus) && defined(_WConst_Return)
--
2.12.1
------ Original Message ------
Subject: Re: [Mingw-w64-public] [PATCH] stralign: cast ua_wcschr and
ua_wcsrchr returns to wchar_t *
Date: Thu, 6 Apr 2017 11:48:24 +0200
To: Mingw-w64-public
From: Kai Tietz

Post by Kai Tietz
Hmm, using here "wchar_t *" as cast looks wrong. Actually we should
use anyway PUWSTR_C instead.

I noticed it a bit too late and sent another patch casting to PUWSTR_C.

Conversion from const pointer to normal pointer is definitely unsafe but
that's probably what GCC just did.
I'm unsure what you mean by "union-cast" but you can commit your fix.

------------------------------------------------------------------------------

------------------------------------------------------------------------------

Post by Kai Tietz
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Mingw-w64-public mailing list
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public

Mateusz Mikuła

2017-06-04 13:31:05 UTC

I sent patch casting to PUWSTR_C half hour after first message when I
noticed how stupid it was before.

Should still apply cleanly (attachment has wrong name):
https://sourceforge.net/p/mingw-w64/mailman/message/35770400/

------ Original Message ------
Subject: Re: [Mingw-w64-public] [PATCH] stralign: cast ua_wcschr and
ua_wcsrchr returns to wchar_t *
Date: Sun, 04 Jun 2017 12:40:05 +0000
To: Mingw-w64-public
From: Norbert Pfeiler

Post by Norbert Pfeiler
1st point was to use PUWSTR_C instead of wchar_t * for the cast, still
stands.
2nd point was to Â»to avoid further warningsÂ« and i would like to â once
again â vote to avoid union casts.
As far as i know there is no reason to believe that -Wcast-qual (as well as
-Wsystem-headers) will be a default warning anytime soon. And even then it
would be just a warning but still standards-conforming code (which is also
a reason for it not to become a default warning).
A C-style cast is perfectly appropriate here, no union needed.
FTR, itâs not only Clang which doesnât accept the pre-patch code, GCC
errors too.
Best, Norbert.

Post by Kai Tietz
Hmm, using here "wchar_t *" as cast looks wrong. Actually we should
use anyway PUWSTR_C instead.

I noticed it a bit too late and sent another patch casting to PUWSTR_C.

Conversion from const pointer to normal pointer is definitely unsafe but
that's probably what GCC just did.
I'm unsure what you mean by "union-cast" but you can commit your fix.

------------------------------------------------------------------------------

------------------------------------------------------------------------------

Mateusz Mikuła

2017-07-01 21:40:34 UTC

Ping

Post by Mateusz MikuÅa
I sent patch casting to PUWSTR_C half hour after first message
when I noticed how stupid it was before.
https://sourceforge.net/p/mingw-w64/mailman/message/35770400/
------ Original Message ------
Subject: Re: [Mingw-w64-public] [PATCH] stralign: cast
ua_wcschr
and ua_wcsrchr returns to wchar_t *
Date: Sun, 04 Jun 2017 12:40:05 +0000
To: Mingw-w64-public
From: Norbert Pfeiler

Post by Mateusz MikuÅa
Anything to improve?
From 05bc4cbc93f5f9942fc28a578dc1afa68d69daa2 Mon Sep 17 00:00:00 2001
Date: Sun, 4 Jun 2017 11:33:22 +0200
Subject: [PATCH] cast ua_wcschr and ua_wcsrchr returns when WSTR_ALIGNED is
true
Clang doesn't allow implicit conversion form "const wchar_t *" to
"PUWSTR_C" (aka wchar_t *)
---
mingw-w64-headers/include/stralign.h | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/mingw-w64-headers/include/stralign.h
b/mingw-w64-headers/include/stralign.h
index 9b5637d6..4b157c27 100644
--- a/mingw-w64-headers/include/stralign.h
+++ b/mingw-w64-headers/include/stralign.h
@@ -117,12 +117,23 @@ extern "C" {
size_t ua_wcslen(PCUWSTR String);
#ifndef __CRT__NO_INLINE
+ union {
+ wchar_t *wcharPointer;
+ const wchar_t *constWcharPointer;
+ } cast;
+
__CRT_INLINE PUWSTR_C ua_wcschr(PCUWSTR String,WCHAR
Character) {
- if(WSTR_ALIGNED(String)) return
wcschr((PCWSTR)String,Character);
+ if(WSTR_ALIGNED(String)) {
+ cast.constWcharPointer = wcschr((PCWSTR)String,Character);
+ return cast.wcharPointer;
+ }
return (PUWSTR_C)uaw_wcschr(String,Character);
}
__CRT_INLINE PUWSTR_C ua_wcsrchr(PCUWSTR String,WCHAR
Character) {
- if(WSTR_ALIGNED(String)) return
wcsrchr((PCWSTR)String,Character);
+ if(WSTR_ALIGNED(String)) {
+ cast.constWcharPointer =
wcsrchr((PCWSTR)String,Character);
+ return cast.wcharPointer;
+ }
return (PUWSTR_C)uaw_wcsrchr(String,Character);
}
#if defined(__cplusplus) && defined(_WConst_Return)
--
2.12.1
------ Original Message ------
Subject: Re: [Mingw-w64-public] [PATCH] stralign: cast ua_wcschr and
ua_wcsrchr returns to wchar_t *
Date: Thu, 6 Apr 2017 11:48:24 +0200
To: Mingw-w64-public
From: Kai Tietz

Post by Kai Tietz
type1 *foo(const type1 *my_const_ptr)
{
union {
type1 *t1;
const type1 *ct1;
} v;
v.ct1 = my_const_ptr;
return v.t1;
}
The advantage of such a pattern is that no type conversion
errors/warnings are shown. So for casting from const to none-
const,
this variant is to be preferred. (and it works for C, and C++!!!)
Cheers,
Kai

Post by Kai Tietz
Hmm, using here "wchar_t *" as cast looks
wrong. Actually we should
use anyway PUWSTR_C instead.

I noticed it a bit too late and sent another
patch casting to PUWSTR_C.

Post by Kai Tietz
Nevertheless we can have here a const/none-
const conversion (means
const specifiers for C-runtime function isn't regarded right?). I
would suggest to introduce a union-cast instead to avoid further
warnings instead.

Conversion from const pointer to normal pointer
is definitely unsafe but
that's probably what GCC just did.
I'm unsure what you mean by "union-cast" but you can commit your fix.

---------------------------------------------------------
---------------------