Skip to content

Commit

Permalink
Update presubmit/styleguide language around wstring and UTF-16.
Browse files Browse the repository at this point in the history
wstring is no longer banned in new code, so remove the PRESUBMIT for
that.  Indeed, we do (very rarely) use wstring in cross-platform code
when it only actually matters on Windows.  This should not result in
people wildly misusing wstring, as it is no longer the same type as
string16(/u16string) and thus cannot be silently passed to any APIs.
The only time it's used anymore is generally when it needs to be used,
so this presubmit is just annoying.

Remove outdated language in styleguide around wchar_t and
char16(/char16_t) being the same type.  Instead, provide an explicit
override of the Google styleguide and guidance on how to use UTF-16.

Bug: 911896
Change-Id: Idfce5f43f48ed4a2614c3da0ec793adf91586ff7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2776291
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#864953}
  • Loading branch information
pkasting authored and Chromium LUCI CQ committed Mar 20, 2021
1 parent 822000d commit 9e210a5
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 33 deletions.
29 changes: 0 additions & 29 deletions PRESUBMIT.py
Original file line number Diff line number Diff line change
Expand Up @@ -1699,35 +1699,6 @@ def CheckFlakyTestUsage(input_api, output_api):
return []


def CheckNoNewWStrings(input_api, output_api):
"""Checks to make sure we don't introduce use of wstrings."""
problems = []
for f in input_api.AffectedFiles():
if (not f.LocalPath().endswith(('.cc', '.h')) or
f.LocalPath().endswith(('test.cc', '_win.cc', '_win.h')) or
'/win/' in f.LocalPath() or
'chrome_elf' in f.LocalPath() or
'install_static' in f.LocalPath()):
continue

allowWString = False
for line_num, line in f.ChangedContents():
if 'presubmit: allow wstring' in line:
allowWString = True
elif not allowWString and 'wstring' in line:
problems.append(' %s:%d' % (f.LocalPath(), line_num))
allowWString = False
else:
allowWString = False

if not problems:
return []
return [output_api.PresubmitPromptWarning('New code should not use wstrings.'
' If you are calling a cross-platform API that accepts a wstring, '
'fix the API.\n' +
'\n'.join(problems))]


def CheckNoDEPSGIT(input_api, output_api):
"""Make sure .DEPS.git is never modified manually."""
if any(f.LocalPath().endswith('.DEPS.git') for f in
Expand Down
14 changes: 10 additions & 4 deletions styleguide/c++/c++.md
Original file line number Diff line number Diff line change
Expand Up @@ -162,10 +162,16 @@ Place platform-specific #includes in their own section below the "normal"
not have been compiled with the same sizes for things like `int` and
`size_t`. However, to the greatest degree possible, avoid letting these
sized types bleed through the APIs of the layers in question.
* Don't use `std::wstring`. Use `std::u16string` or `base::FilePath` instead.
(Windows-specific code interfacing with system APIs using `wstring` and
`wchar_t` can still use `string16` and `char16`; it is safe to assume that
these are equivalent to the "wide" types.)
* The Google Style Guide [bans
UTF-16](https://google.github.io/styleguide/cppguide.html#Non-ASCII_Characters).
For various reasons, Chromium uses UTF-16 extensively. Use `std::u16string`
and `char16_t*` for 16-bit strings, `u"..."` to declare UTF-16 literals, and
either the actual characters or the `\uXXXX` or `\UXXXXXXXX` escapes for
Unicode characters. Avoid `\xXX...`-style escapes, which can cause subtle
problems if someone attempts to change the type of string that holds the
literal. In code used only on Windows, it may be necessary to use
`std::wstring` and `wchar_t*`; these are legal, but note that they are
distinct types and are often not 16-bit on other platforms.

## Object ownership and calling conventions

Expand Down

0 comments on commit 9e210a5

Please sign in to comment.