diff --git a/toolchain/base/kind_switch.h b/toolchain/base/kind_switch.h index ac85e58f78cd..54d8406e4696 100644 --- a/toolchain/base/kind_switch.h +++ b/toolchain/base/kind_switch.h @@ -253,6 +253,11 @@ auto Cast(SwitchT&& kind_switch_value) -> decltype(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`. +// +// Because of the dangling else, braces should always be used with a `case +// CARBON_KIND`. Otherwise, only the first statement is going to see the +// variable. Even if that sometimes works, it can lead to confusing issues when +// statements are added, and `if`/`else` coding style already requires braces. #define CARBON_KIND(typed_variable_decl) \ ::Carbon::Internal::Kind::ForCase< \ decltype(carbon_internal_kind_switch_value), \ diff --git a/toolchain/base/kind_switch_test.cpp b/toolchain/base/kind_switch_test.cpp index bdf4804375c5..0b0b3749970c 100644 --- a/toolchain/base/kind_switch_test.cpp +++ b/toolchain/base/kind_switch_test.cpp @@ -16,36 +16,33 @@ namespace { TEST(KindSwitch, Variant) { auto f = [](std::variant v) -> std::string { - RawStringOstream str; CARBON_KIND_SWITCH(v) { - case CARBON_KIND(int n): - str << "int = " << n; - return str.TakeStr(); - case CARBON_KIND(float f): - str << "float = " << f; - return str.TakeStr(); - case CARBON_KIND(char c): - str << "char = " << c; - return str.TakeStr(); + case CARBON_KIND(int n): { + return llvm::formatv("int = {0}", n); + } + case CARBON_KIND(float f): { + return llvm::formatv("float = {0}", f); + } + case CARBON_KIND(char c): { + return llvm::formatv("char = {0}", c); + } } }; EXPECT_EQ(f(int{1}), "int = 1"); - EXPECT_EQ(f(float{2}), "float = 2.000000e+00"); + EXPECT_EQ(f(float{2}), "float = 2.00"); EXPECT_EQ(f(char{'h'}), "char = h"); } TEST(KindSwitch, VariantUnusedValue) { auto f = [](std::variant v) -> std::string { - RawStringOstream str; CARBON_KIND_SWITCH(v) { - case CARBON_KIND(int n): - str << "int = " << n; - return str.TakeStr(); + case CARBON_KIND(int n): { + return llvm::formatv("int = {0}", n); + } case CARBON_KIND(float _): // The float value is not used, we see that using `_` works. - str << "float"; - return str.TakeStr(); + return "float"; } }; diff --git a/toolchain/check/impl_validation.cpp b/toolchain/check/impl_validation.cpp index f446325d4d97..9dcb6f2bd966 100644 --- a/toolchain/check/impl_validation.cpp +++ b/toolchain/check/impl_validation.cpp @@ -92,8 +92,9 @@ static auto DiagnoseFinalImplNotInSameFileAsRootSelfTypeOrInterface( break; } - case CARBON_KIND(Step::Done _): + case CARBON_KIND(Step::Done _): { CARBON_FATAL("self type is empty?"); + } default: break; diff --git a/toolchain/check/type_structure.cpp b/toolchain/check/type_structure.cpp index e44f64a1adae..1864ab177608 100644 --- a/toolchain/check/type_structure.cpp +++ b/toolchain/check/type_structure.cpp @@ -206,67 +206,86 @@ auto TypeStructureBuilder::Build(SemIR::TypeIterator type_iter) while (true) { using Step = SemIR::TypeIterator::Step; CARBON_KIND_SWITCH(type_iter.Next().any) { - case CARBON_KIND(Step::Done _): + case CARBON_KIND(Step::Done _): { // TODO: This requires 4 SmallVector moves (two here and two in the // constructor). Find a way to reduce that. return TypeStructure(std::exchange(structure_, {}), std::exchange(symbolic_type_indices_, {}), std::exchange(concrete_types_, {})); - case CARBON_KIND(Step::End _): + } + case CARBON_KIND(Step::End _): { AppendStructuralConcreteCloseParen(); break; - case CARBON_KIND(Step::ConcreteType concrete): + } + case CARBON_KIND(Step::ConcreteType concrete): { AppendStructuralConcrete(concrete.type_id); break; - case CARBON_KIND(Step::SymbolicType _): + } + case CARBON_KIND(Step::SymbolicType _): { AppendStructuralSymbolic(); break; - case CARBON_KIND(Step::TemplateType _): + } + case CARBON_KIND(Step::TemplateType _): { AppendStructuralSymbolic(); break; - case CARBON_KIND(Step::ConcreteValue value): + } + case CARBON_KIND(Step::ConcreteValue value): { AppendStructuralConcrete( context_->constant_values().Get(value.inst_id)); break; - case CARBON_KIND(Step::SymbolicValue _): + } + case CARBON_KIND(Step::SymbolicValue _): { AppendStructuralSymbolic(); break; - case CARBON_KIND(Step::StructFieldName field_name): + } + case CARBON_KIND(Step::StructFieldName field_name): { AppendStructuralConcrete(field_name.name_id); break; - case CARBON_KIND(Step::ClassStartOnly class_start): + } + case CARBON_KIND(Step::ClassStartOnly class_start): { AppendStructuralConcrete(class_start.class_id); break; - case CARBON_KIND(Step::ClassStart class_start): + } + case CARBON_KIND(Step::ClassStart class_start): { AppendStructuralConcreteOpenParen(class_start.class_id); break; - case CARBON_KIND(Step::StructStartOnly _): + } + case CARBON_KIND(Step::StructStartOnly _): { AppendStructuralConcrete(TypeStructure::ConcreteStructType()); break; - case CARBON_KIND(Step::StructStart _): + } + case CARBON_KIND(Step::StructStart _): { AppendStructuralConcreteOpenParen(TypeStructure::ConcreteStructType()); break; - case CARBON_KIND(Step::TupleStartOnly _): + } + case CARBON_KIND(Step::TupleStartOnly _): { AppendStructuralConcrete(TypeStructure::ConcreteTupleType()); break; - case CARBON_KIND(Step::TupleStart _): + } + case CARBON_KIND(Step::TupleStart _): { AppendStructuralConcreteOpenParen(TypeStructure::ConcreteTupleType()); break; - case CARBON_KIND(Step::InterfaceStartOnly interface_start): + } + case CARBON_KIND(Step::InterfaceStartOnly interface_start): { AppendStructuralConcrete(interface_start.interface_id); break; - case CARBON_KIND(Step::InterfaceStart interface_start): + } + case CARBON_KIND(Step::InterfaceStart interface_start): { AppendStructuralConcreteOpenParen(interface_start.interface_id); break; - case CARBON_KIND(Step::IntStart int_start): + } + case CARBON_KIND(Step::IntStart int_start): { AppendStructuralConcreteOpenParen(int_start.type_id); break; - case CARBON_KIND(Step::ArrayStart _): + } + case CARBON_KIND(Step::ArrayStart _): { AppendStructuralConcreteOpenParen(TypeStructure::ConcreteArrayType()); break; - case CARBON_KIND(Step::PointerStart _): + } + case CARBON_KIND(Step::PointerStart _): { AppendStructuralConcreteOpenParen(TypeStructure::ConcretePointerType()); break; + } } } } diff --git a/toolchain/lex/lex.cpp b/toolchain/lex/lex.cpp index 1d2dc5d0a238..670764dbcef3 100644 --- a/toolchain/lex/lex.cpp +++ b/toolchain/lex/lex.cpp @@ -1105,12 +1105,13 @@ auto Lexer::LexNumericLiteral(llvm::StringRef source_text, ssize_t& position) position += token_size; CARBON_KIND_SWITCH(literal->ComputeValue(emitter_)) { - case CARBON_KIND(NumericLiteral::IntValue && value): + case CARBON_KIND(NumericLiteral::IntValue && value): { return LexTokenWithPayload(TokenKind::IntLiteral, buffer_.value_stores_->ints() .AddUnsigned(std::move(value.value)) .AsTokenPayload(), byte_offset); + } case CARBON_KIND(NumericLiteral::RealValue && value): { auto real_id = buffer_.value_stores_->reals().Add( Real{.mantissa = value.mantissa, @@ -1119,8 +1120,9 @@ auto Lexer::LexNumericLiteral(llvm::StringRef source_text, ssize_t& position) return LexTokenWithPayload(TokenKind::RealLiteral, real_id.index, byte_offset); } - case CARBON_KIND(NumericLiteral::UnrecoverableError _): + case CARBON_KIND(NumericLiteral::UnrecoverableError _): { return LexTokenWithPayload(TokenKind::Error, token_size, byte_offset); + } } } diff --git a/toolchain/sem_ir/stringify.cpp b/toolchain/sem_ir/stringify.cpp index 5b918fc56683..409f3cb372fd 100644 --- a/toolchain/sem_ir/stringify.cpp +++ b/toolchain/sem_ir/stringify.cpp @@ -123,36 +123,46 @@ class StepStack { auto PushArray(llvm::ArrayRef items) -> void { for (auto item : llvm::reverse(items)) { CARBON_KIND_SWITCH(item) { - case CARBON_KIND(InstId inst_id): + case CARBON_KIND(InstId inst_id): { PushInstId(inst_id); break; - case CARBON_KIND(llvm::StringRef string): + } + case CARBON_KIND(llvm::StringRef string): { PushString(string); break; - case CARBON_KIND(NameId name_id): + } + case CARBON_KIND(NameId name_id): { PushNameId(name_id); break; - case CARBON_KIND(ElementIndex element_index): + } + case CARBON_KIND(ElementIndex element_index): { PushElementIndex(element_index); break; - case CARBON_KIND(QualifiedNameItem qualified_name): + } + case CARBON_KIND(QualifiedNameItem qualified_name): { PushQualifiedName(qualified_name.first, qualified_name.second); break; - case CARBON_KIND(EntityNameItem entity_name): + } + case CARBON_KIND(EntityNameItem entity_name): { PushEntityName(entity_name.first, entity_name.second); break; - case CARBON_KIND(EntityNameId entity_name_id): + } + case CARBON_KIND(EntityNameId entity_name_id): { PushEntityNameId(entity_name_id); break; - case CARBON_KIND(TypeId type_id): + } + case CARBON_KIND(TypeId type_id): { PushTypeId(type_id); break; - case CARBON_KIND(SpecificInterface specific_interface): + } + case CARBON_KIND(SpecificInterface specific_interface): { PushSpecificInterface(specific_interface); break; - case CARBON_KIND(llvm::ListSeparator * sep): + } + case CARBON_KIND(llvm::ListSeparator * sep): { PushString(*sep); break; + } } } }