Repository: carbon-language/carbon-lang
File: toolchain/check/check_unit.cpp (C++)

Code before:
          MissingOwningDeclarationInApi, Error,
          "owning declaration required for non-owning declaration");
      if (!import_ir.sem_ir->package_id().has_value() &&
          !context_.sem_ir().package_id().has_value()) {
        emitter_.Emit(function.non_owning_decl_id,
                      MissingOwningDeclarationInApi);
        continue;
      }

      if (import_ir.sem_ir->identifiers().Get(
              import_ir.sem_ir->package_id().AsIdentifierId()) ==
          context_.sem_ir().identifiers().Get(
              context_.sem_ir().package_id().AsIdentifierId())) {
        emitter_.Emit(function.non_owning_decl_id,
                      MissingOwningDeclarationInApi);
      }
    }
  }
}

auto CheckUnit::CheckRequiredDefinitions() -> void {
  CARBON_DIAGNOSTIC(MissingDefinitionInImpl, Error,
                    "no definition found for declaration in impl file");

  // Note that more required definitions can be added during this loop.
  // NOLINTNEXTLINE (modernize-loop-convert)
  for (size_t i = 0; i != context_.definitions_required_by_decl().size(); ++i) {
    SemIR::InstId decl_inst_id = context_.definitions_required_by_decl()[i];
    SemIR::Inst decl_inst = context_.insts().Get(decl_inst_id);
    CARBON_KIND_SWITCH(context_.insts().Get(decl_inst_id)) {
      case CARBON_KIND(SemIR::ClassDecl class_decl): {
        if (!context_.classes().Get(class_decl.class_id).is_defined()) {
          emitter_.Emit(decl_inst_id, MissingDefinitionInImpl);
        }
        break;
      }
      case CARBON_KIND(SemIR::FunctionDecl function_decl): {
        if (context_.functions().Get(function_decl.function_id).definition_id ==
            SemIR::InstId::None) {
          emitter_.Emit(decl_inst_id, MissingDefinitionInImpl);
        }
        break;
      }
      case CARBON_KIND(SemIR::ImplDecl impl_decl): {
        auto& impl = context_.impls().Get(impl_decl.impl_id);
        if (!impl.is_defined()) {
          FillImplWitnessWithErrors(context_, impl);
          CARBON_DIAGNOSTIC(ImplMissingDefinition, Error,
                            "impl declared but not defined");
          emitter_.Emit(decl_inst_id, ImplMissingDefinition);
        }

Review comment (suggestion):
```suggestion
  // NOLINTNEXTLINE(modernize-loop-convert)
```

If there's a space between them, `(modernize-loop-convert)` gets interpreted as a free-text comment, and so this would silence all clang-tidy warnings. See [here](https://clang.llvm.org/extra/clang-tidy/#clang-tidy-nolint).

(Idle thought: making `definitions_required_by_decl` a queue and looping until it's empty might make this code clearer, and would avoid the need to suppress this warning. I don't know if it's worth the trouble, though.)

Code after:
          MissingOwningDeclarationInApi, Error,
          "owning declaration required for non-owning declaration");
      if (!import_ir.sem_ir->package_id().has_value() &&
          !context_.sem_ir().package_id().has_value()) {
        emitter_.Emit(function.non_owning_decl_id,
                      MissingOwningDeclarationInApi);
        continue;
      }

      if (import_ir.sem_ir->identifiers().Get(
              import_ir.sem_ir->package_id().AsIdentifierId()) ==
          context_.sem_ir().identifiers().Get(
              context_.sem_ir().package_id().AsIdentifierId())) {
        emitter_.Emit(function.non_owning_decl_id,
                      MissingOwningDeclarationInApi);
      }
    }
  }
}

auto CheckUnit::CheckRequiredDefinitions() -> void {
  CARBON_DIAGNOSTIC(MissingDefinitionInImpl, Error,
                    "no definition found for declaration in impl file");

  // Note that more required definitions can be added during this loop.
  // NOLINTNEXTLINE(modernize-loop-convert)
  for (size_t i = 0; i != context_.definitions_required_by_decl().size(); ++i) {
    SemIR::InstId decl_inst_id = context_.definitions_required_by_decl()[i];
    SemIR::Inst decl_inst = context_.insts().Get(decl_inst_id);
    CARBON_KIND_SWITCH(context_.insts().Get(decl_inst_id)) {
      case CARBON_KIND(SemIR::ClassDecl class_decl): {
        if (!context_.classes().Get(class_decl.class_id).is_complete()) {
          emitter_.Emit(decl_inst_id, MissingDefinitionInImpl);
        }
        break;
      }
      case CARBON_KIND(SemIR::FunctionDecl function_decl): {
        if (context_.functions().Get(function_decl.function_id).definition_id ==
            SemIR::InstId::None) {
          emitter_.Emit(decl_inst_id, MissingDefinitionInImpl);
        }
        break;
      }
      case CARBON_KIND(SemIR::ImplDecl impl_decl): {
        auto& impl = context_.impls().Get(impl_decl.impl_id);
        if (!impl.is_complete()) {
          FillImplWitnessWithErrors(context_, impl);
          CARBON_DIAGNOSTIC(ImplMissingDefinition, Error,
                            "impl declared but not defined");
          emitter_.Emit(decl_inst_id, ImplMissingDefinition);
        }