New issue
Advanced search Search tips

Issue 752856 link

Starred by 1 user
Status: Fixed
Owner: ----
Closed: Aug 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Koenig lookup of base::string16 causes undesirable compile success of UTF16ToWide without namespace qualifier

Project Member Reported by ricea@chromium.org, Aug 7 2017 
To reproduce:

1. Write UTF16ToUTF8(s) without the base:: qualifier

Expected result:

Fails to compile on all platforms.

Actual result:

Compiles on Linux and Mac. Fails to compile on Windows.

Cause:

On Linux and Mac, base::string16 is a typedef for std::basic_string<char16, base::string16_char_traits>. Koenig lookup find the base::string16_char_traits type and adds base:: to the associated set of namespaces.

Although the one I found is UTF16ToUTF8, the same issue will apply to any function in base:: which takes a base::string16 argument.

Comment 1 by ricea@chromium.org, Aug 7 2017

Summary: Koenig lookup of base::string16 causes undesirable compile success of UTF16ToWide without namespace qualifier (was: Koenig lookup of base::string16 causes undesirable compile success of UTF16ToUTF8 without namespace qualifier)
My example is weird. UTF16ToUTF8 takes a base::StringPiece16 argument, so it would normally be natural the Koenig lookup would work. However, I was calling it with the return value of content::TitleWatcher::WaitAndGetTitle() which is a const base::string16&.

UTF16ToWide() is a better example.

Comment 2 by ricea@chromium.org, Aug 7 2017

Status: Started (was: Untriaged)
I have a proposed fix at https://chromium-review.googlesource.com/c/603888
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 14 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b

commit ff26dcf0afabc83a13a3121612b2f1dfdca97f2b
Author: Adam Rice <ricea@chromium.org>
Date: Mon Aug 14 05:01:52 2017

Prevent Koenig-lookup of base::string16 arguments

Previously functions in base that took a base::string16 argument could
be used unqualified on platforms where std::wstring is 32-bit. This was
surprising and inconsistent.

Move base::string16_char_traits into namespace base::string16_internals
to prevent argument-dependent lookup from finding functions in
base::. This doesn't break existing code because it was already
necessary to specify the base:: namespace for it to compile on Windows.

Also remove the base::string16_char_traits typedef on platforms where
base::string16 is std::wstring, for consistency.

TBR=sky@chromium.org,donnd@chromium.org,mtomasz@chromium.org,rsesek@chromium.org,bauerb@chromium.org,calamity@chromium.org,dvadym@chromium.org,dpranke@chromium.org,rjkroege@chromium.org,tapted@chromium.org
BUG= 752856 

Change-Id: I76ff1691763f552c3140329c2701b0bf61e8c911
Reviewed-on: https://chromium-review.googlesource.com/603888
Commit-Queue: Adam Rice <ricea@chromium.org>
Reviewed-by: Adam Rice <ricea@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494014}
[modify] https://crrev.com/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b/ash/login/ui/login_auth_user_view.cc
[modify] https://crrev.com/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b/ash/system/supervised/tray_supervised_user_unittest.cc
[modify] https://crrev.com/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b/base/BUILD.gn
[modify] https://crrev.com/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b/base/strings/string16.cc
[modify] https://crrev.com/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b/base/strings/string16.h
[add] https://crrev.com/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b/base/strings/string16_unittest.nc
[modify] https://crrev.com/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b/chrome/browser/android/contextualsearch/contextual_search_delegate.cc
[modify] https://crrev.com/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b/chrome/browser/app_controller_mac_browsertest.mm
[modify] https://crrev.com/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b/chrome/browser/chromeos/extensions/file_manager/private_api_misc.cc
[modify] https://crrev.com/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b/chrome/browser/notifications/notification_platform_bridge_mac.mm
[modify] https://crrev.com/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b/chrome/browser/supervised_user/supervised_user_service.cc
[modify] https://crrev.com/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b/chrome/browser/ui/app_list/search/arc/arc_playstore_search_provider.cc
[modify] https://crrev.com/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b/chrome/browser/ui/cocoa/app_menu/app_menu_controller.mm
[modify] https://crrev.com/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b/chrome/browser/ui/cocoa/autofill/autofill_popup_view_cocoa.mm
[modify] https://crrev.com/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b/chrome/browser/ui/cocoa/autofill/card_unmask_prompt_view_bridge.mm
[modify] https://crrev.com/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b/chrome/browser/ui/cocoa/autofill/save_card_bubble_view_bridge.mm
[modify] https://crrev.com/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_bridge.mm
[modify] https://crrev.com/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b/chrome/browser/ui/cocoa/bubble_sync_promo_controller.mm
[modify] https://crrev.com/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b/chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.mm
[modify] https://crrev.com/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b/chrome/browser/ui/cocoa/extensions/extension_install_view_controller.mm
[modify] https://crrev.com/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b/chrome/browser/ui/cocoa/global_error_bubble_controller.mm
[modify] https://crrev.com/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b/chrome/browser/ui/cocoa/page_info/page_info_bubble_controller.mm
[modify] https://crrev.com/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b/chrome/browser/ui/cocoa/profiles/profile_signin_confirmation_view_controller.mm
[modify] https://crrev.com/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b/components/password_manager/core/browser/login_database_ios.cc
[modify] https://crrev.com/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b/components/password_manager/core/browser/login_database_ios_unittest.cc
[modify] https://crrev.com/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b/ios/chrome/browser/ui/autofill/card_unmask_prompt_view_bridge.mm
[modify] https://crrev.com/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b/ios/chrome/browser/ui/contextual_search/contextual_search_delegate.cc
[modify] https://crrev.com/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b/ios/chrome/browser/ui/ntp/recent_tabs/views/session_section_header_view.mm
[modify] https://crrev.com/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b/ios/chrome/browser/ui/ntp/recent_tabs/views/session_tab_data_view.mm
[modify] https://crrev.com/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b/ios/chrome/browser/ui/omnibox/omnibox_view_ios.mm
[modify] https://crrev.com/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b/ios/chrome/browser/ui/payments/payment_items_display_mediator.mm
[modify] https://crrev.com/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b/ios/chrome/browser/ui/payments/payment_request_mediator.mm
[modify] https://crrev.com/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b/ios/chrome/browser/ui/payments/shipping_option_selection_mediator.mm
[modify] https://crrev.com/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b/ios/chrome/browser/ui/tab_switcher/tab_switcher_panel_controller.mm
[modify] https://crrev.com/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b/tools/unused-symbols-report.py
[modify] https://crrev.com/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b/ui/ozone/platform/wayland/wayland_window.cc
[modify] https://crrev.com/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b/ui/views/cocoa/bridged_native_widget_unittest.mm
[modify] https://crrev.com/ff26dcf0afabc83a13a3121612b2f1dfdca97f2b/ui/views/widget/native_widget_mac.mm

Comment 4 by ricea@chromium.org, Aug 14 2017

Status: Fixed (was: Started)
►

Sign in to add a comment