From a97276bcf7db55e724974f34e148b9504ef11042 Mon Sep 17 00:00:00 2001 From: Jonathan Ross-Perkins Date: Tue, 7 Jun 2022 10:49:17 -0700 Subject: [PATCH] TODO -> FIXME (#1316) This is for https://google.github.io/styleguide/cppguide.html#TODO_Comments; FIXME seems to be getting used for the same purpose, and we should probably only have one so that it's easier to scan for. --- docs/design/classes.md | 2 +- explorer/ast/expression.cpp | 2 +- explorer/interpreter/builtins.h | 2 +- explorer/interpreter/type_checker.cpp | 22 +++++++++---------- explorer/interpreter/type_checker.h | 4 ++-- .../match/fail_pattern_type_mismatch.carbon | 2 +- 6 files changed, 17 insertions(+), 17 deletions(-) diff --git a/docs/design/classes.md b/docs/design/classes.md index cadc82db7837..e5a9da6bd3f7 100644 --- a/docs/design/classes.md +++ b/docs/design/classes.md @@ -1913,7 +1913,7 @@ C++ constructors to initialize their base class: ##### Virtual base classes -FIXME: Ask zygoloid to fill this in. +TODO: Ask zygoloid to fill this in. Carbon won't support declaring virtual base classes, and the C++ interop use cases Carbon needs to support are limited. This will allow us to simplify the diff --git a/explorer/ast/expression.cpp b/explorer/ast/expression.cpp index e2b3115479f4..4aaa88c192f8 100644 --- a/explorer/ast/expression.cpp +++ b/explorer/ast/expression.cpp @@ -238,7 +238,7 @@ void Expression::PrintID(llvm::raw_ostream& out) const { out << "Continuation"; break; case ExpressionKind::ValueLiteral: - // FIXME: For layering reasons, we can't print out the value from here. + // TODO: For layering reasons, we can't print out the value from here. out << "ValueLiteral"; break; case ExpressionKind::IndexExpression: diff --git a/explorer/interpreter/builtins.h b/explorer/interpreter/builtins.h index a837fdc4f879..54d0b943c0b0 100644 --- a/explorer/interpreter/builtins.h +++ b/explorer/interpreter/builtins.h @@ -21,7 +21,7 @@ class Builtins { explicit Builtins() {} enum class Builtin { ImplicitAs, Last = ImplicitAs }; - // FIXME: In C++20, replace with `using enum Builtin;`. + // TODO: In C++20, replace with `using enum Builtin;`. static constexpr Builtin ImplicitAs = Builtin::ImplicitAs; // Register a declaration that might be a builtin. diff --git a/explorer/interpreter/type_checker.cpp b/explorer/interpreter/type_checker.cpp index 1962f461b26d..4c688378537f 100644 --- a/explorer/interpreter/type_checker.cpp +++ b/explorer/interpreter/type_checker.cpp @@ -229,7 +229,7 @@ auto TypeChecker::FieldTypesImplicitlyConvertible( if (!destination_field.has_value() || !IsImplicitlyConvertible(source_field.value, destination_field.value().value, - // FIXME: We don't have a way to perform + // TODO: We don't have a way to perform // user-defined conversions of a struct field // yet, because we can't write a suitable impl // for ImplicitAs. @@ -264,7 +264,7 @@ auto TypeChecker::IsImplicitlyConvertible( Nonnull source, Nonnull destination, std::optional> impl_scope) const -> bool { // Check for an exact match or for an implicit conversion. - // FIXME: `impl`s of `ImplicitAs` should be provided to cover these + // TODO: `impl`s of `ImplicitAs` should be provided to cover these // conversions. CARBON_CHECK(IsConcreteType(source)); CARBON_CHECK(IsConcreteType(destination)); @@ -354,7 +354,7 @@ auto TypeChecker::IsImplicitlyConvertible( break; } case Value::Kind::TypeType: - // FIXME: This seems suspicious. Shouldn't this require that the type + // TODO: This seems suspicious. Shouldn't this require that the type // implements the interface? if (destination->kind() == Value::Kind::InterfaceType) { return true; @@ -363,7 +363,7 @@ auto TypeChecker::IsImplicitlyConvertible( case Value::Kind::InterfaceType: case Value::Kind::TypeOfClassType: case Value::Kind::TypeOfChoiceType: - // FIXME: These types should presumably also convert to interface types. + // TODO: These types should presumably also convert to interface types. if (destination->kind() == Value::Kind::TypeType) { return true; } @@ -391,10 +391,10 @@ auto TypeChecker::ImplicitlyConvert(const std::string& context, Nonnull source, Nonnull destination) -> ErrorOr> { - // FIXME: If a builtin conversion works, for now we don't create any + // TODO: If a builtin conversion works, for now we don't create any // expression to do the conversion and rely on the interpreter to know how to // do it. - // FIXME: This doesn't work for cases of combined built-in and user-defined + // TODO: This doesn't work for cases of combined built-in and user-defined // conversion, such as converting a struct element via an `ImplicitAs` impl. if (IsImplicitlyConvertible(&source->static_type(), destination, std::nullopt)) { @@ -1586,7 +1586,7 @@ auto TypeChecker::TypeCheckExp(Nonnull e, llvm::ArrayRef> params = param_name.params().fields(); for (size_t i = 0; i != params.size(); ++i) { - // FIXME: Should we disallow all other kinds of top-level params? + // TODO: Should we disallow all other kinds of top-level params? if (auto* binding = dyn_cast(params[i])) { generic_parameters.push_back({i, binding}); if (binding->impl_binding().has_value()) { @@ -1999,13 +1999,13 @@ auto TypeChecker::TypeCheckStmt(Nonnull s, for (auto& clause : match.clauses()) { ImplScope clause_scope; clause_scope.AddParent(&impl_scope); - // FIXME: Should user-defined conversions be permitted in `match` + // TODO: Should user-defined conversions be permitted in `match` // statements? When would we run them? See #1283. CARBON_RETURN_IF_ERROR(TypeCheckPattern( &clause.pattern(), &match.expression().static_type(), clause_scope, ValueCategory::Let)); if (expected_type.has_value()) { - // FIXME: For now, we require all patterns to have the same type. If + // TODO: For now, we require all patterns to have the same type. If // that's not the same type as the scrutinee, we will convert the // scrutinee. We might want to instead allow a different conversion // to be performed for each pattern. @@ -2576,7 +2576,7 @@ auto TypeChecker::DeclareImplDeclaration(Nonnull impl_decl, binding_map[iface_decl.self()] = impl_type_value; Nonnull iface_mem_type = Substitute(binding_map, &m->static_type()); - // FIXME: How should the signature in the implementation be permitted + // TODO: How should the signature in the implementation be permitted // to differ from the signature in the interface? CARBON_RETURN_IF_ERROR( ExpectExactType((*mem)->source_loc(), "member of implementation", @@ -2715,7 +2715,7 @@ auto TypeChecker::TypeCheck(AST& ast) -> ErrorOr { for (Nonnull decl : ast.declarations) { CARBON_RETURN_IF_ERROR(TypeCheckDeclaration(decl, impl_scope)); // Check to see if this declaration is a builtin. - // FIXME: Only do this when type-checking the prelude. + // TODO: Only do this when type-checking the prelude. builtins_.Register(decl); } CARBON_RETURN_IF_ERROR(TypeCheckExp(*ast.main_call, impl_scope)); diff --git a/explorer/interpreter/type_checker.h b/explorer/interpreter/type_checker.h index a016da7dc544..128154a2950d 100644 --- a/explorer/interpreter/type_checker.h +++ b/explorer/interpreter/type_checker.h @@ -271,9 +271,9 @@ class TypeChecker { // // If `impl_scope` is `std::nullopt`, only built-in conversions are // considered. - // FIXME: Remove this behavior. + // TODO: Remove this behavior. // - // FIXME: Does not actually perform the conversion if a user-defined + // TODO: Does not actually perform the conversion if a user-defined // conversion is needed. Should be used very rarely for that reason. auto ExpectType(SourceLocation source_loc, const std::string& context, Nonnull expected, Nonnull actual, diff --git a/explorer/testdata/match/fail_pattern_type_mismatch.carbon b/explorer/testdata/match/fail_pattern_type_mismatch.carbon index 4a99504f8779..49b620e1e7eb 100644 --- a/explorer/testdata/match/fail_pattern_type_mismatch.carbon +++ b/explorer/testdata/match/fail_pattern_type_mismatch.carbon @@ -18,7 +18,7 @@ impl A as ImplicitAs(i32) { fn Main() -> i32 { var a: A = {}; - // FIXME: It's not clear whether this should be valid. The patterns here have + // TODO: It's not clear whether this should be valid. The patterns here have // different types, but we could perform different conversions on the source // expression when checking each pattern. match ((a, a)) {