You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2021/10/31 17:45:25 UTC
[GitHub] [incubator-nuttx] xiaoxiang781216 opened a new pull request #4754: Revert "sim: Specify -fshort-wchar as NuttX wchar_t is 16-bit"
xiaoxiang781216 opened a new pull request #4754:
URL: https://github.com/apache/incubator-nuttx/pull/4754
## Summary
It's better to apply the default compiler option to improve the compatibility
This reverts commit 3fc06ff2d145adc0fc3e46306e3a9886fa436ed8.
## Impact
sim
## Testing
pass CI and ostest
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #4754: Revert "sim: Specify -fshort-wchar as NuttX wchar_t is 16-bit"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4754:
URL: https://github.com/apache/incubator-nuttx/pull/4754#issuecomment-955960938
> if your app is just abusing wchar_t where it should use uint32_t, maybe this PR is enough. otherwise, you likely need a proper implementation of mbrtowc etc. which is the case?
Yes, we plan to implement mbrtowc for UTF8 encoding.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt merged pull request #4754: Revert "sim: Specify -fshort-wchar as NuttX wchar_t is 16-bit"
Posted by GitBox <gi...@apache.org>.
yamt merged pull request #4754:
URL: https://github.com/apache/incubator-nuttx/pull/4754
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #4754: Revert "sim: Specify -fshort-wchar as NuttX wchar_t is 16-bit"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4754:
URL: https://github.com/apache/incubator-nuttx/pull/4754#issuecomment-989776884
Ok, we will take a look tonight.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #4754: Revert "sim: Specify -fshort-wchar as NuttX wchar_t is 16-bit"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4754:
URL: https://github.com/apache/incubator-nuttx/pull/4754#issuecomment-955949696
>
> no standard mandates unicode or how it's implemented as far as i know.
unicode is accepted by many countries as a standard to represent the character worldwide.
> `__STDC_ISO_10646__` is optional. i feel it makes sense only if we are going to implement unicode wchar_t. are we?
But if we want to support wchar_t, it's better to follow __STDC_ISO_10646__.
> anyway, if we are going to revert this, don't forget to update this.
>
> https://github.com/apache/incubator-nuttx/blob/b58379b7380dcef3346bf9ff1ac76d40dc094095/include/sys/types.h#L170
Fix here: https://github.com/apache/incubator-nuttx/pull/4754/commits/339e54b62d81bcc8830218783d9a6d5fab141310
>
> actually i don't insist. nuttx wchar_t has been 16 bit. (i don't know why.) i just haven't seen a good reason to change it, especially when the change increases memory usage.
The size increasement should be small because:
1. The code which use wchar_t is very rare.
2. wchar_t is builtin type in C++ which mean it is already 4 bytes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #4754: Revert "sim: Specify -fshort-wchar as NuttX wchar_t is 16-bit"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #4754:
URL: https://github.com/apache/incubator-nuttx/pull/4754#issuecomment-988834760
@yamt please merge this PR if you don't have more concern. https://github.com/apache/incubator-nuttx/pull/4881 depends on it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #4754: Revert "sim: Specify -fshort-wchar as NuttX wchar_t is 16-bit"
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #4754:
URL: https://github.com/apache/incubator-nuttx/pull/4754#discussion_r765289382
##########
File path: include/sys/types.h
##########
@@ -161,13 +161,13 @@ typedef int16_t key_t;
typedef intptr_t ptrdiff_t;
#if !defined(__cplusplus)
-/* Wide, 16-bit character types. wchar_t is a built-in type in C++ and
+/* Wide character types. wchar_t is a built-in type in C++ and
* its declaration here may cause compilation errors on some compilers.
*
* REVISIT: wchar_t belongs in stddef.h
*/
-typedef uint16_t wchar_t;
+typedef int wchar_t;
Review comment:
maybe it's safer to use`__WCHAR_TYPE__` where available?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #4754: Revert "sim: Specify -fshort-wchar as NuttX wchar_t is 16-bit"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #4754:
URL: https://github.com/apache/incubator-nuttx/pull/4754#issuecomment-955949696
>
> no standard mandates unicode or how it's implemented as far as i know.
unicode is accepted by many countries as a standard to represent the character worldwide. If we want to support language other English, Unicode is the best option.
> `__STDC_ISO_10646__` is optional. i feel it makes sense only if we are going to implement unicode wchar_t. are we?
But if we want to support wchar_t, it's better to follow `__STDC_ISO_10646__`.
> anyway, if we are going to revert this, don't forget to update this.
>
> https://github.com/apache/incubator-nuttx/blob/b58379b7380dcef3346bf9ff1ac76d40dc094095/include/sys/types.h#L170
Fix here: https://github.com/apache/incubator-nuttx/pull/4754/commits/339e54b62d81bcc8830218783d9a6d5fab141310
>
> actually i don't insist. nuttx wchar_t has been 16 bit. (i don't know why.) i just haven't seen a good reason to change it, especially when the change increases memory usage.
The size increasement should be small because:
1. The code which use wchar_t is very rare.
2. wchar_t is builtin type in C++ which mean it is already 4 bytes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on pull request #4754: Revert "sim: Specify -fshort-wchar as NuttX wchar_t is 16-bit"
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #4754:
URL: https://github.com/apache/incubator-nuttx/pull/4754#issuecomment-955894628
> > > > > It's better to apply the default compiler option to improve the compatibility
> > > >
> > > >
> > > > compatibility with what?
> > >
> > >
> > > 3rd party application, sorry I can't disclose the name publicly.
> >
> >
> > what assumption does the app make?
>
> It assume wchar_t is four bytes and then has the enough space to encode UCS4 encoding.
does it assume `__STDC_ISO_10646__`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #4754: Revert "sim: Specify -fshort-wchar as NuttX wchar_t is 16-bit"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4754:
URL: https://github.com/apache/incubator-nuttx/pull/4754#discussion_r765458294
##########
File path: include/sys/types.h
##########
@@ -161,13 +161,13 @@ typedef int16_t key_t;
typedef intptr_t ptrdiff_t;
#if !defined(__cplusplus)
-/* Wide, 16-bit character types. wchar_t is a built-in type in C++ and
+/* Wide character types. wchar_t is a built-in type in C++ and
* its declaration here may cause compilation errors on some compilers.
*
* REVISIT: wchar_t belongs in stddef.h
*/
-typedef uint16_t wchar_t;
+typedef int wchar_t;
Review comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] masayuki2009 edited a comment on pull request #4754: Revert "sim: Specify -fshort-wchar as NuttX wchar_t is 16-bit"
Posted by GitBox <gi...@apache.org>.
masayuki2009 edited a comment on pull request #4754:
URL: https://github.com/apache/incubator-nuttx/pull/4754#issuecomment-989756816
>@xiaoxiang781216
>
>Hmm, the following commit has a problem with uSD card on Spresense.
@xiaoxiang781216
stm32f4discovery with uSD over SPI has the same problem too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #4754: Revert "sim: Specify -fshort-wchar as NuttX wchar_t is 16-bit"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #4754:
URL: https://github.com/apache/incubator-nuttx/pull/4754#issuecomment-955949696
>
> no standard mandates unicode or how it's implemented as far as i know.
unicode is accepted by many countries as a standard to represent the character worldwide. If we want to support language other English, Unicode is the best option.
> `__STDC_ISO_10646__` is optional. i feel it makes sense only if we are going to implement unicode wchar_t. are we?
But if we want to support wchar_t, it's better to follow __STDC_ISO_10646__.
> anyway, if we are going to revert this, don't forget to update this.
>
> https://github.com/apache/incubator-nuttx/blob/b58379b7380dcef3346bf9ff1ac76d40dc094095/include/sys/types.h#L170
Fix here: https://github.com/apache/incubator-nuttx/pull/4754/commits/339e54b62d81bcc8830218783d9a6d5fab141310
>
> actually i don't insist. nuttx wchar_t has been 16 bit. (i don't know why.) i just haven't seen a good reason to change it, especially when the change increases memory usage.
The size increasement should be small because:
1. The code which use wchar_t is very rare.
2. wchar_t is builtin type in C++ which mean it is already 4 bytes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on pull request #4754: Revert "sim: Specify -fshort-wchar as NuttX wchar_t is 16-bit"
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #4754:
URL: https://github.com/apache/incubator-nuttx/pull/4754#issuecomment-955888270
> > > It's better to apply the default compiler option to improve the compatibility
> >
> >
> > compatibility with what?
>
> 3rd party application, sorry I can't disclose the name publicly.
what assumption does the app make?
> The major reason to revert this patch is that it's wrong to make the different size of wchar_t between sim and real device(e.g. arm) as I list in the summary.
how about using -fshort-wchar for arm then?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on pull request #4754: Revert "sim: Specify -fshort-wchar as NuttX wchar_t is 16-bit"
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #4754:
URL: https://github.com/apache/incubator-nuttx/pull/4754#issuecomment-955891488
> From https://pubs.opengroup.org/onlinepubs/007908799/xsh/stddef.h.html:
>
> ```
> wchar_t
> Integral type whose range of values can represent distinct wide-character codes for all members of the largest character set specified among the locales supported by the compilation environment: the null character has the code value 0 and each member of the Portable Character Set has a code value equal to its value when used as the lone character in an integer character constant.
> ```
>
> wchar_t require to save all possible character encoding. And from https://www.gnu.org/software/libc/manual/html_node/Extended-Char-Intro.html:
>
> ```
> Data type: wchar_t
> This data type is used as the base type for wide character strings. In other words, arrays of objects of this type are the equivalent of char[] for multibyte character strings. The type is defined in stddef.h.
>
> The ISO C90 standard, where wchar_t was introduced, does not say anything specific about the representation. It only requires that this type is capable of storing all elements of the basic character set. Therefore it would be legitimate to define wchar_t as char, which might make sense for embedded systems.
>
> But in the GNU C Library wchar_t is always 32 bits wide and, therefore, capable of representing all UCS-4 values and, therefore, covering all of ISO 10646. Some Unix systems define wchar_t as a 16-bit type and thereby follow Unicode very strictly. This definition is perfectly fine with the standard, but it also means that to represent all characters from Unicode and ISO 10646 one has to use UTF-16 surrogate characters, which is in fact a multi-wide-character encoding. But resorting to multi-wide-character encoding contradicts the purpose of the wchar_t type.
> ```
>
> To cover all possible Unicode encoding, wchar_t require at least 4 bytes.
>
> So, from the standard perspective, this patch should be revert too.
no standard mandates unicode or how it's implemented as far as i know.
`__STDC_ISO_10646__` is optional.
i feel it makes sense only if we are going to implement unicode wchar_t. are we?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on pull request #4754: Revert "sim: Specify -fshort-wchar as NuttX wchar_t is 16-bit"
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #4754:
URL: https://github.com/apache/incubator-nuttx/pull/4754#issuecomment-955806500
> It's better to apply the default compiler option to improve the compatibility
compatibility with what?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #4754: Revert "sim: Specify -fshort-wchar as NuttX wchar_t is 16-bit"
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #4754:
URL: https://github.com/apache/incubator-nuttx/pull/4754#discussion_r765289382
##########
File path: include/sys/types.h
##########
@@ -161,13 +161,13 @@ typedef int16_t key_t;
typedef intptr_t ptrdiff_t;
#if !defined(__cplusplus)
-/* Wide, 16-bit character types. wchar_t is a built-in type in C++ and
+/* Wide character types. wchar_t is a built-in type in C++ and
* its declaration here may cause compilation errors on some compilers.
*
* REVISIT: wchar_t belongs in stddef.h
*/
-typedef uint16_t wchar_t;
+typedef int wchar_t;
Review comment:
maybe it's safer to use __WCHAR_TYPE__ where available?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] masayuki2009 commented on pull request #4754: Revert "sim: Specify -fshort-wchar as NuttX wchar_t is 16-bit"
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #4754:
URL: https://github.com/apache/incubator-nuttx/pull/4754#issuecomment-989756816
>@xiaoxiang781216
>
>Hmm, the following commit has a problem with uSD card on Spresense.
stm32f4discovery with uSD over SPI has the same problem too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #4754: Revert "sim: Specify -fshort-wchar as NuttX wchar_t is 16-bit"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4754:
URL: https://github.com/apache/incubator-nuttx/pull/4754#issuecomment-955881546
From https://pubs.opengroup.org/onlinepubs/007908799/xsh/stddef.h.html:
```
wchar_t
Integral type whose range of values can represent distinct wide-character codes for all members of the largest character set specified among the locales supported by the compilation environment: the null character has the code value 0 and each member of the Portable Character Set has a code value equal to its value when used as the lone character in an integer character constant.
```
wchar_t require to save all possible character encoding.
And from https://www.gnu.org/software/libc/manual/html_node/Extended-Char-Intro.html:
```
Data type: wchar_t
This data type is used as the base type for wide character strings. In other words, arrays of objects of this type are the equivalent of char[] for multibyte character strings. The type is defined in stddef.h.
The ISO C90 standard, where wchar_t was introduced, does not say anything specific about the representation. It only requires that this type is capable of storing all elements of the basic character set. Therefore it would be legitimate to define wchar_t as char, which might make sense for embedded systems.
But in the GNU C Library wchar_t is always 32 bits wide and, therefore, capable of representing all UCS-4 values and, therefore, covering all of ISO 10646. Some Unix systems define wchar_t as a 16-bit type and thereby follow Unicode very strictly. This definition is perfectly fine with the standard, but it also means that to represent all characters from Unicode and ISO 10646 one has to use UTF-16 surrogate characters, which is in fact a multi-wide-character encoding. But resorting to multi-wide-character encoding contradicts the purpose of the wchar_t type.
```
To cover all possible Unicode encoding, wchar_t require at least 4 bytes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on pull request #4754: Revert "sim: Specify -fshort-wchar as NuttX wchar_t is 16-bit"
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #4754:
URL: https://github.com/apache/incubator-nuttx/pull/4754#issuecomment-955894086
> > > > > It's better to apply the default compiler option to improve the compatibility
> > > >
> > > >
> > > > compatibility with what?
> > >
> > >
> > > 3rd party application, sorry I can't disclose the name publicly.
> >
> >
> > what assumption does the app make?
>
> It assume wchar_t is four bytes and then has the enough space to encode UCS4 encoding.
>
> > > The major reason to revert this patch is that it's wrong to make the different size of wchar_t between sim and real device(e.g. arm) as I list in the summary.
> >
> >
> > how about using -fshort-wchar for arm then?
>
> short will make it's very hard to process surrogate pair(https://en.wikipedia.org/wiki/UTF-16) as I mention in the last reply. So I don't see there is any benefit to add the seldom used flag(-fshort-wchar), could you explain why you stick with this flag?
actually i don't insist.
nuttx wchar_t has been 16 bit. (i don't know why.)
i just haven't seen a good reason to change it, especially when the change increases memory usage.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on pull request #4754: Revert "sim: Specify -fshort-wchar as NuttX wchar_t is 16-bit"
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #4754:
URL: https://github.com/apache/incubator-nuttx/pull/4754#issuecomment-955953661
if your app is just abusing wchar_t where it should use uint32_t, maybe this PR is enough.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #4754: Revert "sim: Specify -fshort-wchar as NuttX wchar_t is 16-bit"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #4754:
URL: https://github.com/apache/incubator-nuttx/pull/4754#issuecomment-988834760
@yamt please merge this PR if you don't have more concern, https://github.com/apache/incubator-nuttx/pull/4881 depends on it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #4754: Revert "sim: Specify -fshort-wchar as NuttX wchar_t is 16-bit"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4754:
URL: https://github.com/apache/incubator-nuttx/pull/4754#issuecomment-988834760
@yamt please merge this PR, https://github.com/apache/incubator-nuttx/pull/4881 depends on it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #4754: Revert "sim: Specify -fshort-wchar as NuttX wchar_t is 16-bit"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4754:
URL: https://github.com/apache/incubator-nuttx/pull/4754#issuecomment-955892159
> > > > It's better to apply the default compiler option to improve the compatibility
> > >
> > >
> > > compatibility with what?
> >
> >
> > 3rd party application, sorry I can't disclose the name publicly.
>
> what assumption does the app make?
It assume wchar_t is four bytes and then has the enough space to encode UCS4 encoding.
>
> > The major reason to revert this patch is that it's wrong to make the different size of wchar_t between sim and real device(e.g. arm) as I list in the summary.
>
> how about using -fshort-wchar for arm then?
short will make it's very hard to process surrogate pair(https://en.wikipedia.org/wiki/UTF-16) as I mention in the last reply. So I don't see there is any benefit to add the seldom used flag(-fshort-wchar), could you explain why you stick with this flag?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt edited a comment on pull request #4754: Revert "sim: Specify -fshort-wchar as NuttX wchar_t is 16-bit"
Posted by GitBox <gi...@apache.org>.
yamt edited a comment on pull request #4754:
URL: https://github.com/apache/incubator-nuttx/pull/4754#issuecomment-955953661
if your app is just abusing wchar_t where it should use uint32_t, maybe this PR is enough.
otherwise, you likely need a proper implementation of mbrtowc etc.
which is the case?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] masayuki2009 commented on pull request #4754: Revert "sim: Specify -fshort-wchar as NuttX wchar_t is 16-bit"
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #4754:
URL: https://github.com/apache/incubator-nuttx/pull/4754#issuecomment-989751797
@xiaoxiang781216
Hmm, the following commit has a problem with uSD card on Spresense.
If I revert the commit locally, it works fine.
```
nsh> df
Block Number
Size Blocks Used Available Mounted on
16384 971908 5718 966190 /mnt/sd0
4096 1024 8 1016 /mnt/spif
0 0 0 0 /proc
nsh> ls -l /mnt/sd0
/mnt/sd0:
nsh: ls: stat failed: 22
```
commit 635752389226ad005f52524a933acf864c1a1f8d
Author: Xiang Xiao <xi...@xiaomi.com>
Date: Mon Nov 1 12:40:51 2021 +0800
arch: Add _wchar_t typedef like other basic types
Signed-off-by: Xiang Xiao <xi...@xiaomi.com>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #4754: Revert "sim: Specify -fshort-wchar as NuttX wchar_t is 16-bit"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #4754:
URL: https://github.com/apache/incubator-nuttx/pull/4754#issuecomment-955881546
From https://pubs.opengroup.org/onlinepubs/007908799/xsh/stddef.h.html:
```
wchar_t
Integral type whose range of values can represent distinct wide-character codes for all members of the largest character set specified among the locales supported by the compilation environment: the null character has the code value 0 and each member of the Portable Character Set has a code value equal to its value when used as the lone character in an integer character constant.
```
wchar_t require to save all possible character encoding.
And from https://www.gnu.org/software/libc/manual/html_node/Extended-Char-Intro.html:
```
Data type: wchar_t
This data type is used as the base type for wide character strings. In other words, arrays of objects of this type are the equivalent of char[] for multibyte character strings. The type is defined in stddef.h.
The ISO C90 standard, where wchar_t was introduced, does not say anything specific about the representation. It only requires that this type is capable of storing all elements of the basic character set. Therefore it would be legitimate to define wchar_t as char, which might make sense for embedded systems.
But in the GNU C Library wchar_t is always 32 bits wide and, therefore, capable of representing all UCS-4 values and, therefore, covering all of ISO 10646. Some Unix systems define wchar_t as a 16-bit type and thereby follow Unicode very strictly. This definition is perfectly fine with the standard, but it also means that to represent all characters from Unicode and ISO 10646 one has to use UTF-16 surrogate characters, which is in fact a multi-wide-character encoding. But resorting to multi-wide-character encoding contradicts the purpose of the wchar_t type.
```
To cover all possible Unicode encoding, wchar_t require at least 4 bytes.
So, from the standard perspective, this patch should be revert too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #4754: Revert "sim: Specify -fshort-wchar as NuttX wchar_t is 16-bit"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4754:
URL: https://github.com/apache/incubator-nuttx/pull/4754#issuecomment-955878261
> > It's better to apply the default compiler option to improve the compatibility
>
> compatibility with what?
3rd party application, sorry I can't disclose the name publicly.
The major reason to revert this patch is that it's wrong to make the different size of wchar_t between sim and real device(e.g. arm).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #4754: Revert "sim: Specify -fshort-wchar as NuttX wchar_t is 16-bit"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #4754:
URL: https://github.com/apache/incubator-nuttx/pull/4754#issuecomment-955878261
> > It's better to apply the default compiler option to improve the compatibility
>
> compatibility with what?
3rd party application, sorry I can't disclose the name publicly.
The major reason to revert this patch is that it's wrong to make the different size of wchar_t between sim and real device(e.g. arm) as I list in the summary.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on pull request #4754: Revert "sim: Specify -fshort-wchar as NuttX wchar_t is 16-bit"
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #4754:
URL: https://github.com/apache/incubator-nuttx/pull/4754#issuecomment-955892077
anyway, if we are going to revert this, don't forget to update this. https://github.com/apache/incubator-nuttx/blob/b58379b7380dcef3346bf9ff1ac76d40dc094095/include/sys/types.h#L170
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org