This is an archive of the discontinued LLVM Phabricator instance.

Fix the crash when formatting unsupported encodings
ClosedPublic

Authored by owenpan on May 4 2019, 10:41 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

owenpan created this revision.May 4 2019, 10:41 PM
owenpan updated this revision to Diff 198205.May 5 2019, 3:34 PM

Moved "UTF-32 (LE)" to before "UTF-16 (LE)" in llvm::StringSwitch so that the former BOM wouldn't be misnamed as the latter.

sammccall accepted this revision.May 6 2019, 1:04 AM
sammccall added inline comments.
clang/tools/clang-format/ClangFormat.cpp
273 ↗(On Diff #198205)

Seems unlikely we'll ever see any of these other than UTF{16,32}.
I'd suggest dropping them, but up to you.

276 ↗(On Diff #198205)

SCSU?

This revision is now accepted and ready to land.May 6 2019, 1:04 AM

You might want to add a test for one of these?

I copied the code from clang/lib/Basic/SourceManager.cpp. See D61628.

I will update this patch to correct the typo in SCSU or remove it along with the other rare BOMs. How do you add test cases for this kind of fixes that emit error messages?

owenpan updated this revision to Diff 198652.May 8 2019, 7:01 AM

Fixed the typo for SCSU.

owenpan marked an inline comment as done.May 8 2019, 7:01 AM
owenpan marked an inline comment as done.May 8 2019, 7:05 AM
owenpan added inline comments.
clang/tools/clang-format/ClangFormat.cpp
273 ↗(On Diff #198205)

I will keep the rare BOM cases to keep it in sync with D61628.

owenpan marked an inline comment as done.May 8 2019, 7:06 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMay 8 2019, 7:11 AM