Repository: llvm/llvm-project
File: clang/lib/StaticAnalyzer/Core/BugSuppression.cpp (C++)

Code before:
// The function handles two cases:
// 1. Instantiation from a class template - searches redeclarations to find
//    the definition (not just a forward declaration).
// 2. Instantiation from a partial specialization - returns it directly.
//
// For non-template-specialization decls, returns the input unchanged.
static const Decl *
preferTemplateDefinitionForTemplateSpecializations(const Decl *D) {
  const auto *SpecializationDecl = dyn_cast<ClassTemplateSpecializationDecl>(D);
  if (!SpecializationDecl)
    return D;

  auto InstantiatedFrom = SpecializationDecl->getInstantiatedFrom();
  if (!InstantiatedFrom)
    return D;

  // This might be a class template.
  if (const auto *Tmpl = InstantiatedFrom.dyn_cast<ClassTemplateDecl *>()) {
    // Interestingly, the source template might be a forward declaration, so we
    // need to find the definition redeclaration.
    for (const auto *Redecl : Tmpl->redecls()) {
      if (cast<ClassTemplateDecl>(Redecl)->isThisDeclarationADefinition()) {
        return Redecl;
      }
    }
    assert(false && "A redecl must be a definition");
    return D;
  }

  // It might be a partial specialization.
  const auto *PartialSpecialization =
      InstantiatedFrom.dyn_cast<ClassTemplatePartialSpecializationDecl *>();

  // The partial specialization should be a definition.
  assert(PartialSpecialization->isThisDeclarationADefinition());
  return PartialSpecialization;
}

bool BugSuppression::isSuppressed(const PathDiagnosticLocation &Location,
                                  const Decl *DeclWithIssue,
                                  DiagnosticIdentifierList Hashtags) {
  if (!Location.isValid())
    return false;

  if (!DeclWithIssue) {
    // FIXME: This defeats the purpose of passing DeclWithIssue to begin with.
    // If this branch is ever hit, we're re-doing all the work we've already
    // done as well as perform a lot of work we'll never need.
    // Gladly, none of our on-by-default checkers currently need it.
    DeclWithIssue = ACtx.getTranslationUnitDecl();
  } else {

Review comment (suggestion):
```suggestion
    assert(false && "This class template must have a redecl that is a definition");
```
This string would be more clear for someone who sees the assertion printout without context (at first). To me the original would sound like "every redecl must be a definition" instead of "one of the redecls must be a definition" which you intended to express.


Code after:
// The function handles two cases:
// 1. Instantiation from a class template - searches redeclarations to find
//    the definition (not just a forward declaration).
// 2. Instantiation from a partial specialization - returns it directly.
//
// For non-template-specialization decls, returns the input unchanged.
static const Decl *
preferTemplateDefinitionForTemplateSpecializations(const Decl *D) {
  const auto *SpecializationDecl = dyn_cast<ClassTemplateSpecializationDecl>(D);
  if (!SpecializationDecl)
    return D;

  auto InstantiatedFrom = SpecializationDecl->getInstantiatedFrom();
  if (!InstantiatedFrom)
    return D;

  // This might be a class template.
  if (const auto *Tmpl = InstantiatedFrom.dyn_cast<ClassTemplateDecl *>()) {
    // Interestingly, the source template might be a forward declaration, so we
    // need to find the definition redeclaration.
    for (const auto *Redecl : Tmpl->redecls()) {
      if (cast<ClassTemplateDecl>(Redecl)->isThisDeclarationADefinition()) {
        return Redecl;
      }
    }
    assert(false &&
           "This class template must have a redecl that is a definition");
    return D;
  }

  // It might be a partial specialization.
  const auto *PartialSpecialization =
      InstantiatedFrom.dyn_cast<ClassTemplatePartialSpecializationDecl *>();

  // The partial specialization should be a definition.
  assert(PartialSpecialization->isThisDeclarationADefinition());
  return PartialSpecialization;
}

bool BugSuppression::isSuppressed(const PathDiagnosticLocation &Location,
                                  const Decl *DeclWithIssue,
                                  DiagnosticIdentifierList Hashtags) {
  if (!Location.isValid())
    return false;

  if (!DeclWithIssue) {
    // FIXME: This defeats the purpose of passing DeclWithIssue to begin with.
    // If this branch is ever hit, we're re-doing all the work we've already
    // done as well as perform a lot of work we'll never need.
    // Gladly, none of our on-by-default checkers currently need it.
    DeclWithIssue = ACtx.getTranslationUnitDecl();