This is an archive of the discontinued LLVM Phabricator instance.

[test-suite] Fix Unit Tests for Code Coverage
ClosedPublic

Authored by lenary on Mar 1 2021, 7:03 AM.

Details

Summary

https://reviews.llvm.org/D88931 recently landed to add more tests to the
test suite to improve coverage of parts of clang. These tests caused
errors on some platforms, which was caught in post-commit review.

This change replaces the use of "%i" with the equivalent specifiers for
those types from inttypes.h, and updates the reference output to
correctly account for formatting unsigned arguments using unsigned
format specifiers.

Diff Detail

Repository
rT test-suite

Event Timeline

lenary requested review of this revision.Mar 1 2021, 7:03 AM
lenary created this revision.
yroux added a comment.Mar 1 2021, 11:05 PM

Hi Sam,

the overall patch looks good to me, I just spot that these macros are explicitly defined into SingleSource/Regression/C/casts.c if PRId64 doesn't exist.
So I wonder if this guarf is needed on some plateform

lenary added a comment.Mar 2 2021, 2:15 AM

Hi Sam,

the overall patch looks good to me, I just spot that these macros are explicitly defined into SingleSource/Regression/C/casts.c if PRId64 doesn't exist.
So I wonder if this guarf is needed on some plateform

I don't think we should be providing definitions if they aren't provided already, as we don't know if the platform is ILP32/LP64/LLP64 -- which was the original point in this patch.

As it is right now, this will fail at compile-time if you don't have these macros, which I think is ok. It should be up to the platforms without these inttypes.h definitions to exclude these tests with a downstream cmake patch if they don't support these macros (or indeed the types that go with them).

yroux accepted this revision.Mar 2 2021, 11:55 PM

It makes sense, thanks.

This revision is now accepted and ready to land.Mar 2 2021, 11:55 PM