New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 666723 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

Dependency on //base/test:test_support disable -Wunused-function

Project Member Reported by sdefresne@chromium.org, Nov 18 2016

Issue description

I discovered today that if your target has a dependency on //base/test:test_support, then the flag -Wno-unused-function is added to the command-line and thus disabling errors about unused functions (at least on iOS).

Some investigation showed that this is caused by the following:

$ gn desc out/default //base/test:test_support configs
//build/config:feature_flags
//build/config/compiler:afdo
//build/config/compiler:compiler
//build/config/compiler:clang_stackrealign
//build/config/compiler:compiler_arm_fpu
//build/config/compiler:chromium_code
//build/config/compiler:default_include_dirs
//build/config/compiler:default_optimization
//build/config/compiler:default_stack_frames
//build/config/compiler:default_symbols
//build/config/compiler:no_rtti
//build/config/compiler:runtime_library
//build/config/sanitizers:default_sanitizer_flags
//build/config/gcc:no_exceptions
//build/config/gcc:symbol_visibility_hidden
//build/config/clang:find_bad_constructs
//build/config/clang:extra_warnings
//build/config:debug
//build/config:precompiled_headers
//testing/gtest:gtest_config
//third_party/ced:ced_config
//third_party/icu:icu_config
//testing/gmock:gmock_config
//testing/gtest:gtest_direct_config
//third_party/libxml:libxml_config

$ for config in $(gn desc out/default //base/test:test_support configs); do
    if gn desc out/default $config|grep -q Wno-unused-function; then
      echo $config;
    fi;
  done
//third_party/ced:ced_config

$ gn desc out/default //third_party/ced:ced_config
Config: //third_party/ced:ced_config
Toolchain: //build/toolchain/mac:ios_clang_x64

cflags
  -Wno-unused-function

include_dirs
  //third_party/ced/src/

I think this config should be split in two, with the cflags limited to the //third_party/cec and the include_dirs exported as public_configs. This may requires fixing many test targets that currently get this dependency, so created a bug to track this.
 
CL to fix ced_config is visible here: https://codereview.chromium.org/2512783005/

It has only been tested on iOS and will probably have to be updated to work correctly on other platform if dead function have crept in since the config was introduced.
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 21 2016

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/ios_internal.git/+/cc8c58f1c46c1e1106cee2f2bd334a0b987524d5

commit cc8c58f1c46c1e1106cee2f2bd334a0b987524d5
Author: sdefresne <sdefresne@google.com>
Date: Mon Nov 21 07:33:50 2016

Comment 3 by thakis@chromium.org, Nov 21 2016

Lame :-( We had a similar problem where -Wdeprecated-problems got accidentally disabled for like half the mac build (https://bugs.chromium.org/p/chromium/issues/detail?id=622481#c10, see next comment for fix).

I thought there was some discussion on gn-dev about how to detect if -Wno-foo warnings end up in public_configs, but I can't find that thread.

Thanks for finding this!
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 23 2016

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

commit bfd9c73b16d0bdb92ab04b1d5606fe82fe5fdac2
Author: sdefresne <sdefresne@chromium.org>
Date: Wed Nov 23 16:34:15 2016

Fix //third_party/cec to not disable -Wunused-function.

Move the -Wno-unused-function to a private config that is not exported
via public_configs to avoid disabling the corresponding warning for all
targets depending on //base/test:test_support (//third_party/cec is a
public_deps of //base/test:test_support).

BUG= 666723 

Review-Url: https://codereview.chromium.org/2512783005
Cr-Commit-Position: refs/heads/master@{#434180}

[modify] https://crrev.com/bfd9c73b16d0bdb92ab04b1d5606fe82fe5fdac2/ash/system/web_notification/web_notification_tray_unittest.cc
[modify] https://crrev.com/bfd9c73b16d0bdb92ab04b1d5606fe82fe5fdac2/chrome/browser/printing/print_preview_dialog_controller_browsertest.cc
[modify] https://crrev.com/bfd9c73b16d0bdb92ab04b1d5606fe82fe5fdac2/chrome/browser/ui/cocoa/omnibox/omnibox_popup_cell.mm
[modify] https://crrev.com/bfd9c73b16d0bdb92ab04b1d5606fe82fe5fdac2/chrome/browser/ui/cocoa/omnibox/omnibox_view_mac.mm
[modify] https://crrev.com/bfd9c73b16d0bdb92ab04b1d5606fe82fe5fdac2/chrome/test/base/tracing_browsertest.cc
[modify] https://crrev.com/bfd9c73b16d0bdb92ab04b1d5606fe82fe5fdac2/components/leveldb_proto/proto_database_impl.h
[modify] https://crrev.com/bfd9c73b16d0bdb92ab04b1d5606fe82fe5fdac2/components/search_engines/template_url_prepopulate_data.cc
[modify] https://crrev.com/bfd9c73b16d0bdb92ab04b1d5606fe82fe5fdac2/device/bluetooth/bluetooth_device_unittest.cc
[modify] https://crrev.com/bfd9c73b16d0bdb92ab04b1d5606fe82fe5fdac2/third_party/ced/BUILD.gn
[modify] https://crrev.com/bfd9c73b16d0bdb92ab04b1d5606fe82fe5fdac2/ui/aura/mus/window_tree_client.cc
[modify] https://crrev.com/bfd9c73b16d0bdb92ab04b1d5606fe82fe5fdac2/ui/aura/mus/window_tree_client_unittest.cc

Status: Fixed (was: Assigned)
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 24 2016

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

commit 96444691f5a406941efad004b285eb59ef755731
Author: thakis <thakis@chromium.org>
Date: Thu Nov 24 14:41:37 2016

clang/win/x64: Fix official build after https://codereview.chromium.org/2512783005

C:\b\c\b\ClangToTWin64\src\chrome\browser\component_updater\swiftshader_component_installer.cc(240,6):
     error: unused function 'RegisterSwiftShaderPath' [-Werror,-Wunused-function]
void RegisterSwiftShaderPath(ComponentUpdateService* cus) {
     ^
1 error generated.

BUG= 666723 
TBR=sdefresne@chromium.org
NOTRY=true

Review-Url: https://codereview.chromium.org/2524193002
Cr-Commit-Position: refs/heads/master@{#434315}

[modify] https://crrev.com/96444691f5a406941efad004b285eb59ef755731/chrome/browser/component_updater/swiftshader_component_installer.cc

Sign in to add a comment