diff --git a/.clang-format b/.clang-format index 02cfd9df7a51..15576c473583 100644 --- a/.clang-format +++ b/.clang-format @@ -10,6 +10,7 @@ AllowShortLoopsOnASingleLine: 'false' DerivePointerAlignment: 'false' ExperimentalAutoDetectBinPacking: 'false' FixNamespaceComments: 'true' +InsertBraces: 'true' PointerAlignment: Left # We abuse control macros for formatting other kinds of macros. SpaceBeforeParens: ControlStatementsExceptControlMacros diff --git a/.clang-tidy b/.clang-tidy index e969604c5d63..9375b2801fe9 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -10,14 +10,17 @@ WarningsAsErrors: '*' # - bugprone-exception-escape finds issues like out-of-memory in main(). We # don't use exceptions, so it's unlikely to find real issues. +# - bugprone-macro-parentheses has false positives in places such as using an +# argument to declare a name, which cannot have parentheses. For our limited +# use of macros, this is a common conflict. +# - bugprone-switch-missing-default-case has false positives for `enum_base.h`. +# Clang's built-in switch warnings cover most of our risk of bugs here. # - bugprone-unchecked-optional-access in clang-tidy 16 has false positives on # code like: # while (auto name_ref = insts().Get(inst_id).TryAs()) { # inst_id = name_ref->value_id; # ^ unchecked access to optional value # } -# - bugprone-switch-missing-default-case has false positives for `enum_base.h`. -# Clang's built-in switch warnings cover most of our risk of bugs here. # - google-readability-function-size overlaps with readability-function-size. # - modernize-use-designated-initializers is disabled because it fires on # creation of SemIR typed insts, for which we do not currently want to use @@ -28,9 +31,10 @@ WarningsAsErrors: '*' # modernize-pass-by-value. Checks: -*, bugprone-*, -bugprone-branch-clone, -bugprone-easily-swappable-parameters, - -bugprone-exception-escape, -bugprone-narrowing-conversions, - -bugprone-switch-missing-default-case, -bugprone-unchecked-optional-access, - google-*, -google-readability-function-size, -google-readability-todo, + -bugprone-exception-escape, -bugprone-macro-parentheses, + -bugprone-narrowing-conversions, -bugprone-switch-missing-default-case, + -bugprone-unchecked-optional-access, google-*, + -google-readability-function-size, -google-readability-todo, misc-definitions-in-headers, misc-misplaced-const, misc-redundant-expression, misc-static-assert, misc-unconventional-assign-operator, misc-uniqueptr-reset-release, misc-unused-*, modernize-*, diff --git a/common/enum_base.h b/common/enum_base.h index a642166776b2..dfdf62daf32f 100644 --- a/common/enum_base.h +++ b/common/enum_base.h @@ -173,7 +173,6 @@ class EnumBase : public Printable { // For use when multiple enums use the same list of names. #define CARBON_DEFINE_RAW_ENUM_CLASS_NO_NAMES(EnumClassName, UnderlyingType) \ namespace Internal { \ - /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \ enum class EnumClassName##RawEnum : UnderlyingType; \ } \ enum class Internal::EnumClassName##RawEnum : UnderlyingType diff --git a/common/error.h b/common/error.h index 496af17c4fb7..9364afae75a8 100644 --- a/common/error.h +++ b/common/error.h @@ -179,10 +179,9 @@ class ErrorBuilder { // argument separator. #define CARBON_PROTECT_COMMAS(...) __VA_ARGS__ -#define CARBON_RETURN_IF_ERROR_IMPL(unique_name, expr) \ - if (auto unique_name = (expr); /* NOLINT(bugprone-macro-parentheses) */ \ - !(unique_name).ok()) { \ - return std::move(unique_name).error(); \ +#define CARBON_RETURN_IF_ERROR_IMPL(unique_name, expr) \ + if (auto unique_name = (expr); !(unique_name).ok()) { \ + return std::move(unique_name).error(); \ } #define CARBON_RETURN_IF_ERROR(expr) \ @@ -190,12 +189,12 @@ class ErrorBuilder { CARBON_MAKE_UNIQUE_NAME(_llvm_error_line, __LINE__, __COUNTER__), \ CARBON_PROTECT_COMMAS(expr)) -#define CARBON_ASSIGN_OR_RETURN_IMPL(unique_name, var, expr) \ - auto unique_name = (expr); /* NOLINT(bugprone-macro-parentheses) */ \ - if (!(unique_name).ok()) { \ - return std::move(unique_name).error(); \ - } \ - var = std::move(*(unique_name)); /* NOLINT(bugprone-macro-parentheses) */ +#define CARBON_ASSIGN_OR_RETURN_IMPL(unique_name, var, expr) \ + auto unique_name = (expr); \ + if (!(unique_name).ok()) { \ + return std::move(unique_name).error(); \ + } \ + var = std::move(*(unique_name)); #define CARBON_ASSIGN_OR_RETURN(var, expr) \ CARBON_ASSIGN_OR_RETURN_IMPL( \ diff --git a/toolchain/base/kind_switch.h b/toolchain/base/kind_switch.h index f1cd5c6bfa55..b985986a8ef7 100644 --- a/toolchain/base/kind_switch.h +++ b/toolchain/base/kind_switch.h @@ -78,8 +78,6 @@ auto Cast(ValueT&& kind_switch_value) -> auto { // This uses `if` to scope the variable, and provides a dangling `else` in order // to prevent accidental `else` use. The label allows `:` to follow the macro // name, making it look more like a typical `case`. -// -// NOLINTBEGIN(bugprone-macro-parentheses) #define CARBON_KIND(typed_variable_declaration) \ ::Carbon::Internal::Kind::ForCase< \ decltype([]([[maybe_unused]] typed_variable_declaration) {})>() \ @@ -88,6 +86,5 @@ auto Cast(ValueT&& kind_switch_value) -> auto { carbon_internal_kind_switch_value); \ false) {} \ else [[maybe_unused]] CARBON_INTERNAL_KIND_LABEL(__LINE__) -// NOLINTEND(bugprone-macro-parentheses) #endif // CARBON_TOOLCHAIN_BASE_KIND_SWITCH_H_ diff --git a/toolchain/parse/node_kind.h b/toolchain/parse/node_kind.h index 369c37b40e82..560aa4eb1127 100644 --- a/toolchain/parse/node_kind.h +++ b/toolchain/parse/node_kind.h @@ -95,7 +95,6 @@ class NodeKind : public CARBON_ENUM_BASE(NodeKind) { #include "toolchain/parse/node_kind.def" constexpr int NodeKind::ValidCount = 0 -// NOLINTNEXTLINE(bugprone-macro-parentheses) #define CARBON_PARSE_NODE_KIND(Name) +1 #include "toolchain/parse/node_kind.def" ;