file_path_watcher_linux.cc fails to build with GCC 6.1.1 |
|||||
Issue description
When building Chromium for Linux with |is_clang=false| and using my distro's GCC (Fedora 24, GCC 6.1.1), building file_path_watcher_linux.cc fails with the following error:
../../base/files/file_path_watcher_linux.cc:276:1: error: 'base::{anonymous}::InotifyReader::~InotifyReader()' defined but not used [-Werror=unused-function]
InotifyReader::~InotifyReader() {
^~~~~~~~~~~~~
That's because |g_inotify_reader| is declared as a base::LazyInstance<...>::Leaky so InotifyReader's destructor is never called. This change was made years ago in commit 18c0fac7d1d48e2b85db9feb5bfc14ed0354cc8f ("Make InotifyReader a leaky lazy instance"). Since it refers to an internal bug I don't have access to, it's unclear to me if ~InotifyReader() should be removed altogether or if |g_inotify_reader| can finally be turned back into a non-leaky base::LazyInstance.
,
Aug 10 2016
,
Aug 10 2016
... what the heck, monorail. CC list fixed for real I hope.
,
Aug 10 2016
> How does gcc determine this is unused anyway? Is this complaining at link time? How does that work if there's a component build? The error happens during the build, not when linking, so |is_component_build| is irrelevant here (I've tried turning it off and on again :-). FWIW, here's my full compiler invocation: g++ -MMD -MF obj/base/base/file_path_watcher_linux.o.d -DUSE_SYMBOLIZE -DV8_DEPRECATION_WARNINGS -DENABLE_MDNS=1 -DENABLE_NOTIFICATIONS -DENABLE_PEPPER_CDMS -DENABLE_PLUGINS=1 -DENABLE_PDF=1 -DENABLE_PRINTING=1 -DENABLE_BASIC_PRINTING=1 -DENABLE_PRINT_PREVIEW=1 -DENABLE_SPELLCHECK=1 -DUSE_UDEV -DUI_COMPOSITOR_IMAGE_TRANSPORT -DUSE_AURA=1 -DUSE_PANGO=1 -DUSE_CAIRO=1 -DUSE_CLIPBOARD_AURAX11=1 -DUSE_DEFAULT_RENDER_THEME=1 -DUSE_GLIB=1 -DUSE_NSS_CERTS=1 -DUSE_X11=1 -DENABLE_WEBRTC=1 -DDISABLE_NACL -DENABLE_EXTENSIONS=1 -DENABLE_TASK_MANAGER=1 -DENABLE_THEMES=1 -DENABLE_CAPTIVE_PORTAL_DETECTION=1 -DENABLE_SESSION_SERVICE=1 -DENABLE_SUPERVISED_USERS=1 -DENABLE_SERVICE_DISCOVERY=1 -DFULL_SAFE_BROWSING -DSAFE_BROWSING_CSD -DSAFE_BROWSING_DB_LOCAL -DCHROMIUM_BUILD -DENABLE_MEDIA_ROUTER=1 -DFIELDTRIAL_TESTING_ENABLED -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D_FORTIFY_SOURCE=2 -DCOMPONENT_BUILD -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DBASE_IMPLEMENTATION -I../.. -Igen -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -fno-strict-aliasing --param=ssp-buffer-size=4 -fstack-protector -funwind-tables -fPIC -pipe -B../../third_party/binutils/Linux_x64/Release/bin -pthread -m64 -march=x86-64 -Wall -Werror -Wno-unused-local-typedefs -Wno-maybe-uninitialized -Wno-missing-field-initializers -Wno-unused-parameter -g0 -fvisibility=hidden -O2 -fno-ident -fdata-sections -ffunction-sections -fno-threadsafe-statics -fvisibility-inlines-hidden -std=gnu++11 -Wno-narrowing -fno-rtti -fno-exceptions -c ../../base/files/file_path_watcher_linux.cc -o obj/base/base/file_path_watcher_linux.o
,
Aug 10 2016
Ah I see, InotifyReader is defined and used only within the .cc, so it can make that determination. I suppose we could tweak the clang plugin to not emit a warning in this case, but I'm not really sure how high priority it would be.
,
Aug 10 2016
Hmm, I've just tried removing InotifyReader's destructor and building with Chromium's bundled clang (and its plugins) and there were no complaints. I can upload a CL with this change.
,
Aug 10 2016
Works for me. I guess I shouldn't post theories without verifying them =)
,
Aug 11 2016
https://codereview.chromium.org/2236523002/ was landed earlier today, but bugdroid remained silent. I guess it's safe to close the bug now, thanks everyone.
,
Aug 17 2016
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by dcheng@chromium.org
, Aug 10 2016