Skip to content

Commit

Permalink
pw_build: Specify -Wshadow-all for Clang
Browse files Browse the repository at this point in the history
GCC's `-Wshadow` is stricter than Clang's. Use `-Wshadow-all` in
internal Clang builds. Disable it in a few places as needed.

Fixes: b/346999138
Change-Id: I3647ce5f0c3d30e2615f9adadae026e401ab52ab
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/216033
Reviewed-by: Dave Roth <[email protected]>
Pigweed-Auto-Submit: Wyatt Hepler <[email protected]>
Lint: Lint 🤖 <[email protected]>
Commit-Queue: Wyatt Hepler <[email protected]>
  • Loading branch information
255 authored and CQ Bot Account committed Aug 26, 2024
1 parent c6e858e commit 16e6c7a
Show file tree
Hide file tree
Showing 17 changed files with 71 additions and 58 deletions.
5 changes: 4 additions & 1 deletion pw_build/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,10 @@ config("extra_strict_warnings") {
# This config MUST NOT be used downstream to allow for warnings to be
# added in the future without breaking downstream.
config("internal_strict_warnings") {
configs = [ ":internal_strict_warnings_core" ]
configs = [
":internal_strict_warnings_core",
":shadow_all",
]
}

# Core flags from ":internal_strict_warnings" that are enabled throughout
Expand Down
14 changes: 7 additions & 7 deletions pw_digital_io_linux/digital_io_cli.cc
Original file line number Diff line number Diff line change
Expand Up @@ -281,19 +281,19 @@ int main(int argc, char* argv[]) {
pw::Status status;
if (command == "get") {
LinuxInputConfig config(
/* index= */ index,
/* polarity= */ polarity);
/* gpio_index= */ index,
/* gpio_polarity= */ polarity);
status = GetInput(chip, config);
} else if (command == "set") {
LinuxOutputConfig config(
/* index= */ index,
/* polarity= */ polarity,
/* default_state= */ *set_value);
/* gpio_index= */ index,
/* gpio_polarity= */ polarity,
/* gpio_default_state= */ *set_value);
status = SetOutput(chip, config);
} else if (command == "watch") {
LinuxInputConfig config(
/* index= */ index,
/* polarity= */ polarity);
/* gpio_index= */ index,
/* gpio_polarity= */ polarity);
status = WatchInput(chip, config, trigger);
}

Expand Down
42 changes: 21 additions & 21 deletions pw_digital_io_linux/digital_io_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -430,8 +430,8 @@ TEST_F(DigitalIoTest, DoInput) {

auto& line = line0();
LinuxInputConfig config(
/* index= */ 0,
/* polarity= */ Polarity::kActiveHigh);
/* gpio_index= */ 0,
/* gpio_polarity= */ Polarity::kActiveHigh);

ASSERT_OK_AND_ASSIGN(auto input, chip.GetInputLine(config));

Expand Down Expand Up @@ -464,8 +464,8 @@ TEST_F(DigitalIoTest, DoInputInvert) {

auto& line = line0();
LinuxInputConfig config(
/* index= */ 0,
/* polarity= */ Polarity::kActiveLow);
/* gpio_index= */ 0,
/* gpio_polarity= */ Polarity::kActiveLow);

ASSERT_OK_AND_ASSIGN(auto input, chip.GetInputLine(config));

Expand Down Expand Up @@ -498,9 +498,9 @@ TEST_F(DigitalIoTest, DoOutput) {

auto& line = line1();
LinuxOutputConfig config(
/* index= */ 1,
/* polarity= */ Polarity::kActiveHigh,
/* default_state= */ State::kActive);
/* gpio_index= */ 1,
/* gpio_polarity= */ Polarity::kActiveHigh,
/* gpio_default_state== */ State::kActive);

ASSERT_OK_AND_ASSIGN(auto output, chip.GetOutputLine(config));

Expand Down Expand Up @@ -532,9 +532,9 @@ TEST_F(DigitalIoTest, DoOutputInvert) {

auto& line = line1();
LinuxOutputConfig config(
/* index= */ 1,
/* polarity= */ Polarity::kActiveLow,
/* default_state= */ State::kActive);
/* gpio_index= */ 1,
/* gpio_polarity= */ Polarity::kActiveLow,
/* gpio_default_state== */ State::kActive);

ASSERT_OK_AND_ASSIGN(auto output, chip.GetOutputLine(config));

Expand Down Expand Up @@ -567,9 +567,9 @@ TEST_F(DigitalIoTest, OutputGetState) {

auto& line = line1();
LinuxOutputConfig config(
/* index= */ 1,
/* polarity= */ Polarity::kActiveHigh,
/* default_state= */ State::kInactive);
/* gpio_index= */ 1,
/* gpio_polarity= */ Polarity::kActiveHigh,
/* gpio_default_state== */ State::kInactive);

ASSERT_OK_AND_ASSIGN(auto output, chip.GetOutputLine(config));

Expand Down Expand Up @@ -603,8 +603,8 @@ TEST_F(DigitalIoTest, DoInputInterruptsEnabledBefore) {

auto& line = line0();
LinuxInputConfig config(
/* index= */ 0,
/* polarity= */ Polarity::kActiveHigh);
/* gpio_index= */ 0,
/* gpio_polarity= */ Polarity::kActiveHigh);

ASSERT_OK_AND_ASSIGN(auto input, chip.GetInterruptLine(config, notifier));

Expand Down Expand Up @@ -634,8 +634,8 @@ TEST_F(DigitalIoTest, DoInputInterruptsEnabledAfter) {

auto& line = line0();
LinuxInputConfig config(
/* index= */ 0,
/* polarity= */ Polarity::kActiveHigh);
/* gpio_index= */ 0,
/* gpio_polarity= */ Polarity::kActiveHigh);

ASSERT_OK_AND_ASSIGN(auto input, chip.GetInterruptLine(config, notifier));

Expand Down Expand Up @@ -668,8 +668,8 @@ TEST_F(DigitalIoTest, DoInputInterruptsReadOne) {

auto& line = line0();
LinuxInputConfig config(
/* index= */ 0,
/* polarity= */ Polarity::kActiveHigh);
/* gpio_index= */ 0,
/* gpio_polarity= */ Polarity::kActiveHigh);

ASSERT_OK_AND_ASSIGN(auto input, chip.GetInterruptLine(config, notifier));

Expand Down Expand Up @@ -715,8 +715,8 @@ TEST_F(DigitalIoTest, DoInputInterruptsThread) {

auto& line = line0();
LinuxInputConfig config(
/* index= */ 0,
/* polarity= */ Polarity::kActiveHigh);
/* gpio_index= */ 0,
/* gpio_polarity= */ Polarity::kActiveHigh);

ASSERT_OK_AND_ASSIGN(auto input, chip.GetInterruptLine(config, notifier));

Expand Down
4 changes: 2 additions & 2 deletions pw_digital_io_linux/examples/input.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ pw::Status InputExample() {

// Configure input line.
LinuxInputConfig config(
/* index= */ 5,
/* polarity= */ Polarity::kActiveHigh);
/* gpio_index= */ 5,
/* gpio_polarity= */ Polarity::kActiveHigh);
PW_TRY_ASSIGN(auto input, chip.GetInputLine(config));
PW_TRY(input.Enable());

Expand Down
4 changes: 2 additions & 2 deletions pw_digital_io_linux/examples/interrupt.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ pw::Status InterruptExample() {

// Configure input line.
LinuxInputConfig config(
/* index= */ 5,
/* polarity= */ Polarity::kActiveHigh);
/* gpio_index= */ 5,
/* gpio_polarity= */ Polarity::kActiveHigh);
PW_TRY_ASSIGN(auto input, chip.GetInterruptLine(config, notifier));

// Configure the interrupt handler.
Expand Down
6 changes: 3 additions & 3 deletions pw_digital_io_linux/examples/output.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ pw::Status OutputExample() {
// Configure output line.
// Set the polarity to active-low and default state to active.
LinuxOutputConfig config(
/* index= */ 4,
/* polarity= */ Polarity::kActiveLow,
/* default_state= */ State::kActive);
/* gpio_index= */ 4,
/* gpio_polarity= */ Polarity::kActiveLow,
/* gpio_default_state== */ State::kActive);
PW_TRY_ASSIGN(auto output, chip.GetOutputLine(config));

// Enable the output pin. This pulls the pin to ground since the
Expand Down
15 changes: 9 additions & 6 deletions pw_digital_io_linux/public/pw_digital_io_linux/digital_io.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,22 +31,25 @@ struct LinuxConfig {
uint32_t index;
Polarity polarity;

LinuxConfig(uint32_t index, Polarity polarity)
: index(index), polarity(polarity) {}
LinuxConfig(uint32_t gpio_index, Polarity gpio_polarity)
: index(gpio_index), polarity(gpio_polarity) {}
uint32_t GetFlags() const;
};

struct LinuxInputConfig final : LinuxConfig {
LinuxInputConfig(uint32_t index, Polarity polarity)
: LinuxConfig(index, polarity) {}
LinuxInputConfig(uint32_t gpio_index, Polarity gpio_polarity)
: LinuxConfig(gpio_index, gpio_polarity) {}
uint32_t GetFlags() const;
};

struct LinuxOutputConfig final : LinuxConfig {
State default_state;

LinuxOutputConfig(uint32_t index, Polarity polarity, State default_state)
: LinuxConfig(index, polarity), default_state(default_state) {}
LinuxOutputConfig(uint32_t gpio_index,
Polarity gpio_polarity,
State gpio_default_state)
: LinuxConfig(gpio_index, gpio_polarity),
default_state(gpio_default_state) {}
uint32_t GetFlags() const;
};

Expand Down
3 changes: 1 addition & 2 deletions pw_function/function_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -380,8 +380,7 @@ TEST(Function, CallbackDestroysTargetAfterBeingCalled) {

int destroyed_count = 0;
MoveOnlyDestructionCounter destruction_counter(&destroyed_count);
Callback<void()> cb = [destruction_counter =
std::move(destruction_counter)]() {};
Callback<void()> cb = [counter = std::move(destruction_counter)]() {};
EXPECT_EQ(destroyed_count, 0);
cb();
EXPECT_EQ(destroyed_count, 1);
Expand Down
6 changes: 3 additions & 3 deletions pw_rpc/benchmark.cc
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,9 @@ void BenchmarkService::BidirectionalEcho(

auto captures = std::make_unique<Captures>(Captures{.self = this, .id = id});
new_reader_writer.set_on_next(
[captures = std::move(captures)](ConstByteSpan request) {
auto& reader_writers = captures->self->reader_writers_;
auto rw_id = captures->id;
[context = std::move(captures)](ConstByteSpan request) {
auto& reader_writers = context->self->reader_writers_;
auto rw_id = context->id;
auto reader_writer = reader_writers.find(rw_id);
if (reader_writer == reader_writers.end()) {
return;
Expand Down
8 changes: 4 additions & 4 deletions pw_rpc/client_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -110,12 +110,12 @@ TEST(RawRpcIntegrationTest, DISABLED_OnNextOverwritesItsOwnCall) {

// Chain together three calls. The first and third copy the string in normal
// order, while the second copies the string in reverse order.
ctx.call = kServiceClient.BidirectionalEcho([&ctx](ConstByteSpan data) {
ctx.call = kServiceClient.BidirectionalEcho([&ctx](ConstByteSpan data) {
ctx.receiver.ReverseCopyStringPayload(data);
ctx.call = kServiceClient.BidirectionalEcho([&ctx](ConstByteSpan data_1) {
ctx.call = kServiceClient.BidirectionalEcho([&ctx](ConstByteSpan data_2) {
ctx.receiver.ReverseCopyStringPayload(data_2);
ctx.call = kServiceClient.BidirectionalEcho(ctx.receiver.OnNext());
});
ctx.receiver.CopyStringPayload(data);
ctx.receiver.CopyStringPayload(data_1);
});

ASSERT_EQ(OkStatus(), ctx.call.Write(pw::as_bytes(pw::span("Window"))));
Expand Down
4 changes: 2 additions & 2 deletions pw_rpc/fuzz/engine.cc
Original file line number Diff line number Diff line change
Expand Up @@ -374,8 +374,8 @@ void Fuzzer::Run(const pw::Vector<uint32_t>& actions) {
callback_iterator_ = callback_actions_.begin();
} else {
threads_.emplace_back(
[this, thread_id, actions = std::move(thread_actions)]() {
for (const auto& encoded : actions) {
[this, thread_id, actions_to_perform = std::move(thread_actions)]() {
for (const auto& encoded : actions_to_perform) {
Action action(encoded);
action.set_thread_id(thread_id);
Perform(action);
Expand Down
4 changes: 2 additions & 2 deletions pw_rpc_transport/public/pw_rpc_transport/hdlc_framing.h
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,9 @@ class HdlcRpcPacketDecoder
Status Decode(ConstByteSpan buffer, OnRpcPacketDecodedCallback&& callback) {
decoder_.Process(
buffer,
[callback = std::move(callback)](Result<hdlc::Frame> hdlc_frame) {
[function = std::move(callback)](Result<hdlc::Frame> hdlc_frame) {
if (hdlc_frame.ok()) {
callback(hdlc_frame->data());
function(hdlc_frame->data());
}
});
return OkStatus();
Expand Down
4 changes: 2 additions & 2 deletions pw_rpc_transport/rpc_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ class TestService final

template <size_t kMaxPacketSize, size_t kLocalEgressQueueSize>
struct SocketRpcEndpoint {
explicit SocketRpcEndpoint(SocketRpcTransport<kMaxPacketSize>& transport)
: transport(transport),
explicit SocketRpcEndpoint(SocketRpcTransport<kMaxPacketSize>& rpc_transport)
: transport(rpc_transport),
rpc_egress("tx", transport),
tx_channels({rpc::Channel::Create<kTestChannelId>(&rpc_egress)}),
rx_channels({ChannelEgress{kTestChannelId, local_egress}}),
Expand Down
3 changes: 3 additions & 0 deletions pw_toolchain/cc/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@ pw_cc_flag_set(
flags = [
"-Wswitch-enum",
"-Wextra-semi",
# TODO: b/361229275 - Add -Wshadow-all, but for Clang only, since GCC
# does not support it.
# "-Wshadow-all",
] + select({
"@platforms//os:windows": [],
# TODO: b/243069432 - Enable pedantic warnings on Windows once passing.
Expand Down
5 changes: 4 additions & 1 deletion third_party/fuchsia/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,10 @@ pw_test("function_tests") {
defines = [ "EXPECT_NULL(arg)=EXPECT_EQ((arg), nullptr)" ]

# This test does not build with strict warnings, so disable them.
remove_configs = [ "$dir_pw_build:strict_warnings" ]
remove_configs = [
"$dir_pw_build:internal_strict_warnings",
"$dir_pw_build:strict_warnings",
]
}

group("stdcompat") {
Expand Down
1 change: 1 addition & 0 deletions third_party/fuzztest/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ config("fuzztest_public_config") {
"-Wno-shorten-64-to-32",
"-Wno-unused-parameter",
"-Wno-missing-field-initializers",
"-Wno-shadow-all",
]
cflags_cc = [ "-Wno-extra-semi" ]
if (dir_pw_third_party_fuzztest != "") {
Expand Down
1 change: 1 addition & 0 deletions third_party/googletest/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ if (dir_pw_third_party_googletest != "") {
cflags = [
"-Wno-undef",
"-Wno-conversion",
"-Wno-shadow-all",
"-Wno-switch-enum",
]
}
Expand Down

0 comments on commit 16e6c7a

Please sign in to comment.