Encountered warnings related to `-Wshadow` when compiling Chromium

127 views
Skip to first unread message

gz83

unread,
Feb 2, 2023, 10:31:06 AM2/2/23
to Chromium-dev
I use llvm 17 to compile the Chromium browser on the latest branch. During the compilation process, the compiler generated multiple warnings, all related to `-Wshadow`. I think we should find a way to fix these warnings instead of using flags to suppress these warnings.

./../gpu/config/gpu_control_list.cc(573,25): warning: declaration shadows a field of 'gpu::GpuControlList::Conditions' [-Wshadow]
                       [vendor_id = vendor_id](const GPUInfo::GPUDevice& gpu) {
                        ^
../../gpu/config/gpu_control_list.h(200,14): note: previous declaration is here
    uint32_t vendor_id;
             ^
1 warning generated.

 obj/chrome/browser/ui/ui/autofill_context_menu_manager.obj
../../chrome/browser/ui/autofill/autofill_context_menu_manager.cc(414,15): warning: declaration shadows a structured binding [-Wshadow]
            [&field_type = field_type](const CreditCard* card) {
              ^
../../chrome/browser/ui/autofill/autofill_context_menu_manager.cc(403,32): note: previous declaration is here
  for (const auto& [item_type, field_type] :
                               ^
../../chrome/browser/ui/autofill/autofill_context_menu_manager.cc(419,15): warning: declaration shadows a structured binding [-Wshadow]
            [&field_type = field_type](const AutofillProfile* profile) {
              ^
../../chrome/browser/ui/autofill/autofill_context_menu_manager.cc(403,32): note: previous declaration is here
  for (const auto& [item_type, field_type] :
                               ^
 obj/components/autofill/core/browser/browser/form_data_importer.obj
../../components/autofill/core/browser/form_data_importer.cc(880,36): warning: declaration shadows a structured binding [-Wshadow]
  auto is_matching_server_card = [&candidate =
                                   ^
../../components/autofill/core/browser/form_data_importer.cc(827,9): note: previous declaration is here
  auto [candidate, form_has_duplicate_cc_type] =
        ^
1 warning generated.

Peter Kasting

unread,
Feb 2, 2023, 10:49:10 AM2/2/23
to gz83, Chromium-dev
We don't suppress -Wshadow and we use llvm trunk to compile. It's not clear to me why you might be seeing errors that aren't appearing on the build bots.

PK

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/badd6d43-31d0-4380-9dce-19846189a64en%40chromium.org.

gz83

unread,
Feb 2, 2023, 11:08:47 AM2/2/23
to Chromium-dev, Peter Kasting, Chromium-dev, gz83
I'm cross compiling for Windows on a Debian system using llvm installed here https://apt.llvm.org/.

I modified the source code of Chromium, but I didn't modify the files where the warning appeared.

dan...@chromium.org

unread,
Feb 2, 2023, 11:15:53 AM2/2/23
to uiop...@gmail.com, Chromium-dev, Peter Kasting
The lexan team works on fixing warnings that show up in Chrome as we roll Clang, and they disable warnings when they are incorrect. A bug tracking these was filed here when the last clang roll was attempted.

Ho Cheung

unread,
Feb 2, 2023, 11:21:37 AM2/2/23
to Chromium-dev, danakj, Chromium-dev, Peter Kasting, Ho Cheung

Thank you, I will continue to monitor the dynamics of Bug:1412080

Bruce Dawson

unread,
Feb 2, 2023, 8:17:38 PM2/2/23
to Chromium-dev, Ho Cheung, danakj, Chromium-dev, Peter Kasting
When I looked at variable shadowing warnings at a previous job I estimated that about 2% of them represented actual bugs. Now, this was working in a code base that was (ahem) much sloppier than Chromium's, but to me this suggests two things:

1) Fixing variable shadowing can be risky because each fix comes with some small risk of causing a bug, with very low odds of fixing an actual existing bug.
2) Scanning through variable shadowing warnings can be a way of finding previously unnoticed bugs, and just fixing those may be worthwhile.

Ho Cheung

unread,
Feb 3, 2023, 3:56:04 AM2/3/23
to Chromium-dev, bruce...@google.com, Ho Cheung, danakj, Chromium-dev, Peter Kasting

In `commit 156e33ceacdb37f223561423bf8f685382079328`, the compiler has new warnings, and the files with the warnings are located in third-party libraries. I think we should fix them too.

../../third_party/skia/src/core/SkPath.cpp(2269,9): warning: implicit capture of 'this' with a capture default of '=' is deprecated [-Wdeprecated-this-capture]
         this->setConvexity(convexity);
         ^
../../third_party/skia/src/core/SkPath.cpp(2267,34): note: add an explicit capture of 'this' to capture '*this' by reference
     auto setComputedConvexity = [=](SkPathConvexity convexity){
                                  ^
                                   , this
1 warning generated.

../../third_party/skia/src/core/SkTaskGroup.cpp(27,13): warning: implicit capture of 'this' with a capture default of '=' is deprecated [-Wdeprecated-this-capture]
             fPending. fetch_add(-1, std::memory_order_release);
             ^
../../third_party/skia/src/core/SkTaskGroup.cpp(25,24): note: add an explicit capture of 'this' to capture '*this' by reference
         fExecutor. add([=] {
                        ^
                         , this
1 warning generated.

../../third_party/angle/src/compiler/translator/Compiler.cpp(1226,44): warning: implicit capture of 'this' with a capture default of '=' is deprecated [-Wdeprecated-this-capture ]
             static_cast<const TVariable *>(mSymbolTable.findBuiltIn(name, getShaderVersion()));
                                            ^
../../third_party/angle/src/compiler/translator/Compiler.cpp(1220,28): note: add an explicit capture of 'this' to capture '*this' by reference
     auto resizeVariable = [=](const ImmutableString &name, uint32_t size, uint32_t maxSize) {
                            ^
                             , this
1 warning generated.

Ho Cheung

unread,
Feb 3, 2023, 3:57:08 AM2/3/23
to Chromium-dev, Ho Cheung, bruce...@google.com, danakj, Chromium-dev, Peter Kasting
In addition, I also encountered the following errors, but they did not affect the compilation

Exception in thread Thread-2:
Traceback (most recent call last):
   File "E:\depot_tools\bootstrap-2@3_8_10_chromium_26_bin\python3\bin\lib\threading.py", line 932, in _bootstrap_inner
     self. run()
   File "E:\depot_tools\bootstrap-2@3_8_10_chromium_26_bin\python3\bin\lib\threading.py", line 870, in run
     self._target(*self._args, **self._kwargs)
   File "E:\depot_tools\bootstrap-2@3_8_10_chromium_26_bin\python3\bin\lib\subprocess.py", line 1370, in _readerthread
     buffer.append(fh.read())
UnicodeDecodeError: 'gbk' codec can't decode byte 0x92 in position 112: illegal multibyte sequence

Hans Wennborg

unread,
Feb 3, 2023, 5:52:28 AM2/3/23
to uiop...@gmail.com, Chromium-dev, bruce...@google.com, danakj, Peter Kasting

Note that Chromium only supports building with the clang version it comes with, so if you use a different llvm/clang version seeing different warnings or hitting other problems is likely.

Ho Cheung

unread,
Feb 3, 2023, 5:54:19 AM2/3/23
to Chromium-dev, Hans Wennborg, Chromium-dev, bruce...@google.com, danakj, Peter Kasting, Ho Cheung

Thank you for your attention to this issue
Reply all
Reply to author
Forward
0 new messages