Page MenuHomePhabricator

[ADT] Replace std::isprint by llvm::isPrint.

Authored by Meinersbur on Jul 23 2018, 10:27 AM.



The standard library functions isprint/std::isprint have platform- and locale-dependent behavior which makes LLVM's output less predictable. In particular, regression tests my fail depending on the implementation of these functions.

Implement llvm::isPrint in StringExtras.h with a standard behavior and replace all uses of isprint/std::isprint by a call it llvm::isPrint. The function is inlined and does not look up language settings so it should perform better than the standard library's version.

Such a replacement has already been done for isdigit, isalpha, isxdigit in r314883. gtest does the same in using the following justification:

// Returns true if c is a printable ASCII character.  We test the
// value of c directly instead of calling isprint(), which is buggy on
// Windows Mobile.
inline bool IsPrintableAscii(wchar_t c) {
  return 0x20 <= c && c <= 0x7E;

Similar issues have also been encountered by Julia:

I noticed the problem myself when on Windows isprint('\t') started to evaluate to true (see and thus caused several unit tests to fail. The result of isprint doesn't seem to be well-defined even for ASCII characters. Therefore I suggest to replace isprint by a system-independent version.

Diff Detail


Event Timeline

Meinersbur created this revision.Jul 23 2018, 10:27 AM
This revision is now accepted and ready to land.Jul 23 2018, 12:40 PM

Rebase to r338031 before commit

This revision was automatically updated to reflect the committed changes.