This is an archive of the discontinued LLVM Phabricator instance.

[SystemZ][z/OS] Missing locale functions libc++
ClosedPublic

Authored by muiez on Mar 5 2021, 6:27 AM.

Details

Reviewers
ldionne
zibi
curdeius
Group Reviewers
Restricted Project
Commits
rGebe6161c54b9: [SystemZ][z/OS] Missing locale functions libc++
Summary

The aim is to add the missing z/OS specific locale functions for libc++ (newlocale, freelocale and uselocale).

Diff Detail

Event Timeline

muiez created this revision.Mar 5 2021, 6:27 AM
muiez requested review of this revision.Mar 5 2021, 6:27 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 5 2021, 6:27 AM
Herald added 1 blocking reviewer(s): Restricted Project. · View Herald Transcript
ldionne accepted this revision.Mar 5 2021, 6:31 AM

I am fine with this change in the sense of how it integrates with the rest of libc++. I did not review the implementation itself - I trust you folks that it works, otherwise it'll break on your own platform :-).

I strongly recommend addressing my small comment about the formatting in uselocale.

libcxx/src/support/ibm/xlocale_zos.cpp
124–127

This is a really really weird way to save a few lines of code. I would never do that, it looks too much like an unintended mistake.

This revision is now accepted and ready to land.Mar 5 2021, 6:31 AM

Also, please wait for CI to finish before you ship this.

muiez updated this revision to Diff 328510.Mar 5 2021, 6:41 AM

Coding style addressed

Some nitty-picky comments.

libcxx/src/support/ibm/xlocale_zos.cpp
18

Nit (here and elsewhere): comments are supposed to be full phrases with full stops (dots) at the end.

77

CamelCase? If think that snake_case should be used.

98

previously current -> previous?

101

Typo: separated.

muiez updated this revision to Diff 328526.Mar 5 2021, 7:34 AM
muiez marked an inline comment as done.

addressed comments

curdeius accepted this revision.Mar 5 2021, 7:36 AM

LGTM.

libcxx/include/__support/ibm/locale_mgmt_zos.h
45

Last missing dot (no need to update the review, just add it when landing).

muiez updated this revision to Diff 328530.Mar 5 2021, 7:44 AM

added last dot in comment

This revision was automatically updated to reflect the committed changes.