Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compilation failure in C++ runfiles.h under Werror=shadow #16796

Closed
jwnimmer-tri opened this issue Nov 18, 2022 · 3 comments
Closed

Compilation failure in C++ runfiles.h under Werror=shadow #16796

jwnimmer-tri opened this issue Nov 18, 2022 · 3 comments
Labels

Comments

@jwnimmer-tri
Copy link
Contributor

jwnimmer-tri commented Nov 18, 2022

Description of the bug:

This commit at 8f28513 introduces a typo in the Runfiles constructor:

private:
Runfiles(
std::map<std::string, std::string> runfiles_map, std::string directory,
std::map<std::pair<std::string, std::string>, std::string> repo_mapping,
std::vector<std::pair<std::string, std::string> > envvars,
std::string source_repository_)
: runfiles_map_(std::move(runfiles_map)),
directory_(std::move(directory)),
repo_mapping_(std::move(repo_mapping)),
envvars_(std::move(envvars)),
source_repository_(std::move(source_repository_)) {}

Note that the argument name on L198 is std::string source_repository_ with an underscore; only member fields should end with an underscore.

When compiled using -Werror=shadow, GCC fails:

external/bazel_tools/tools/cpp/runfiles/runfiles.h: In constructor 'bazel::tools::cpp::runfiles::Runfiles::Runfiles(std::map<std::__cxx11::basic_string<char>, std::__cxx11::basic_string<char> >, std::string, std::map<std::pair<std::__cxx11::basic_string<char>, std::__cxx11::basic_string<char> >, std::__cxx11::basic_string<char> >, std::vector<std::pair<std::__cxx11::basic_string<char>, std::__cxx11::basic_string<char> > >, std::string)':
external/bazel_tools/tools/cpp/runfiles/runfiles.h:199:7: error: declaration of 'source_repository_' shadows a member of 'bazel::tools::cpp::runfiles::Runfiles' [-Werror=shadow]
  199 |       : runfiles_map_(std::move(runfiles_map)),
      |       ^
external/bazel_tools/tools/cpp/runfiles/runfiles.h:219:21: note: shadowed declaration is here
  219 |   const std::string source_repository_;
      |                     ^~~~~~~~~~~~~~~~~~

This is from GCC 9.4.0 on Ubuntu 20.04, but I suspect most all versions would likewise fail.

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Any C++ code that does #include "tools/cpp/runfiles/runfiles.h" built with bazel build --copt=-Werror=shadow should demonstrate this. I can provide a full repro if needed, but it doesn't seem necessary in this case.

Which operating system are you running Bazel on?

Ubuntu 20.04

What is the output of bazel info release?

6.0.0rc2

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

This is a regression in the 6.0.0 release candidate.

@fmeum
Copy link
Collaborator

fmeum commented Nov 18, 2022

@bazel-io flag

@bazel-io bazel-io added the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Nov 18, 2022
@fmeum
Copy link
Collaborator

fmeum commented Nov 18, 2022

@jwnimmer-tri Thanks for the report. Does #16801 look reasonable to you?

@meteorcloudy
Copy link
Member

@bazel-io fork 6.0.0

@bazel-io bazel-io removed the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Nov 28, 2022
Wyverald pushed a commit that referenced this issue Nov 29, 2022
Fixes #16796

Closes #16801.

PiperOrigin-RevId: 491349584
Change-Id: I7498d9343a5d7849e2d726fbc1e5447178c39d79

Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants
@meteorcloudy @fmeum @bazel-io @jwnimmer-tri and others