Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch to C11, and use char16_t for UTF-16 strings #1010

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

pcc
Copy link

@pcc pcc commented Feb 7, 2023

The use of wchar_t with -fshort-wchar to get UTF-16 strings is non-standard and can lead to porting issues. So let's switch to char16_t, which also requires switching the project to build with C11.

This all builds in my WIP fork of DAPLink that targets upstream LLVM/Clang/LLD. I haven't been able to try this in any of the officially supported compilers yet, I'm mostly just guessing at the required build file updates.

Signed-off-by: Peter Collingbourne <[email protected]>
@mbrossard
Copy link
Contributor

I have been considering adding support for LLVM Embedded Toolchain for Arm, but this will require some changes to project-generator.

@pcc
Copy link
Author

pcc commented Feb 7, 2023

Interesting, I wasn't aware of that project. I was just building LLVM directly and using musl as the libc, which justifiably rejected my patch to add -fshort-wchar support in favor of having projects use char16_t instead. Given DAPLink's modest libc requirements (see below) it may be simpler to copy the required parts of musl or another libc into DAPLink rather than depending on an external project.

But either way this seems like a good change to make in order to avoid depending on a build flag which the musl developers correctly describe as being fundamentally incompatible with libc. What do you think?

    b08c     b08c       56     4         ../../../external/musl/lib/libc.a(memcmp.o):(.text.memcmp)
    b0e4     b0e4      2d2     4         ../../../external/musl/lib/libc.a(memmove.o):(.text.memmove)
    b3b8     b3b8       ee     4         ../../../external/musl/lib/libc.a(memset.o):(.text.memset)
    b4a6     b4a6       16     2         ../../../external/musl/lib/libc.a(strcat.o):(.text.strcat)
    b4bc     b4bc        c     2         ../../../external/musl/lib/libc.a(strcpy.o):(.text.strcpy)
    b4c8     b4c8      126     4         ../../../external/musl/lib/libc.a(stpcpy.o):(.text.__stpcpy)
    b5f0     b5f0       c2     4         ../../../external/musl/lib/libc.a(strlen.o):(.text.strlen)
    b6b4     b6b4      104     4         ../../../external/musl/lib/libc.a(strncmp.o):(.text.strncmp)

This is no longer necessary now that we no longer use wchar_t.

Signed-off-by: Peter Collingbourne <[email protected]>
@mathias-arm
Copy link
Collaborator

This change does not work with armcc which does not have a --c11 flag. The most salient point is that it does not support u"xxxx" strings.

@pcc
Copy link
Author

pcc commented Feb 14, 2023

armcc is ARM Compiler 5, right? I noticed that the mbed OS project has dropped support for it: https://os.mbed.com/blog/entry/Removal-of-ARM-Compiler-5-and-uARM-toolc/

Have you considered following suit in DAPLink?

@mathias-arm
Copy link
Collaborator

Yes we have decided to also move from armcc to armclang. That was mentioned in #819. I see probably two hurdles that need to be cleared before we can remove armcc support:

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.

3 participants