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

Code before:
}

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 {
    // This is the fast path. However, we should still consider the topmost
    // declaration that isn't TranslationUnitDecl, because we should respect
    // attributes on the entire declaration chain.
    while (true) {

      // Template specializations (e.g., Wrapper<int>) should inherit
      // suppression attributes from their primary template or partial
      // specialization. Transform specializations to their template definitions
      // before checking for suppressions or walking up the lexical parent
      // chain.
      DeclWithIssue =
          preferTemplateDefinitionForTemplateSpecializations(DeclWithIssue);

      // Use the "lexical" parent. Eg., if the attribute is on a class, suppress
      // warnings in inline methods but not in out-of-line methods.
      const Decl *Parent =
          dyn_cast_or_null<Decl>(DeclWithIssue->getLexicalDeclContext());
      if (Parent == nullptr || isa<TranslationUnitDecl>(Parent))
        break;

      DeclWithIssue = Parent;
    }
  }

  // While some warnings are attached to AST nodes (mostly path-sensitive
  // checks), others are simply associated with a plain source location
  // or range.  Figuring out the node based on locations can be tricky,
  // so instead, we traverse the whole body of the declaration and gather
  // information on ALL suppressions.  After that we can simply check if
  // any of those suppressions affect the warning in question.
  //
  // Traversing AST of a function is not a heavy operation, but for
  // large functions with a lot of bugs it can make a dent in performance.
  // In order to avoid this scenario, we cache traversal results.
  auto InsertionResult = CachedSuppressionLocations.insert(
      std::make_pair(DeclWithIssue, CachedRanges{}));

Review comment (bug):
So far my understanding is:
- Template specialization is fine--it does inherit suppression attributes (according to the printed AST).
- The reason that we need this fix is that `getLexicalDeclContext` is not doing what we expect it to do. Though it may not be a bug in `getLexicalDeclContext` because I do not know if "lexical parent of a template specialization" is well defined.

Code after:
  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 {
    // This is the fast path. However, we should still consider the topmost
    // declaration that isn't TranslationUnitDecl, because we should respect
    // attributes on the entire declaration chain.
    while (true) {

      // Template specializations (e.g., Wrapper<int>) should inherit
      // suppression attributes from their primary template or partial
      // specialization. Transform specializations to their template definitions
      // before checking for suppressions or walking up the lexical parent
      // chain.
      // Simply taking the lexical parent of template specializations might land
      // us in a completely different namespace.
      DeclWithIssue =
          preferTemplateDefinitionForTemplateSpecializations(DeclWithIssue);

      // Use the "lexical" parent. Eg., if the attribute is on a class, suppress
      // warnings in inline methods but not in out-of-line methods.
      const Decl *Parent =
          dyn_cast_or_null<Decl>(DeclWithIssue->getLexicalDeclContext());
      if (Parent == nullptr || isa<TranslationUnitDecl>(Parent))
        break;

      DeclWithIssue = Parent;
    }
  }

  // While some warnings are attached to AST nodes (mostly path-sensitive
  // checks), others are simply associated with a plain source location
  // or range.  Figuring out the node based on locations can be tricky,
  // so instead, we traverse the whole body of the declaration and gather
  // information on ALL suppressions.  After that we can simply check if
  // any of those suppressions affect the warning in question.
  //
  // Traversing AST of a function is not a heavy operation, but for
  // large functions with a lot of bugs it can make a dent in performance.