[APINotes] Upstream Sema logic to apply API Notes to decls

[egorzhdan]

This upstreams more of the Clang API Notes functionality that is currently implemented in the Apple fork: https://github.com/apple/llvm-project/tree/next/clang/lib/APINotes

This was extracted from a larger PR: #73017

[llvmbot]

@llvm/pr-subscribers-clang

Author: Egor Zhdan (egorzhdan)

Changes

This upstreams more of the Clang API Notes functionality that is currently implemented in the Apple fork: https://github.com/apple/llvm-project/tree/next/clang/lib/APINotes

This was extracted from a larger PR: #73017

---

Patch is 47.11 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/78445.diff

10 Files Affected:

• (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+7)
• (modified) clang/include/clang/Sema/Sema.h (+6)
• (modified) clang/lib/Sema/CMakeLists.txt (+1)
• (modified) clang/lib/Sema/SemaDecl.cpp (+4)
• (modified) clang/lib/Sema/SemaDeclAttr.cpp (+3)
• (modified) clang/lib/Sema/SemaDeclCXX.cpp (+5-1)
• (modified) clang/lib/Sema/SemaDeclObjC.cpp (+4)
• (modified) clang/lib/Sema/SemaObjCProperty.cpp (+5)
• (modified) clang/lib/Sema/SemaTemplate.cpp (+7)
• (added) llvm/clang/lib/Sema/SemaAPINotes.cpp (+1014)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 03b0122d1c08f7..b705310dd37d6c 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10723,6 +10723,13 @@ def warn_imp_cast_drops_unaligned : Warning<
 
 } // end of sema category
 
+let CategoryName = "API Notes Issue" in {
+
+def err_incompatible_replacement_type : Error<
+  "API notes replacement type %0 has a different size from original type %1">;
+
+} // end of API Notes category
+
 let CategoryName = "OpenMP Issue" in {
 // OpenMP support.
 def err_omp_expected_var_arg : Error<
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 2c0ad022bcf19d..f191f791741363 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -4763,6 +4763,12 @@ class Sema final {
   bool checkCommonAttributeFeatures(const Stmt *S, const ParsedAttr &A,
                                     bool SkipArgCountCheck = false);
 
+  /// Map any API notes provided for this declaration to attributes on the
+  /// declaration.
+  ///
+  /// Triggered by declaration-attribute processing.
+  void ProcessAPINotes(Decl *D);
+
   /// Determine if type T is a valid subject for a nonnull and similar
   /// attributes. By default, we look through references (the behavior used by
   /// nonnull), but if the second parameter is true, then we treat a reference
diff --git a/clang/lib/Sema/CMakeLists.txt b/clang/lib/Sema/CMakeLists.txt
index 1856a88e9a3271..4f72bce98fbbec 100644
--- a/clang/lib/Sema/CMakeLists.txt
+++ b/clang/lib/Sema/CMakeLists.txt
@@ -27,6 +27,7 @@ add_clang_library(clangSema
   Sema.cpp
   SemaAccess.cpp
   SemaAttr.cpp
+  SemaAPINotes.cpp
   SemaAvailability.cpp
   SemaCXXScopeSpec.cpp
   SemaCast.cpp
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 9e6e4ab882c0be..d001305d5ac175 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -16364,6 +16364,7 @@ void Sema::ActOnFinishDelayedAttribute(Scope *S, Decl *D,
   if (TemplateDecl *TD = dyn_cast<TemplateDecl>(D))
     D = TD->getTemplatedDecl();
   ProcessDeclAttributeList(S, D, Attrs);
+  ProcessAPINotes(D);
 
   if (CXXMethodDecl *Method = dyn_cast_or_null<CXXMethodDecl>(D))
     if (Method->isStatic())
@@ -19583,6 +19584,7 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc, Decl *EnclosingDecl,
       CDecl->setIvarRBraceLoc(RBrac);
     }
   }
+  ProcessAPINotes(Record);
 }
 
 /// Determine whether the given integral value is representable within
@@ -19897,6 +19899,7 @@ Decl *Sema::ActOnEnumConstant(Scope *S, Decl *theEnumDecl, Decl *lastEnumConst,
   // Process attributes.
   ProcessDeclAttributeList(S, New, Attrs);
   AddPragmaAttributes(S, New);
+  ProcessAPINotes(New);
 
   // Register this decl in the current scope stack.
   New->setAccess(TheEnumDecl->getAccess());
@@ -20095,6 +20098,7 @@ void Sema::ActOnEnumBody(SourceLocation EnumLoc, SourceRange BraceRange,
   QualType EnumType = Context.getTypeDeclType(Enum);
 
   ProcessDeclAttributeList(S, Enum, Attrs);
+  ProcessAPINotes(Enum);
 
   if (Enum->isDependentType()) {
     for (unsigned i = 0, e = Elements.size(); i != e; ++i) {
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index a482919356e1bc..21d3f1bb900986 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -10129,6 +10129,9 @@ void Sema::ProcessDeclAttributes(Scope *S, Decl *D, const Declarator &PD) {
 
   // Apply additional attributes specified by '#pragma clang attribute'.
   AddPragmaAttributes(S, D);
+
+  // Look for API notes that map to attributes.
+  ProcessAPINotes(D);
 }
 
 /// Is the given declaration allowed to use a forbidden type?
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index a2ce96188b4f16..5305da18e4cd10 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -11700,6 +11700,7 @@ Decl *Sema::ActOnStartNamespaceDef(Scope *NamespcScope,
 
   ProcessDeclAttributeList(DeclRegionScope, Namespc, AttrList);
   AddPragmaAttributes(DeclRegionScope, Namespc);
+  ProcessAPINotes(Namespc);
 
   // FIXME: Should we be merging attributes?
   if (const VisibilityAttr *Attr = Namespc->getAttr<VisibilityAttr>())
@@ -12246,8 +12247,10 @@ Decl *Sema::ActOnUsingDirective(Scope *S, SourceLocation UsingLoc,
     Diag(IdentLoc, diag::err_expected_namespace_name) << SS.getRange();
   }
 
-  if (UDir)
+  if (UDir) {
     ProcessDeclAttributeList(S, UDir, AttrList);
+    ProcessAPINotes(UDir);
+  }
 
   return UDir;
 }
@@ -13539,6 +13542,7 @@ Decl *Sema::ActOnAliasDeclaration(Scope *S, AccessSpecifier AS,
 
   ProcessDeclAttributeList(S, NewTD, AttrList);
   AddPragmaAttributes(S, NewTD);
+  ProcessAPINotes(NewTD);
 
   CheckTypedefForVariablyModifiedType(S, NewTD);
   Invalid |= NewTD->isInvalidDecl();
diff --git a/clang/lib/Sema/SemaDeclObjC.cpp b/clang/lib/Sema/SemaDeclObjC.cpp
index bb0d0cd2030ba8..2011f4084dd2ab 100644
--- a/clang/lib/Sema/SemaDeclObjC.cpp
+++ b/clang/lib/Sema/SemaDeclObjC.cpp
@@ -1072,6 +1072,7 @@ ObjCInterfaceDecl *Sema::ActOnStartClassInterface(
 
   ProcessDeclAttributeList(TUScope, IDecl, AttrList);
   AddPragmaAttributes(TUScope, IDecl);
+  ProcessAPINotes(IDecl);
 
   // Merge attributes from previous declarations.
   if (PrevIDecl)
@@ -1273,6 +1274,7 @@ ObjCProtocolDecl *Sema::ActOnStartProtocolInterface(
 
   ProcessDeclAttributeList(TUScope, PDecl, AttrList);
   AddPragmaAttributes(TUScope, PDecl);
+  ProcessAPINotes(PDecl);
 
   // Merge attributes from previous declarations.
   if (PrevDecl)
@@ -4807,6 +4809,7 @@ Decl *Sema::ActOnMethodDeclaration(
     // Apply the attributes to the parameter.
     ProcessDeclAttributeList(TUScope, Param, ArgInfo[i].ArgAttrs);
     AddPragmaAttributes(TUScope, Param);
+    ProcessAPINotes(Param);
 
     if (Param->hasAttr<BlocksAttr>()) {
       Diag(Param->getLocation(), diag::err_block_on_nonlocal);
@@ -4837,6 +4840,7 @@ Decl *Sema::ActOnMethodDeclaration(
 
   ProcessDeclAttributeList(TUScope, ObjCMethod, AttrList);
   AddPragmaAttributes(TUScope, ObjCMethod);
+  ProcessAPINotes(ObjCMethod);
 
   // Add the method now.
   const ObjCMethodDecl *PrevMethod = nullptr;
diff --git a/clang/lib/Sema/SemaObjCProperty.cpp b/clang/lib/Sema/SemaObjCProperty.cpp
index 349c7fc9c91bdb..4636d89ebf2b84 100644
--- a/clang/lib/Sema/SemaObjCProperty.cpp
+++ b/clang/lib/Sema/SemaObjCProperty.cpp
@@ -2509,6 +2509,8 @@ void Sema::ProcessPropertyDecl(ObjCPropertyDecl *property) {
       GetterMethod->addAttr(SectionAttr::CreateImplicit(
           Context, SA->getName(), Loc, SectionAttr::GNU_section));
 
+    ProcessAPINotes(GetterMethod);
+
     if (getLangOpts().ObjCAutoRefCount)
       CheckARCMethodDecl(GetterMethod);
   } else
@@ -2578,6 +2580,9 @@ void Sema::ProcessPropertyDecl(ObjCPropertyDecl *property) {
       if (const SectionAttr *SA = property->getAttr<SectionAttr>())
         SetterMethod->addAttr(SectionAttr::CreateImplicit(
             Context, SA->getName(), Loc, SectionAttr::GNU_section));
+
+      ProcessAPINotes(SetterMethod);
+
       // It's possible for the user to have set a very odd custom
       // setter selector that causes it to have a method family.
       if (getLangOpts().ObjCAutoRefCount)
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index c0dcbda1fd6221..476c13760789f0 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -2154,6 +2154,7 @@ DeclResult Sema::CheckClassTemplate(
     NewClass->startDefinition();
 
   ProcessDeclAttributeList(S, NewClass, Attr);
+  ProcessAPINotes(NewClass);
 
   if (PrevClassTemplate)
     mergeDeclAttributes(NewClass, PrevClassTemplate->getTemplatedDecl());
@@ -9045,6 +9046,7 @@ DeclResult Sema::ActOnClassTemplateSpecialization(
   }
 
   ProcessDeclAttributeList(S, Specialization, Attr);
+  ProcessAPINotes(Specialization);
 
   // Add alignment attributes if necessary; these attributes are checked when
   // the ASTContext lays out the structure.
@@ -10280,6 +10282,7 @@ DeclResult Sema::ActOnExplicitInstantiation(
 
   bool PreviouslyDLLExported = Specialization->hasAttr<DLLExportAttr>();
   ProcessDeclAttributeList(S, Specialization, Attr);
+  ProcessAPINotes(Specialization);
 
   // Add the explicit instantiation into its lexical context. However,
   // since explicit instantiations are never found by name lookup, we
@@ -10690,6 +10693,9 @@ DeclResult Sema::ActOnExplicitInstantiation(Scope *S,
       Prev->setTemplateSpecializationKind(TSK, D.getIdentifierLoc());
       // Merge attributes.
       ProcessDeclAttributeList(S, Prev, D.getDeclSpec().getAttributes());
+      if (PrevTemplate)
+        ProcessAPINotes(Prev);
+
       if (TSK == TSK_ExplicitInstantiationDefinition)
         InstantiateVariableDefinition(D.getIdentifierLoc(), Prev);
     }
@@ -10865,6 +10871,7 @@ DeclResult Sema::ActOnExplicitInstantiation(Scope *S,
   }
 
   ProcessDeclAttributeList(S, Specialization, D.getDeclSpec().getAttributes());
+  ProcessAPINotes(Specialization);
 
   // In MSVC mode, dllimported explicit instantiation definitions are treated as
   // instantiation declarations.
diff --git a/llvm/clang/lib/Sema/SemaAPINotes.cpp b/llvm/clang/lib/Sema/SemaAPINotes.cpp
new file mode 100644
index 00000000000000..0a04a998918f6f
--- /dev/null
+++ b/llvm/clang/lib/Sema/SemaAPINotes.cpp
@@ -0,0 +1,1014 @@
+//===--- SemaAPINotes.cpp - API Notes Handling ----------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+//  This file implements the mapping from API notes to declaration attributes.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/APINotes/APINotesReader.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclObjC.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Sema/SemaInternal.h"
+
+using namespace clang;
+
+namespace {
+enum IsActive_t : bool { IsNotActive, IsActive };
+enum IsReplacement_t : bool { IsNotReplacement, IsReplacement };
+
+struct VersionedInfoMetadata {
+  /// An empty version refers to unversioned metadata.
+  VersionTuple Version;
+  unsigned IsActive : 1;
+  unsigned IsReplacement : 1;
+
+  VersionedInfoMetadata(VersionTuple Version, IsActive_t Active,
+                        IsReplacement_t Replacement)
+      : Version(Version), IsActive(Active == IsActive_t::IsActive),
+        IsReplacement(Replacement == IsReplacement_t::IsReplacement) {}
+};
+} // end anonymous namespace
+
+/// Determine whether this is a multi-level pointer type.
+static bool isMultiLevelPointerType(QualType Type) {
+  QualType Pointee = Type->getPointeeType();
+  if (Pointee.isNull())
+    return false;
+
+  return Pointee->isAnyPointerType() || Pointee->isObjCObjectPointerType() ||
+         Pointee->isMemberPointerType();
+}
+
+/// Apply nullability to the given declaration.
+static void applyNullability(Sema &S, Decl *D, NullabilityKind Nullability,
+                             VersionedInfoMetadata Metadata) {
+  if (!Metadata.IsActive)
+    return;
+
+  QualType Type;
+
+  // Nullability for a function/method appertains to the retain type.
+  if (auto Function = dyn_cast<FunctionDecl>(D))
+    Type = Function->getReturnType();
+  else if (auto Method = dyn_cast<ObjCMethodDecl>(D))
+    Type = Method->getReturnType();
+  else if (auto Value = dyn_cast<ValueDecl>(D))
+    Type = Value->getType();
+  else if (auto Property = dyn_cast<ObjCPropertyDecl>(D))
+    Type = Property->getType();
+  else
+    return;
+
+  // Check the nullability specifier on this type.
+  QualType OrigType = Type;
+  S.CheckImplicitNullabilityTypeSpecifier(Type, Nullability, D->getLocation(),
+                                          isa<ParmVarDecl>(D),
+                                          /*overrideExisting=*/true);
+  if (Type.getTypePtr() == OrigType.getTypePtr())
+    return;
+
+  if (auto Function = dyn_cast<FunctionDecl>(D)) {
+    const FunctionType *FnType = Function->getType()->castAs<FunctionType>();
+    if (const FunctionProtoType *Proto = dyn_cast<FunctionProtoType>(FnType))
+      Function->setType(S.Context.getFunctionType(Type, Proto->getParamTypes(),
+                                                  Proto->getExtProtoInfo()));
+    else
+      Function->setType(
+          S.Context.getFunctionNoProtoType(Type, FnType->getExtInfo()));
+  } else if (auto Method = dyn_cast<ObjCMethodDecl>(D)) {
+    Method->setReturnType(Type);
+
+    // Make it a context-sensitive keyword if we can.
+    if (!isMultiLevelPointerType(Type))
+      Method->setObjCDeclQualifier(Decl::ObjCDeclQualifier(
+          Method->getObjCDeclQualifier() | Decl::OBJC_TQ_CSNullability));
+
+  } else if (auto Value = dyn_cast<ValueDecl>(D)) {
+    Value->setType(Type);
+
+    // Make it a context-sensitive keyword if we can.
+    if (auto Parm = dyn_cast<ParmVarDecl>(D)) {
+      if (Parm->isObjCMethodParameter() && !isMultiLevelPointerType(Type))
+        Parm->setObjCDeclQualifier(Decl::ObjCDeclQualifier(
+            Parm->getObjCDeclQualifier() | Decl::OBJC_TQ_CSNullability));
+    }
+  } else if (auto Property = dyn_cast<ObjCPropertyDecl>(D)) {
+    Property->setType(Type, Property->getTypeSourceInfo());
+
+    // Make it a property attribute if we can.
+    if (!isMultiLevelPointerType(Type))
+      Property->setPropertyAttributes(
+          ObjCPropertyAttribute::kind_null_resettable);
+
+  } else
+    llvm_unreachable("cannot handle nullability here");
+}
+
+/// Copy a string into ASTContext-allocated memory.
+static StringRef CopyString(ASTContext &Ctx, StringRef String) {
+  void *mem = Ctx.Allocate(String.size(), alignof(char));
+  memcpy(mem, String.data(), String.size());
+  return StringRef(static_cast<char *>(mem), String.size());
+}
+
+static AttributeCommonInfo getDummyAttrInfo() {
+  return AttributeCommonInfo(SourceRange(),
+                             AttributeCommonInfo::UnknownAttribute,
+                             {AttributeCommonInfo::AS_GNU,
+                              /*Spelling*/ 0, /*IsAlignas*/ false,
+                              /*IsRegularKeywordAttribute*/ false});
+}
+
+namespace {
+template <typename A> struct AttrKindFor {};
+
+#define ATTR(X)                                                                \
+  template <> struct AttrKindFor<X##Attr> {                                    \
+    static const attr::Kind value = attr::X;                                   \
+  };
+#include "clang/Basic/AttrList.inc"
+
+/// Handle an attribute introduced by API notes.
+///
+/// \param ShouldAddAttribute Whether we should add a new attribute
+/// (otherwise, we might remove an existing attribute).
+/// \param CreateAttr Create the new attribute to be added.
+template <typename A>
+void handleAPINotedAttribute(
+    Sema &S, Decl *D, bool ShouldAddAttribute, VersionedInfoMetadata Metadata,
+    llvm::function_ref<A *()> CreateAttr,
+    llvm::function_ref<Decl::attr_iterator(const Decl *)> GetExistingAttr) {
+  if (Metadata.IsActive) {
+    auto Existing = GetExistingAttr(D);
+    if (Existing != D->attr_end()) {
+      // Remove the existing attribute, and treat it as a superseded
+      // non-versioned attribute.
+      auto *Versioned = SwiftVersionedAdditionAttr::CreateImplicit(
+          S.Context, Metadata.Version, *Existing, /*IsReplacedByActive*/ true);
+
+      D->getAttrs().erase(Existing);
+      D->addAttr(Versioned);
+    }
+
+    // If we're supposed to add a new attribute, do so.
+    if (ShouldAddAttribute) {
+      if (auto Attr = CreateAttr())
+        D->addAttr(Attr);
+    }
+
+  } else {
+    if (ShouldAddAttribute) {
+      if (auto Attr = CreateAttr()) {
+        auto *Versioned = SwiftVersionedAdditionAttr::CreateImplicit(
+            S.Context, Metadata.Version, Attr,
+            /*IsReplacedByActive*/ Metadata.IsReplacement);
+        D->addAttr(Versioned);
+      }
+    } else {
+      // FIXME: This isn't preserving enough information for things like
+      // availability, where we're trying to remove a /specific/ kind of
+      // attribute.
+      auto *Versioned = SwiftVersionedRemovalAttr::CreateImplicit(
+          S.Context, Metadata.Version, AttrKindFor<A>::value,
+          /*IsReplacedByActive*/ Metadata.IsReplacement);
+      D->addAttr(Versioned);
+    }
+  }
+}
+
+template <typename A>
+void handleAPINotedAttribute(Sema &S, Decl *D, bool ShouldAddAttribute,
+                             VersionedInfoMetadata Metadata,
+                             llvm::function_ref<A *()> CreateAttr) {
+  handleAPINotedAttribute<A>(
+      S, D, ShouldAddAttribute, Metadata, CreateAttr, [](const Decl *D) {
+        return llvm::find_if(D->attrs(),
+                             [](const Attr *Next) { return isa<A>(Next); });
+      });
+}
+} // namespace
+
+template <typename A = CFReturnsRetainedAttr>
+static void handleAPINotedRetainCountAttribute(Sema &S, Decl *D,
+                                               bool ShouldAddAttribute,
+                                               VersionedInfoMetadata Metadata) {
+  // The template argument has a default to make the "removal" case more
+  // concise; it doesn't matter /which/ attribute is being removed.
+  handleAPINotedAttribute<A>(
+      S, D, ShouldAddAttribute, Metadata,
+      [&] { return new (S.Context) A(S.Context, getDummyAttrInfo()); },
+      [](const Decl *D) -> Decl::attr_iterator {
+        return llvm::find_if(D->attrs(), [](const Attr *Next) -> bool {
+          return isa<CFReturnsRetainedAttr>(Next) ||
+                 isa<CFReturnsNotRetainedAttr>(Next) ||
+                 isa<NSReturnsRetainedAttr>(Next) ||
+                 isa<NSReturnsNotRetainedAttr>(Next) ||
+                 isa<CFAuditedTransferAttr>(Next);
+        });
+      });
+}
+
+static void handleAPINotedRetainCountConvention(
+    Sema &S, Decl *D, VersionedInfoMetadata Metadata,
+    std::optional<api_notes::RetainCountConventionKind> Convention) {
+  if (!Convention)
+    return;
+  switch (*Convention) {
+  case api_notes::RetainCountConventionKind::None:
+    if (isa<FunctionDecl>(D)) {
+      handleAPINotedRetainCountAttribute<CFUnknownTransferAttr>(
+          S, D, /*shouldAddAttribute*/ true, Metadata);
+    } else {
+      handleAPINotedRetainCountAttribute(S, D, /*shouldAddAttribute*/ false,
+                                         Metadata);
+    }
+    break;
+  case api_notes::RetainCountConventionKind::CFReturnsRetained:
+    handleAPINotedRetainCountAttribute<CFReturnsRetainedAttr>(
+        S, D, /*shouldAddAttribute*/ true, Metadata);
+    break;
+  case api_notes::RetainCountConventionKind::CFReturnsNotRetained:
+    handleAPINotedRetainCountAttribute<CFReturnsNotRetainedAttr>(
+        S, D, /*shouldAddAttribute*/ true, Metadata);
+    break;
+  case api_notes::RetainCountConventionKind::NSReturnsRetained:
+    handleAPINotedRetainCountAttribute<NSReturnsRetainedAttr>(
+        S, D, /*shouldAddAttribute*/ true, Metadata);
+    break;
+  case api_notes::RetainCountConventionKind::NSReturnsNotRetained:
+    handleAPINotedRetainCountAttribute<NSReturnsNotRetainedAttr>(
+        S, D, /*shouldAddAttribute*/ true, Metadata);
+    break;
+  }
+}
+
+static void ProcessAPINotes(Sema &S, Decl *D,
+                            const api_notes::CommonEntityInfo &Info,
+                            VersionedInfoMetadata Metadata) {
+  // Availability
+  if (Info.Unavailable) {
+    handleAPINotedAttribute<UnavailableAttr>(S, D, true, Metadata, [&] {
+      return new (S.Context)
+          UnavailableAttr(S.Context, getDummyAttrInfo(),
+                          CopyString(S.Context, Info.UnavailableMsg));
+    });
+  }
+
+  if (Info.UnavailableInSwift) {
+    handleAPINotedAttribute<AvailabilityAttr>(
+        S, D, true, Metadata,
+        [&] {
+          return new (S.Context) AvailabilityAttr(
+              S.Context, getDummyAttrInfo(), &S.Context.Idents.get("swift"),
+              VersionTuple(), VersionTuple(), VersionTuple(),
+              /*Unavailable=*/true, CopyString(S.Context, Info.Unavailable...
[truncated]

[egorzhdan]

@compnerd ping :)

[egorzhdan]

@compnerd ping! ;)

[egorzhdan]

@compnerd ping ;)

[compnerd]

I wonder if it is better to switch over the type id and use cast instead

[egorzhdan]

This is a bit more involved since we're actually relying on the result of ObjCContainer = in some of the conditions here. We could make this work with a switch and LLVM_FALLTHROUGH but do you think it's worth the trouble?

[compnerd]

Hmm, if its not too bad, it might be - avoiding the double dyn_cast would matter when applying larger notes I imagine.

[egorzhdan]

Hmm, I'm not sure we can do a switch here given the mutation of ObjCContainer, but I effectively removed one dyn_cast by doing getClassInterface() preemptively.

[compnerd]

Wait if there is no change to the nullability, why reset the type?

[egorzhdan]

Oops, my bad, fixed.

[compnerd]

Similar

[compnerd]

And here

[compnerd]

And here

[compnerd]

Most of the new comments are nits and small readability tweaks. I still would like to avoid the double dyn_cast checks, but I don't know if this is worth holding up on at that point. Thank you for the multiple rounds on this and sorry about the delay, the length made it daunting.

[compnerd]

I'd consider renaming these now with the enum class:

Suggested change

	enum class IsActive_t : bool { Inactive, Active };
	enum class IsReplacement_t : bool { Original, Replacement };
	enum class State : bool { Inactive, Active };
	enum class SubstitutionType : bool { Original, Replacement };

[egorzhdan]

State seems a bit too generic of a name to me, and these are still bools so they won't get more cases. I renamed IsReplacement_t to IsSubstitution_t, but I would prefer to leave IsActive_t as is.

[compnerd]

An alternative to consider: rename the function to IsModified and return the negated value so simplify the callees.

[egorzhdan]

Agreed, done.

[compnerd]

I'm a bit torn. I don't know if it helps too much, but you can possibly end the if (Metadata.IsActive) clause with return to avoid the else and undent the rest of the code.

[egorzhdan]

Sounds good, done.

[compnerd]

Might be worth a comment explaining the rationale for the default value for the template typename. It is not particularly obvious why CFReturnsRetainedAttr is being used as the default value. The default value saves the explicit parameter value at exactly 1 site I believe so I'm not entirely convinced that it is the best approach.

[egorzhdan]

Good point, let's remove the default value here.

[compnerd]

An early return feels like it is slightly beneficial.

Suggested change

	if (S.Context.getTypeSize(OrigType) !=
	S.Context.getTypeSize(ReplacementType)) {
	S.Diag(Loc, diag::err_incompatible_replacement_type)
	<< ReplacementType << OrigType;
	return true;
	}
	return false;
	if (S.Context.getTypeSize(OrigType) ==
	S.Context.getTypeSize(ReplacementType))
	return false;
	S.Diag(Loc, diag::err_incompatible_replacement_type)
	<< ReplacementType << OrigType;
	return true;

[egorzhdan]

We might in theory employ more strict checks in the future, so I think it's better to leave this as is – we would likely add another if condition with a different error message if that happens.

[compnerd]

Suggested change

[egorzhdan]

Done

[nikic]

FYI this change adds some overhead (about 0.2% for O0) builds (http://llvm-compile-time-tracker.com/compare.php?from=046682ef88a254443e8620bfd48b35bfa0a83809&to=440b1743ee0c8bfb7bf0c4b503bde5ab9af88dc0&stat=instructions:u). Is it possible to avoid it for people not using API notes?

[egorzhdan]

Oh, thanks @nikic for that data point. Let me try to avoid the overhead, I'll put up a patch tomorrow morning.

[aeubanks]

Oh, thanks @nikic for that data point. Let me try to avoid the overhead, I'll put up a patch tomorrow morning.

Did this ever happen?

[egorzhdan]

Apologies for the delay!
I looked into this performance hit, and it seems largely expected to me: the compiler spends extra time checking if there is an apinotes file for a given clang module with the notes for the declarations that are being processed. We can consider adding a Clang driver option to disable API Notes, if this performance regression is significant enough.