This is an archive of the discontinued LLVM Phabricator instance.

Use the ConvertUTF.cpp from breakpad which is under a compatible license And reintegrate our custom changes
Needs ReviewPublic

Authored by sylvestre.ledru on Jul 15 2019, 6:50 AM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Summary

It breaks the following tests

LLVM-Unit :: Support/./SupportTests/ConvertUTFTest.ConvertUTF8toWide
LLVM-Unit :: Support/./SupportTests/ConvertUTFTest.UTF8ToUTF32Lenient
LLVM-Unit :: Support/./SupportTests/ConvertUTFTest.UTF8ToUTF32PartialLenient
LLVM-Unit :: Support/./SupportTests/ConvertUTFTest.convertWideToUTF8
LLVM-Unit :: Support/./SupportTests/JSONTest.UTF8

but only these (making me wonder if these tests are interesting or not)

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptJul 15 2019, 6:50 AM
mgorny added a subscriber: mgorny.Aug 18 2019, 2:40 AM

Any chance to move this forward? I've just noticed we're missing this one license, and fixing it would render LLVM non-free in Gentoo (read: cause a massive shitstorm for some of our users). I just wanted to do a similar port and found this while looking for earlier work.

unittests/Support/ConvertUTFTest.cpp
166

Why is this commented out? It looks at least weird, with no comment to explain it.

Actually, I think the license change can be submitted separately, so I'll do just that.

sylvestre.ledru marked an inline comment as done.Aug 18 2019, 4:58 AM
sylvestre.ledru added inline comments.
unittests/Support/ConvertUTFTest.cpp
166

I think i removed it to build.
It should indeed be fixed or commented

Do you want me to try to take a look at the problems?

@sylvestre.ledru, I just went through breakpad code, comparing it with the initial code imported to clang. After reordering and aligning whitespace, the code is identical. This means that 1) the tests are failing because of regressions reintroduced by porting breakpad code, and 2) we can just change the license header because all differences in code are of our own making. Therefore, I propose that we go forward with D66390 instead of putting more time into regressing code just to fix it again.