In D100631#2695941, @sivachandra wrote:

Note that Fuchsia does not block us from using this feature in LLVM libc source tree as only ctype tests from LLVM libc run in Fuchsia.

In D100631#2695607, @phosek wrote:

In D100631#2695553, @phosek wrote:

I'll check with zxtest maintainers.

Looks like we already have a few use cases for TYPED_TEST in Fuchsia, this is tracked as fxbug.dev/62890.

• Added documentation

• Recover ParamType from PRETTY_FUNCTION instead of RTTI or user provided type name.

In D100631#2695126, @sivachandra wrote:

While I am not opposed to this, I want to learn the usefulness of this feature. IIUC, this limits to a single ParamType - as in a uni-dimensional type variance. We have examples in math functions where there is two-dimensional type variance. I am not sure the feature added in this patch can help with it. For such cases, have used template test classes and helper macros to list the various combination of tests: https://github.com/llvm/llvm-project/blob/main/libc/test/src/math/RoundToIntegerTest.h.

Thx for the suggestion, I didn't go with explicit member function as it would allow for integer promotion.
In the case of endianness we want to make sure we only accept one of uint8_t, uint16_t, uint32_t, uint64_t and no other type.
I've added the symmetric functions as you suggested.

• Simplify tests

• Add tests and remove macros

• Fixed base revision

• Fix typo and static assert message.

In D100569#2693901, @sivachandra wrote:

In D100569#2691816, @gchatelet wrote:

Should we start to add tests?

Sorry I missed this. We can add tests now in a different pass.

• Fix argument capitalization

• Added missing modification to CMakeLists.txt

Let me know if you want this header to live somewhere else, or if you had other plans in mind.

I removed the various asserts from the original code as we did for StringView.
Should we start to add tests?

Thank you for the clean up. Really appreciate it.
Before submitting the patch did you check that there are no users in experimental targets? (ARC, VE)

LGTM as well

In D97223#2581587, @jyknight wrote:

In D97223#2581126, @rjmccall wrote:

Looks like you didn't update the .bc reader/writer and the .ll printer/parser.

The instructions already had alignment support added to the type constructor and IR/BC in prior changes by gchatelet -- this is just updating the IRBuilder constructor.

In D83465#2557909, @gchatelet wrote:

Thx @jyknight ! Let me know if you prefer me to push it ( and D87443 ).
If you do push them please mention the two reviews (D87443 and D83465) in the description so we can track everything and abandon these changes.

Thx @jyknight ! Let me know if you prefer me to push it ( and D87443 ).
If you do push them please mention the two reviews (D87443 and D83465) in the description so we can track everything and abandon these changes.

In D82876#2545580, @RKSimon wrote:

@gchatelet Abandon after D96097 ?

I had a similar patch - D82876 - but it never went through...
Thx for working on this!

LGTM thx @qiucf

Hey Andre, my apologies for the lag. Answers inlined

Thx!

• buffer values are never modified so we can use char instead of unsigned char.

In D94770#2501005, @avieira wrote:

Eh you beat me to it. Had a similar patch I've been measuring and testing. Your's is more complete though :)

LGTM.

• Address comments

LGTM, can you please wait for @sivachandra answer about using assert in code before submitting? Thx!

My apologies for the late reply. I've made a bunch of comments.

In D93457#2481610, @avieira wrote:

Sure, LGTM. What is the usual convention for Libc reviewing? Should I 'accept revision' if I agree with it even though I'm not a maintainer?

In D93457#2481596, @avieira wrote:

Hi Guillaume,

Sorry for the late reply, I was looking at some other things and then holidays happened. I started looking at making some changes to CopyAlignedBlocks just like we discussed earlier. One of them is this one and I noticed this makes the MemcpyUtilsTest.CopyAlignedBlocks fail in libc_string_unittests. Specifically the ones that test destination misalignment. It seems the testing infrastructure keeps track of what bytes were read and written. I checked and making the source the misaligned input and switching the read and written expected traces makes it passs. Which makes sense I guess.

Would be good to make the change at the same time though.

• Address comments and rebase

In D93210#2460597, @sivachandra wrote:

In D93210#2460196, @gchatelet wrote:

@sivachandra I'll submit this as is. Let me know if you have any concerns and we can amend it in a follow up patch.

Sorry, I am no expert here so I just kept silent while @courbet was steering the review. LGTM.

@sivachandra I'll submit this as is. Let me know if you have any concerns and we can amend it in a follow up patch.

rebasing

• Address comments

• Address comments and fix naming style

In D82134#2451435, @tschuett wrote:

The granularity of ARMv8.3 PAC is 16 byte. Can you read an invalid address?