Skip to content

Commit

Permalink
Add -Wmissing-prototypes and fix issues it finds.
Browse files Browse the repository at this point in the history
Most of these are places where we failed to include a header file and
simply never got an error about this. The fix is to include the header
file.

Most other cases are functions that should have been marked `static` but
were not. Finding all of these was a main motivation for me enabling the
warning despite how much work it is.

One complicating factor was that we weren't including the `handle.h` for
all the state-based handler functions. While this isn't a tiny amount of
code, it is just declarations and doesn't add any extra dependencies. It
also lets us have the checking for which functions need to be `static`
and which don't. For the `parse` library I had to add the `handle.h`
header as well, I tried to match the design of it in `check`.

I have also had to work around a bug in the warning, but given the value
it seems to be providing, that seems reasonable. I've filed the bug
upstream: bazelbuild/bazel#22610

I also had to use some hacks to work around limitations of Bazel rules
that wrap `cc_library` rules and don't expose `copts`. I filed a bug for
`cc_proto_library` specifically:
bazelbuild/bazel#22610
  • Loading branch information
chandlerc committed Jun 2, 2024
1 parent 89bd4a6 commit d4d2d32
Show file tree
Hide file tree
Showing 85 changed files with 167 additions and 19 deletions.
15 changes: 15 additions & 0 deletions .bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,21 @@ build --experimental_guard_against_concurrent_changes
build --per_file_copt=external/.*\.(c|cc|cpp|cxx)$@-w
build --host_per_file_copt=external/.*\.(c|cc|cpp|cxx)$@-w

# The `rules_treesitter` synthesized libraries don't allow us to inject flags,
# and compile generated code where we can't fix warnings.
build --per_file_copt=utils/treesitter/_treesitter.tree_sitter/.*\.c$@-w
build --host_per_file_copt=utils/treesitter/_treesitter.tree_sitter/.*\.c$@-w

# The `cc_proto_library` rule doesn't allow providing `copts`:
# https://github.com/bazelbuild/bazel/issues/22610
#
# We unfortunately need to work around issues with `-Wmissing-prototypes` in
# generated protobuf code, so pass the `copt` manually here. The warning issue
# is likely part of:
# https://github.com/llvm/llvm-project/issues/94138
build --per_file_copt=.*\.pb\.cc$@-Wno-missing-prototypes
build --host_per_file_copt=.*\.pb\.cc$@-Wno-missing-prototypes

# Default dynamic linking to off. While this can help build performance in some
# edge cases with very large linked executables and a slow linker, between using
# fast linkers on all platforms (LLD and the Apple linker), as well as having
Expand Down
1 change: 1 addition & 0 deletions bazel/cc_toolchains/clang_cc_toolchain_config.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ def _impl(ctx):
"-Wimplicit-fallthrough",
"-Wctad-maybe-unsupported",
"-Wextra-semi",
"-Wmissing-prototypes",
"-Wzero-as-null-pointer-constant",
# Don't warn on external code as we can't
# necessarily patch it easily. Note that these have
Expand Down
2 changes: 1 addition & 1 deletion common/check_internal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
namespace Carbon::Internal {

// Prints the buffered message.
auto PrintAfterStackTrace(void* str) -> void {
static auto PrintAfterStackTrace(void* str) -> void {
llvm::errs() << reinterpret_cast<char*>(str);
}

Expand Down
2 changes: 1 addition & 1 deletion explorer/ast/value.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1042,7 +1042,7 @@ auto TypeEqual(Nonnull<const Value*> t1, Nonnull<const Value*> t2,

// Returns true if the two values are known to be equal and are written in the
// same way at the top level.
auto ValueStructurallyEqual(
static auto ValueStructurallyEqual(
Nonnull<const Value*> v1, Nonnull<const Value*> v2,
std::optional<Nonnull<const EqualityContext*>> equality_ctx) -> bool {
if (v1 == v2) {
Expand Down
1 change: 1 addition & 0 deletions explorer/fuzzing/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ cc_fuzz_test(
name = "explorer_fuzzer",
size = "small",
srcs = ["explorer_fuzzer.cpp"],
copts = ["-Wno-missing-prototypes"],
corpus = glob(["fuzzer_corpus/*"]),
shard_count = 8,
tags = [
Expand Down
2 changes: 1 addition & 1 deletion explorer/fuzzing/ast_to_proto_main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

namespace Carbon::Testing {

auto Main(int argc, char** argv) -> ErrorOr<Success> {
static auto Main(int argc, char** argv) -> ErrorOr<Success> {
SetWorkingDirForBazel();

if (argc != 2) {
Expand Down
5 changes: 3 additions & 2 deletions explorer/interpreter/resolve_control_flow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,9 @@ static auto ResolveControlFlow(Nonnull<TraceStream*> trace_stream,
}
}

auto ResolveControlFlow(Nonnull<TraceStream*> trace_stream,
Nonnull<Declaration*> declaration) -> ErrorOr<Success> {
static auto ResolveControlFlow(Nonnull<TraceStream*> trace_stream,
Nonnull<Declaration*> declaration)
-> ErrorOr<Success> {
switch (declaration->kind()) {
case DeclarationKind::DestructorDeclaration:
case DeclarationKind::FunctionDeclaration: {
Expand Down
2 changes: 1 addition & 1 deletion testing/fuzzing/proto_to_carbon_main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

namespace Carbon {

auto Main(int argc, char** argv) -> ErrorOr<Success> {
static auto Main(int argc, char** argv) -> ErrorOr<Success> {
Carbon::SetWorkingDirForBazel();

if (argc != 2) {
Expand Down
6 changes: 6 additions & 0 deletions toolchain/check/check_fuzzer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@

namespace Carbon::Testing {

// Declare this first to work around a bug in `-Wmissing-prototypes`:
// https://github.com/llvm/llvm-project/issues/94138
// NOLINTNEXTLINE: Match the documented fuzzer entry point declaration style.
extern "C" int LLVMFuzzerTestOneInput(const unsigned char* data,
std::size_t size);

// NOLINTNEXTLINE: Match the documented fuzzer entry point declaration style.
extern "C" int LLVMFuzzerTestOneInput(const unsigned char* data,
std::size_t size) {
Expand Down
8 changes: 4 additions & 4 deletions toolchain/check/eval.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -317,8 +317,8 @@ static auto PerformAggregateIndex(Context& context, SemIR::Inst inst)
}

// Enforces that an integer type has a valid bit width.
auto ValidateIntType(Context& context, SemIRLoc loc, SemIR::IntType result)
-> bool {
static auto ValidateIntType(Context& context, SemIRLoc loc,
SemIR::IntType result) -> bool {
auto bit_width =
context.insts().TryGetAs<SemIR::IntLiteral>(result.bit_width_id);
if (!bit_width) {
Expand Down Expand Up @@ -381,8 +381,8 @@ static auto ValidateFloatBitWidth(Context& context, SemIRLoc loc,
}

// Enforces that a float type has a valid bit width.
auto ValidateFloatType(Context& context, SemIRLoc loc, SemIR::FloatType result)
-> bool {
static auto ValidateFloatType(Context& context, SemIRLoc loc,
SemIR::FloatType result) -> bool {
auto bit_width =
context.insts().TryGetAs<SemIR::IntLiteral>(result.bit_width_id);
if (!bit_width) {
Expand Down
1 change: 1 addition & 0 deletions toolchain/check/handle_alias.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

#include "toolchain/check/context.h"
#include "toolchain/check/handle.h"
#include "toolchain/check/modifiers.h"
#include "toolchain/check/name_component.h"
#include "toolchain/parse/node_ids.h"
Expand Down
1 change: 1 addition & 0 deletions toolchain/check/handle_array.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "toolchain/check/context.h"
#include "toolchain/check/convert.h"
#include "toolchain/check/handle.h"
#include "toolchain/parse/node_kind.h"

namespace Carbon::Check {
Expand Down
5 changes: 3 additions & 2 deletions toolchain/check/handle_binding_pattern.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@

#include "toolchain/check/context.h"
#include "toolchain/check/convert.h"
#include "toolchain/check/handle.h"
#include "toolchain/check/return.h"
#include "toolchain/sem_ir/ids.h"
#include "toolchain/sem_ir/inst.h"

namespace Carbon::Check {

auto HandleAnyBindingPattern(Context& context, Parse::NodeId node_id,
bool is_generic) -> bool {
static auto HandleAnyBindingPattern(Context& context, Parse::NodeId node_id,
bool is_generic) -> bool {
auto [type_node, parsed_type_id] = context.node_stack().PopExprWithNodeId();
auto cast_type_id = ExprAsType(context, type_node, parsed_type_id);

Expand Down
1 change: 1 addition & 0 deletions toolchain/check/handle_call_expr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "toolchain/check/call.h"
#include "toolchain/check/context.h"
#include "toolchain/check/handle.h"
#include "toolchain/sem_ir/inst.h"

namespace Carbon::Check {
Expand Down
1 change: 1 addition & 0 deletions toolchain/check/handle_choice.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

#include "toolchain/check/context.h"
#include "toolchain/check/handle.h"

namespace Carbon::Check {

Expand Down
1 change: 1 addition & 0 deletions toolchain/check/handle_class.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "toolchain/check/convert.h"
#include "toolchain/check/decl_name_stack.h"
#include "toolchain/check/eval.h"
#include "toolchain/check/handle.h"
#include "toolchain/check/merge.h"
#include "toolchain/check/modifiers.h"
#include "toolchain/check/name_component.h"
Expand Down
1 change: 1 addition & 0 deletions toolchain/check/handle_codeblock.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

#include "toolchain/check/context.h"
#include "toolchain/check/handle.h"

namespace Carbon::Check {

Expand Down
1 change: 1 addition & 0 deletions toolchain/check/handle_export.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "toolchain/check/context.h"
#include "toolchain/check/decl_name_stack.h"
#include "toolchain/check/handle.h"
#include "toolchain/check/modifiers.h"
#include "toolchain/check/name_component.h"
#include "toolchain/parse/typed_nodes.h"
Expand Down
1 change: 1 addition & 0 deletions toolchain/check/handle_expr_statement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "toolchain/check/context.h"
#include "toolchain/check/convert.h"
#include "toolchain/check/handle.h"
#include "toolchain/sem_ir/inst.h"

namespace Carbon::Check {
Expand Down
1 change: 1 addition & 0 deletions toolchain/check/handle_file.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

#include "toolchain/check/context.h"
#include "toolchain/check/handle.h"

namespace Carbon::Check {

Expand Down
1 change: 1 addition & 0 deletions toolchain/check/handle_function.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "toolchain/check/decl_name_stack.h"
#include "toolchain/check/decl_state.h"
#include "toolchain/check/function.h"
#include "toolchain/check/handle.h"
#include "toolchain/check/interface.h"
#include "toolchain/check/merge.h"
#include "toolchain/check/modifiers.h"
Expand Down
1 change: 1 addition & 0 deletions toolchain/check/handle_if_expr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "toolchain/check/context.h"
#include "toolchain/check/convert.h"
#include "toolchain/check/handle.h"

namespace Carbon::Check {

Expand Down
1 change: 1 addition & 0 deletions toolchain/check/handle_if_statement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "toolchain/check/context.h"
#include "toolchain/check/convert.h"
#include "toolchain/check/handle.h"

namespace Carbon::Check {

Expand Down
1 change: 1 addition & 0 deletions toolchain/check/handle_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "toolchain/check/context.h"
#include "toolchain/check/convert.h"
#include "toolchain/check/decl_name_stack.h"
#include "toolchain/check/handle.h"
#include "toolchain/check/impl.h"
#include "toolchain/check/modifiers.h"
#include "toolchain/parse/typed_nodes.h"
Expand Down
1 change: 1 addition & 0 deletions toolchain/check/handle_import_and_package.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "toolchain/check/context.h"
#include "toolchain/check/decl_state.h"
#include "toolchain/check/handle.h"
#include "toolchain/check/modifiers.h"

namespace Carbon::Check {
Expand Down
1 change: 1 addition & 0 deletions toolchain/check/handle_index.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "toolchain/base/kind_switch.h"
#include "toolchain/check/context.h"
#include "toolchain/check/convert.h"
#include "toolchain/check/handle.h"
#include "toolchain/sem_ir/inst.h"

namespace Carbon::Check {
Expand Down
1 change: 1 addition & 0 deletions toolchain/check/handle_interface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

#include "toolchain/check/context.h"
#include "toolchain/check/handle.h"
#include "toolchain/check/interface.h"
#include "toolchain/check/modifiers.h"
#include "toolchain/check/name_component.h"
Expand Down
1 change: 1 addition & 0 deletions toolchain/check/handle_let.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "toolchain/check/context.h"
#include "toolchain/check/convert.h"
#include "toolchain/check/handle.h"
#include "toolchain/check/interface.h"
#include "toolchain/check/modifiers.h"
#include "toolchain/diagnostics/diagnostic_emitter.h"
Expand Down
1 change: 1 addition & 0 deletions toolchain/check/handle_literal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "toolchain/check/call.h"
#include "toolchain/check/context.h"
#include "toolchain/check/handle.h"
#include "toolchain/sem_ir/typed_insts.h"

namespace Carbon::Check {
Expand Down
1 change: 1 addition & 0 deletions toolchain/check/handle_loop_statement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "toolchain/check/context.h"
#include "toolchain/check/convert.h"
#include "toolchain/check/handle.h"

namespace Carbon::Check {

Expand Down
1 change: 1 addition & 0 deletions toolchain/check/handle_match.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "toolchain/check/context.h"
#include "toolchain/check/convert.h"
#include "toolchain/check/handle.h"

namespace Carbon::Check {

Expand Down
1 change: 1 addition & 0 deletions toolchain/check/handle_modifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "toolchain/check/context.h"
#include "toolchain/check/decl_state.h"
#include "toolchain/check/handle.h"
#include "toolchain/lex/token_kind.h"

namespace Carbon::Check {
Expand Down
1 change: 1 addition & 0 deletions toolchain/check/handle_name.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

#include "toolchain/check/context.h"
#include "toolchain/check/handle.h"
#include "toolchain/check/member_access.h"
#include "toolchain/check/name_component.h"
#include "toolchain/check/pointer_dereference.h"
Expand Down
1 change: 1 addition & 0 deletions toolchain/check/handle_named_constraint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

#include "toolchain/check/context.h"
#include "toolchain/check/handle.h"

namespace Carbon::Check {

Expand Down
1 change: 1 addition & 0 deletions toolchain/check/handle_namespace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "toolchain/check/context.h"
#include "toolchain/check/decl_state.h"
#include "toolchain/check/handle.h"
#include "toolchain/check/modifiers.h"
#include "toolchain/check/name_component.h"
#include "toolchain/sem_ir/ids.h"
Expand Down
1 change: 1 addition & 0 deletions toolchain/check/handle_noop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

#include "toolchain/check/context.h"
#include "toolchain/check/handle.h"

namespace Carbon::Check {

Expand Down
1 change: 1 addition & 0 deletions toolchain/check/handle_operator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "toolchain/check/context.h"
#include "toolchain/check/convert.h"
#include "toolchain/check/handle.h"
#include "toolchain/check/operator.h"
#include "toolchain/check/pointer_dereference.h"
#include "toolchain/diagnostics/diagnostic_emitter.h"
Expand Down
1 change: 1 addition & 0 deletions toolchain/check/handle_paren_expr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

#include "toolchain/check/context.h"
#include "toolchain/check/handle.h"

namespace Carbon::Check {

Expand Down
1 change: 1 addition & 0 deletions toolchain/check/handle_pattern_list.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

#include "toolchain/check/context.h"
#include "toolchain/check/handle.h"

namespace Carbon::Check {

Expand Down
1 change: 1 addition & 0 deletions toolchain/check/handle_return_statement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

#include "toolchain/check/context.h"
#include "toolchain/check/handle.h"
#include "toolchain/check/return.h"

namespace Carbon::Check {
Expand Down
1 change: 1 addition & 0 deletions toolchain/check/handle_struct.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "toolchain/check/context.h"
#include "toolchain/check/convert.h"
#include "toolchain/check/handle.h"

namespace Carbon::Check {

Expand Down
1 change: 1 addition & 0 deletions toolchain/check/handle_tuple_literal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

#include "toolchain/check/context.h"
#include "toolchain/check/handle.h"

namespace Carbon::Check {

Expand Down
1 change: 1 addition & 0 deletions toolchain/check/handle_variable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "toolchain/check/context.h"
#include "toolchain/check/convert.h"
#include "toolchain/check/handle.h"
#include "toolchain/check/modifiers.h"

namespace Carbon::Check {
Expand Down
Loading

0 comments on commit d4d2d32

Please sign in to comment.