Page MenuHomePhabricator

[NFC][compiler-rt] Cleanup Implicit Conversion Sanitizer tests to use sized types
ClosedPublic

Authored by lebedev.ri on Oct 30 2018, 8:03 AM.

Diff Detail

Event Timeline

lebedev.ri created this revision.Oct 30 2018, 8:03 AM

Looks good, thank you. Ideally, we'd have different names on those functions, but unless you have a script that generates this and is very easy to change, I'm ok with keeping them as they are.

LGTM

Thanks for working on this!
Filipe

Looks good, thank you.

Ideally, we'd have different names on those functions, but unless you have a script that generates this and is very easy to change,

It's super easy to change using mass search-and-replace, but i actually intentionally kept the *function names* as-is,
to be in sync with the clang codegen tests.

I'm ok with keeping them as they are.

LGTM

Feel like actually marking these two reviews as accepted? :)

Thanks for working on this!

Filipe
vitalybuka added inline comments.
test/fuzzer/ImplicitIntegerSignChangeTest.cpp
13

Could you please clang format it with something like
git clang-format -f --style=file --extensions=inc,cc,h,c,cpp HEAD^

Some formatting nitpicking.

test/fuzzer/ImplicitIntegerSignChangeTest.cpp
13

That completely messes up all the check-lines here.

vitalybuka added inline comments.Oct 30 2018, 11:37 AM
test/fuzzer/ImplicitIntegerSignChangeTest.cpp
13

you can disable it in such tests with
clang-format off
...
clang-format on

13
// clang-format off
// clang-format on
filcab accepted this revision.Oct 30 2018, 11:59 AM

Feel like actually marking these two reviews as accepted? :)

Sure! LGTMing now.
But please make sure Vitaly is happy with the clang-format stuff.

This revision is now accepted and ready to land.Oct 30 2018, 11:59 AM

Feel like actually marking these two reviews as accepted? :)

Sure! LGTMing now.

Thank you for the review!

But please make sure Vitaly is happy with the clang-format stuff.

Sure.
I did end up running clang-format, and other than messing up the check-lines,
it didn't do any other (other than those i just applied in the last update here) changes.

This revision was automatically updated to reflect the committed changes.