Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[APINotes] Upstream the remaining API Notes fixes and tests #84773

Merged
merged 3 commits into from
Mar 27, 2024

Conversation

egorzhdan
Copy link
Contributor

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

@egorzhdan egorzhdan requested a review from compnerd March 11, 2024 15:26
@egorzhdan
Copy link
Contributor Author

This depends on #84772.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Mar 11, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 11, 2024

@llvm/pr-subscribers-clang

Author: Egor Zhdan (egorzhdan)

Changes

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


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

96 Files Affected:

  • (modified) clang/lib/Sema/SemaAPINotes.cpp (+23-18)
  • (modified) clang/lib/Sema/SemaObjCProperty.cpp (+2-2)
  • (added) clang/test/APINotes/Inputs/APINotes/SomeOtherKit.apinotes (+8)
  • (added) clang/test/APINotes/Inputs/BrokenHeaders/APINotes.apinotes (+4)
  • (added) clang/test/APINotes/Inputs/BrokenHeaders/SomeBrokenLib.h (+6)
  • (added) clang/test/APINotes/Inputs/BrokenHeaders2/APINotes.apinotes (+7)
  • (added) clang/test/APINotes/Inputs/BrokenHeaders2/SomeBrokenLib.h (+6)
  • (added) clang/test/APINotes/Inputs/Frameworks/FrameworkWithActualPrivateModule.framework/Headers/FrameworkWithActualPrivateModule.h (+1)
  • (added) clang/test/APINotes/Inputs/Frameworks/FrameworkWithActualPrivateModule.framework/Modules/module.modulemap (+5)
  • (added) clang/test/APINotes/Inputs/Frameworks/FrameworkWithActualPrivateModule.framework/Modules/module.private.modulemap (+5)
  • (added) clang/test/APINotes/Inputs/Frameworks/FrameworkWithActualPrivateModule.framework/PrivateHeaders/FrameworkWithActualPrivateModule_Private.apinotes (+1)
  • (added) clang/test/APINotes/Inputs/Frameworks/FrameworkWithActualPrivateModule.framework/PrivateHeaders/FrameworkWithActualPrivateModule_Private.h (+2)
  • (added) clang/test/APINotes/Inputs/Frameworks/FrameworkWithWrongCase.framework/Headers/FrameworkWithWrongCase.h (+1)
  • (added) clang/test/APINotes/Inputs/Frameworks/FrameworkWithWrongCase.framework/Modules/module.modulemap (+5)
  • (added) clang/test/APINotes/Inputs/Frameworks/FrameworkWithWrongCase.framework/PrivateHeaders/FrameworkWithWrongCase_Private.apinotes (+1)
  • (added) clang/test/APINotes/Inputs/Frameworks/FrameworkWithWrongCasePrivate.framework/Headers/FrameworkWithWrongCasePrivate.h (+1)
  • (added) clang/test/APINotes/Inputs/Frameworks/FrameworkWithWrongCasePrivate.framework/Modules/module.modulemap (+5)
  • (added) clang/test/APINotes/Inputs/Frameworks/FrameworkWithWrongCasePrivate.framework/Modules/module.private.modulemap (+1)
  • (added) clang/test/APINotes/Inputs/Frameworks/FrameworkWithWrongCasePrivate.framework/PrivateHeaders/FrameworkWithWrongCasePrivate_Private.apinotes (+1)
  • (added) clang/test/APINotes/Inputs/Frameworks/LayeredKit.framework/Headers/LayeredKit.h (+11)
  • (added) clang/test/APINotes/Inputs/Frameworks/LayeredKit.framework/Modules/module.modulemap (+5)
  • (added) clang/test/APINotes/Inputs/Frameworks/LayeredKitImpl.framework/Headers/LayeredKitImpl.apinotes (+9)
  • (added) clang/test/APINotes/Inputs/Frameworks/LayeredKitImpl.framework/Headers/LayeredKitImpl.h (+7)
  • (added) clang/test/APINotes/Inputs/Frameworks/LayeredKitImpl.framework/Modules/module.modulemap (+5)
  • (added) clang/test/APINotes/Inputs/Frameworks/SimpleKit.framework/Modules/module.modulemap (+5)
  • (added) clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/APINotes/SomeKit.apinotes (+74)
  • (added) clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/APINotes/SomeKit_private.apinotes (+15)
  • (added) clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKitForNullAnnotation.h (+55)
  • (added) clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Modules/module.modulemap (+5)
  • (added) clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Modules/module.private.modulemap (+8)
  • (added) clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Modules/module_private.modulemap (+8)
  • (added) clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/PrivateHeaders/SomeKit_Private.h (+16)
  • (added) clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/PrivateHeaders/SomeKit_PrivateForNullAnnotation.h (+17)
  • (added) clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/PrivateHeaders/SomeKit_private.apinotes (+15)
  • (added) clang/test/APINotes/Inputs/Frameworks/SomeOtherKit.framework/APINotes/SomeOtherKit.apinotes (+8)
  • (added) clang/test/APINotes/Inputs/Frameworks/SomeOtherKit.framework/Headers/SomeOtherKit.apinotes (+8)
  • (added) clang/test/APINotes/Inputs/Frameworks/SomeOtherKit.framework/Headers/SomeOtherKit.h (+9)
  • (added) clang/test/APINotes/Inputs/Frameworks/SomeOtherKit.framework/Modules/module.modulemap (+5)
  • (added) clang/test/APINotes/Inputs/Frameworks/TopLevelPrivateKit.framework/Headers/TopLevelPrivateKit.h (+1)
  • (added) clang/test/APINotes/Inputs/Frameworks/TopLevelPrivateKit.framework/Headers/TopLevelPrivateKit_Private.apinotes (+1)
  • (added) clang/test/APINotes/Inputs/Frameworks/TopLevelPrivateKit.framework/Modules/module.modulemap (+5)
  • (added) clang/test/APINotes/Inputs/Frameworks/TopLevelPrivateKit.framework/Modules/module.private.modulemap (+5)
  • (added) clang/test/APINotes/Inputs/Frameworks/TopLevelPrivateKit.framework/PrivateHeaders/TopLevelPrivateKit.apinotes (+1)
  • (added) clang/test/APINotes/Inputs/Frameworks/TopLevelPrivateKit.framework/PrivateHeaders/TopLevelPrivateKit_Private.apinotes (+4)
  • (added) clang/test/APINotes/Inputs/Frameworks/TopLevelPrivateKit.framework/PrivateHeaders/TopLevelPrivateKit_Private.h (+1)
  • (added) clang/test/APINotes/Inputs/Frameworks/TopLevelPrivateKit.framework/PrivateHeaders/TopLevelPrivateKit_Private_private.apinotes (+1)
  • (added) clang/test/APINotes/Inputs/Frameworks/VersionedKit.framework/Headers/VersionedKit.apinotes (+156)
  • (added) clang/test/APINotes/Inputs/Frameworks/VersionedKit.framework/Headers/VersionedKit.h (+137)
  • (added) clang/test/APINotes/Inputs/Frameworks/VersionedKit.framework/Modules/module.modulemap (+5)
  • (added) clang/test/APINotes/Inputs/Headers/APINotes.apinotes (+18)
  • (added) clang/test/APINotes/Inputs/Headers/BrokenTypes.apinotes (+10)
  • (added) clang/test/APINotes/Inputs/Headers/BrokenTypes.h (+8)
  • (added) clang/test/APINotes/Inputs/Headers/ExternCtx.apinotes (+15)
  • (added) clang/test/APINotes/Inputs/Headers/ExternCtx.h (+11)
  • (added) clang/test/APINotes/Inputs/Headers/HeaderLib.apinotes (+37)
  • (added) clang/test/APINotes/Inputs/Headers/HeaderLib.h (+19)
  • (added) clang/test/APINotes/Inputs/Headers/InstancetypeModule.apinotes (+10)
  • (added) clang/test/APINotes/Inputs/Headers/InstancetypeModule.h (+10)
  • (added) clang/test/APINotes/Inputs/Headers/ModuleWithWrongCase.h (+1)
  • (added) clang/test/APINotes/Inputs/Headers/ModuleWithWrongCasePrivate.h (+1)
  • (added) clang/test/APINotes/Inputs/Headers/ModuleWithWrongCasePrivate_Private.apinotes (+1)
  • (added) clang/test/APINotes/Inputs/Headers/ModuleWithWrongCase_Private.apinotes (+1)
  • (added) clang/test/APINotes/Inputs/Headers/Namespaces.apinotes (+53)
  • (added) clang/test/APINotes/Inputs/Headers/Namespaces.h (+39)
  • (added) clang/test/APINotes/Inputs/Headers/PrivateLib.apinotes (+4)
  • (added) clang/test/APINotes/Inputs/Headers/PrivateLib.h (+1)
  • (added) clang/test/APINotes/Inputs/Headers/PrivateLib_private.apinotes (+1)
  • (added) clang/test/APINotes/Inputs/Headers/SwiftImportAs.apinotes (+9)
  • (added) clang/test/APINotes/Inputs/Headers/SwiftImportAs.h (+6)
  • (added) clang/test/APINotes/Inputs/Headers/module.modulemap (+31)
  • (added) clang/test/APINotes/Inputs/Headers/module.private.modulemap (+5)
  • (added) clang/test/APINotes/Inputs/yaml-reader-errors/UIKit.apinotes (+65)
  • (added) clang/test/APINotes/Inputs/yaml-reader-errors/UIKit.h (+1)
  • (added) clang/test/APINotes/Inputs/yaml-reader-errors/module.modulemap (+3)
  • (added) clang/test/APINotes/availability.m (+48)
  • (added) clang/test/APINotes/broken_types.m (+19)
  • (added) clang/test/APINotes/case-for-private-apinotes-file.c (+22)
  • (added) clang/test/APINotes/extern-context.cpp (+23)
  • (added) clang/test/APINotes/instancetype.m (+9)
  • (added) clang/test/APINotes/module-cache.m (+65)
  • (added) clang/test/APINotes/namespaces.cpp (+69)
  • (added) clang/test/APINotes/nullability.c (+21)
  • (added) clang/test/APINotes/nullability.m (+44)
  • (added) clang/test/APINotes/objc-forward-declarations.m (+12)
  • (added) clang/test/APINotes/objc_designated_inits.m (+17)
  • (added) clang/test/APINotes/properties.m (+42)
  • (added) clang/test/APINotes/retain-count-convention.m (+38)
  • (added) clang/test/APINotes/search-order.m (+25)
  • (added) clang/test/APINotes/swift-import-as.cpp (+16)
  • (added) clang/test/APINotes/top-level-private-modules.c (+8)
  • (added) clang/test/APINotes/types.m (+28)
  • (added) clang/test/APINotes/versioned-multi.c (+69)
  • (added) clang/test/APINotes/versioned.m (+187)
  • (added) clang/test/APINotes/yaml-convert-diags.c (+6)
  • (added) clang/test/APINotes/yaml-parse-diags.c (+6)
  • (added) clang/test/APINotes/yaml-reader-errors.m (+5)
diff --git a/clang/lib/Sema/SemaAPINotes.cpp b/clang/lib/Sema/SemaAPINotes.cpp
index 836c633e9d2042..a3128306c664fe 100644
--- a/clang/lib/Sema/SemaAPINotes.cpp
+++ b/clang/lib/Sema/SemaAPINotes.cpp
@@ -52,49 +52,54 @@ static void applyNullability(Sema &S, Decl *D, NullabilityKind Nullability,
   if (!Metadata.IsActive)
     return;
 
-  auto IsModified = [&](Decl *D, QualType QT,
-                        NullabilityKind Nullability) -> bool {
+  auto GetModified =
+      [&](Decl *D, QualType QT,
+          NullabilityKind Nullability) -> std::optional<QualType> {
     QualType Original = QT;
     S.CheckImplicitNullabilityTypeSpecifier(QT, Nullability, D->getLocation(),
                                             isa<ParmVarDecl>(D),
                                             /*OverrideExisting=*/true);
-    return QT.getTypePtr() != Original.getTypePtr();
+    return (QT.getTypePtr() != Original.getTypePtr()) ? std::optional(QT)
+                                                      : std::nullopt;
   };
 
   if (auto Function = dyn_cast<FunctionDecl>(D)) {
-    if (IsModified(D, Function->getReturnType(), Nullability)) {
-      QualType FnType = Function->getType();
-      Function->setType(FnType);
+    if (auto Modified =
+            GetModified(D, Function->getReturnType(), Nullability)) {
+      const FunctionType *FnType = Function->getType()->castAs<FunctionType>();
+      if (const FunctionProtoType *proto = dyn_cast<FunctionProtoType>(FnType))
+        Function->setType(S.Context.getFunctionType(
+            *Modified, proto->getParamTypes(), proto->getExtProtoInfo()));
+      else
+        Function->setType(
+            S.Context.getFunctionNoProtoType(*Modified, FnType->getExtInfo()));
     }
   } else if (auto Method = dyn_cast<ObjCMethodDecl>(D)) {
-    QualType Type = Method->getReturnType();
-    if (IsModified(D, Type, Nullability)) {
-      Method->setReturnType(Type);
+    if (auto Modified = GetModified(D, Method->getReturnType(), Nullability)) {
+      Method->setReturnType(*Modified);
 
       // Make it a context-sensitive keyword if we can.
-      if (!isIndirectPointerType(Type))
+      if (!isIndirectPointerType(*Modified))
         Method->setObjCDeclQualifier(Decl::ObjCDeclQualifier(
             Method->getObjCDeclQualifier() | Decl::OBJC_TQ_CSNullability));
     }
   } else if (auto Value = dyn_cast<ValueDecl>(D)) {
-    QualType Type = Value->getType();
-    if (IsModified(D, Type, Nullability)) {
-      Value->setType(Type);
+    if (auto Modified = GetModified(D, Value->getType(), Nullability)) {
+      Value->setType(*Modified);
 
       // Make it a context-sensitive keyword if we can.
       if (auto Parm = dyn_cast<ParmVarDecl>(D)) {
-        if (Parm->isObjCMethodParameter() && !isIndirectPointerType(Type))
+        if (Parm->isObjCMethodParameter() && !isIndirectPointerType(*Modified))
           Parm->setObjCDeclQualifier(Decl::ObjCDeclQualifier(
               Parm->getObjCDeclQualifier() | Decl::OBJC_TQ_CSNullability));
       }
     }
   } else if (auto Property = dyn_cast<ObjCPropertyDecl>(D)) {
-    QualType Type = Property->getType();
-    if (IsModified(D, Type, Nullability)) {
-      Property->setType(Type, Property->getTypeSourceInfo());
+    if (auto Modified = GetModified(D, Property->getType(), Nullability)) {
+      Property->setType(*Modified, Property->getTypeSourceInfo());
 
       // Make it a property attribute if we can.
-      if (!isIndirectPointerType(Type))
+      if (!isIndirectPointerType(*Modified))
         Property->setPropertyAttributes(
             ObjCPropertyAttribute::kind_null_resettable);
     }
diff --git a/clang/lib/Sema/SemaObjCProperty.cpp b/clang/lib/Sema/SemaObjCProperty.cpp
index 4636d89ebf2b84..f9e1ad0121e2a2 100644
--- a/clang/lib/Sema/SemaObjCProperty.cpp
+++ b/clang/lib/Sema/SemaObjCProperty.cpp
@@ -638,8 +638,6 @@ ObjCPropertyDecl *Sema::CreatePropertyDecl(Scope *S,
     PDecl->setInvalidDecl();
   }
 
-  ProcessDeclAttributes(S, PDecl, FD.D);
-
   // Regardless of setter/getter attribute, we save the default getter/setter
   // selector names in anticipation of declaration of setter/getter methods.
   PDecl->setGetterName(GetterSel, GetterNameLoc);
@@ -647,6 +645,8 @@ ObjCPropertyDecl *Sema::CreatePropertyDecl(Scope *S,
   PDecl->setPropertyAttributesAsWritten(
                           makePropertyAttributesAsWritten(AttributesAsWritten));
 
+  ProcessDeclAttributes(S, PDecl, FD.D);
+
   if (Attributes & ObjCPropertyAttribute::kind_readonly)
     PDecl->setPropertyAttributes(ObjCPropertyAttribute::kind_readonly);
 
diff --git a/clang/test/APINotes/Inputs/APINotes/SomeOtherKit.apinotes b/clang/test/APINotes/Inputs/APINotes/SomeOtherKit.apinotes
new file mode 100644
index 00000000000000..ccdc4e15d34d1b
--- /dev/null
+++ b/clang/test/APINotes/Inputs/APINotes/SomeOtherKit.apinotes
@@ -0,0 +1,8 @@
+Name: SomeOtherKit
+Classes:
+  - Name: A
+    Methods:
+      - Selector:        "methodB"
+        MethodKind:      Instance
+        Availability:    none
+        AvailabilityMsg: "anything but this"
diff --git a/clang/test/APINotes/Inputs/BrokenHeaders/APINotes.apinotes b/clang/test/APINotes/Inputs/BrokenHeaders/APINotes.apinotes
new file mode 100644
index 00000000000000..d5473175ecf8e5
--- /dev/null
+++ b/clang/test/APINotes/Inputs/BrokenHeaders/APINotes.apinotes
@@ -0,0 +1,4 @@
+Name: SomeBrokenLib
+Functions:
+  - Name: do_something_with_pointers
+    Nu llabilityOfRet: O
diff --git a/clang/test/APINotes/Inputs/BrokenHeaders/SomeBrokenLib.h b/clang/test/APINotes/Inputs/BrokenHeaders/SomeBrokenLib.h
new file mode 100644
index 00000000000000..b09c6f63eae02e
--- /dev/null
+++ b/clang/test/APINotes/Inputs/BrokenHeaders/SomeBrokenLib.h
@@ -0,0 +1,6 @@
+#ifndef SOME_BROKEN_LIB_H
+#define SOME_BROKEN_LIB_H
+
+void do_something_with_pointers(int *ptr1, int *ptr2);
+
+#endif // SOME_BROKEN_LIB_H
diff --git a/clang/test/APINotes/Inputs/BrokenHeaders2/APINotes.apinotes b/clang/test/APINotes/Inputs/BrokenHeaders2/APINotes.apinotes
new file mode 100644
index 00000000000000..33eeaaada999d6
--- /dev/null
+++ b/clang/test/APINotes/Inputs/BrokenHeaders2/APINotes.apinotes
@@ -0,0 +1,7 @@
+Name: SomeBrokenLib
+Functions:
+  - Name: do_something_with_pointers
+    NullabilityOfRet: O
+  - Name: do_something_with_pointers
+    NullabilityOfRet: O
+    
diff --git a/clang/test/APINotes/Inputs/BrokenHeaders2/SomeBrokenLib.h b/clang/test/APINotes/Inputs/BrokenHeaders2/SomeBrokenLib.h
new file mode 100644
index 00000000000000..b09c6f63eae02e
--- /dev/null
+++ b/clang/test/APINotes/Inputs/BrokenHeaders2/SomeBrokenLib.h
@@ -0,0 +1,6 @@
+#ifndef SOME_BROKEN_LIB_H
+#define SOME_BROKEN_LIB_H
+
+void do_something_with_pointers(int *ptr1, int *ptr2);
+
+#endif // SOME_BROKEN_LIB_H
diff --git a/clang/test/APINotes/Inputs/Frameworks/FrameworkWithActualPrivateModule.framework/Headers/FrameworkWithActualPrivateModule.h b/clang/test/APINotes/Inputs/Frameworks/FrameworkWithActualPrivateModule.framework/Headers/FrameworkWithActualPrivateModule.h
new file mode 100644
index 00000000000000..7e816819d469a9
--- /dev/null
+++ b/clang/test/APINotes/Inputs/Frameworks/FrameworkWithActualPrivateModule.framework/Headers/FrameworkWithActualPrivateModule.h
@@ -0,0 +1 @@
+extern int FrameworkWithActualPrivateModule;
\ No newline at end of file
diff --git a/clang/test/APINotes/Inputs/Frameworks/FrameworkWithActualPrivateModule.framework/Modules/module.modulemap b/clang/test/APINotes/Inputs/Frameworks/FrameworkWithActualPrivateModule.framework/Modules/module.modulemap
new file mode 100644
index 00000000000000..859d723716be21
--- /dev/null
+++ b/clang/test/APINotes/Inputs/Frameworks/FrameworkWithActualPrivateModule.framework/Modules/module.modulemap
@@ -0,0 +1,5 @@
+framework module FrameworkWithActualPrivateModule {
+  umbrella header "FrameworkWithActualPrivateModule.h"
+  export *
+  module * { export * }
+}
diff --git a/clang/test/APINotes/Inputs/Frameworks/FrameworkWithActualPrivateModule.framework/Modules/module.private.modulemap b/clang/test/APINotes/Inputs/Frameworks/FrameworkWithActualPrivateModule.framework/Modules/module.private.modulemap
new file mode 100644
index 00000000000000..e7fafe3bcbb17f
--- /dev/null
+++ b/clang/test/APINotes/Inputs/Frameworks/FrameworkWithActualPrivateModule.framework/Modules/module.private.modulemap
@@ -0,0 +1,5 @@
+framework module FrameworkWithActualPrivateModule_Private {
+  umbrella header "FrameworkWithActualPrivateModule_Private.h"
+  export *
+  module * { export * }
+}
diff --git a/clang/test/APINotes/Inputs/Frameworks/FrameworkWithActualPrivateModule.framework/PrivateHeaders/FrameworkWithActualPrivateModule_Private.apinotes b/clang/test/APINotes/Inputs/Frameworks/FrameworkWithActualPrivateModule.framework/PrivateHeaders/FrameworkWithActualPrivateModule_Private.apinotes
new file mode 100644
index 00000000000000..831cf1e93d3519
--- /dev/null
+++ b/clang/test/APINotes/Inputs/Frameworks/FrameworkWithActualPrivateModule.framework/PrivateHeaders/FrameworkWithActualPrivateModule_Private.apinotes
@@ -0,0 +1 @@
+Name: FrameworkWithActualPrivateModule_Private
diff --git a/clang/test/APINotes/Inputs/Frameworks/FrameworkWithActualPrivateModule.framework/PrivateHeaders/FrameworkWithActualPrivateModule_Private.h b/clang/test/APINotes/Inputs/Frameworks/FrameworkWithActualPrivateModule.framework/PrivateHeaders/FrameworkWithActualPrivateModule_Private.h
new file mode 100644
index 00000000000000..3d1d2d57947a42
--- /dev/null
+++ b/clang/test/APINotes/Inputs/Frameworks/FrameworkWithActualPrivateModule.framework/PrivateHeaders/FrameworkWithActualPrivateModule_Private.h
@@ -0,0 +1,2 @@
+#include <FrameworkWithActualPrivateModule/FrameworkWithActualPrivateModule.h>
+extern int FrameworkWithActualPrivateModule_Private;
\ No newline at end of file
diff --git a/clang/test/APINotes/Inputs/Frameworks/FrameworkWithWrongCase.framework/Headers/FrameworkWithWrongCase.h b/clang/test/APINotes/Inputs/Frameworks/FrameworkWithWrongCase.framework/Headers/FrameworkWithWrongCase.h
new file mode 100644
index 00000000000000..8def0b1d8b2f90
--- /dev/null
+++ b/clang/test/APINotes/Inputs/Frameworks/FrameworkWithWrongCase.framework/Headers/FrameworkWithWrongCase.h
@@ -0,0 +1 @@
+extern int FrameworkWithWrongCase;
\ No newline at end of file
diff --git a/clang/test/APINotes/Inputs/Frameworks/FrameworkWithWrongCase.framework/Modules/module.modulemap b/clang/test/APINotes/Inputs/Frameworks/FrameworkWithWrongCase.framework/Modules/module.modulemap
new file mode 100644
index 00000000000000..e97d361039a150
--- /dev/null
+++ b/clang/test/APINotes/Inputs/Frameworks/FrameworkWithWrongCase.framework/Modules/module.modulemap
@@ -0,0 +1,5 @@
+framework module FrameworkWithWrongCase {
+  umbrella header "FrameworkWithWrongCase.h"
+  export *
+  module * { export * }
+}
diff --git a/clang/test/APINotes/Inputs/Frameworks/FrameworkWithWrongCase.framework/PrivateHeaders/FrameworkWithWrongCase_Private.apinotes b/clang/test/APINotes/Inputs/Frameworks/FrameworkWithWrongCase.framework/PrivateHeaders/FrameworkWithWrongCase_Private.apinotes
new file mode 100644
index 00000000000000..ae5447c61e33d0
--- /dev/null
+++ b/clang/test/APINotes/Inputs/Frameworks/FrameworkWithWrongCase.framework/PrivateHeaders/FrameworkWithWrongCase_Private.apinotes
@@ -0,0 +1 @@
+Name: FrameworkWithWrongCase
diff --git a/clang/test/APINotes/Inputs/Frameworks/FrameworkWithWrongCasePrivate.framework/Headers/FrameworkWithWrongCasePrivate.h b/clang/test/APINotes/Inputs/Frameworks/FrameworkWithWrongCasePrivate.framework/Headers/FrameworkWithWrongCasePrivate.h
new file mode 100644
index 00000000000000..a9e3271cf3ecf9
--- /dev/null
+++ b/clang/test/APINotes/Inputs/Frameworks/FrameworkWithWrongCasePrivate.framework/Headers/FrameworkWithWrongCasePrivate.h
@@ -0,0 +1 @@
+extern int FrameworkWithWrongCasePrivate;
\ No newline at end of file
diff --git a/clang/test/APINotes/Inputs/Frameworks/FrameworkWithWrongCasePrivate.framework/Modules/module.modulemap b/clang/test/APINotes/Inputs/Frameworks/FrameworkWithWrongCasePrivate.framework/Modules/module.modulemap
new file mode 100644
index 00000000000000..04b96adbbfeb99
--- /dev/null
+++ b/clang/test/APINotes/Inputs/Frameworks/FrameworkWithWrongCasePrivate.framework/Modules/module.modulemap
@@ -0,0 +1,5 @@
+framework module FrameworkWithWrongCasePrivate {
+  umbrella header "FrameworkWithWrongCasePrivate.h"
+  export *
+  module * { export * }
+}
diff --git a/clang/test/APINotes/Inputs/Frameworks/FrameworkWithWrongCasePrivate.framework/Modules/module.private.modulemap b/clang/test/APINotes/Inputs/Frameworks/FrameworkWithWrongCasePrivate.framework/Modules/module.private.modulemap
new file mode 100644
index 00000000000000..d6ad53cdc71797
--- /dev/null
+++ b/clang/test/APINotes/Inputs/Frameworks/FrameworkWithWrongCasePrivate.framework/Modules/module.private.modulemap
@@ -0,0 +1 @@
+module FrameworkWithWrongCasePrivate.Inner {}
diff --git a/clang/test/APINotes/Inputs/Frameworks/FrameworkWithWrongCasePrivate.framework/PrivateHeaders/FrameworkWithWrongCasePrivate_Private.apinotes b/clang/test/APINotes/Inputs/Frameworks/FrameworkWithWrongCasePrivate.framework/PrivateHeaders/FrameworkWithWrongCasePrivate_Private.apinotes
new file mode 100644
index 00000000000000..d7af293e8125f1
--- /dev/null
+++ b/clang/test/APINotes/Inputs/Frameworks/FrameworkWithWrongCasePrivate.framework/PrivateHeaders/FrameworkWithWrongCasePrivate_Private.apinotes
@@ -0,0 +1 @@
+Name: FrameworkWithWrongCasePrivate
diff --git a/clang/test/APINotes/Inputs/Frameworks/LayeredKit.framework/Headers/LayeredKit.h b/clang/test/APINotes/Inputs/Frameworks/LayeredKit.framework/Headers/LayeredKit.h
new file mode 100644
index 00000000000000..a95d19ecbe9afc
--- /dev/null
+++ b/clang/test/APINotes/Inputs/Frameworks/LayeredKit.framework/Headers/LayeredKit.h
@@ -0,0 +1,11 @@
+@import LayeredKitImpl;
+
+// @interface declarations already don't inherit attributes from forward 
+// declarations, so in order to test this properly we have to /not/ define
+// UpwardClass anywhere.
+
+// @interface UpwardClass
+// @end
+
+@protocol UpwardProto
+@end
diff --git a/clang/test/APINotes/Inputs/Frameworks/LayeredKit.framework/Modules/module.modulemap b/clang/test/APINotes/Inputs/Frameworks/LayeredKit.framework/Modules/module.modulemap
new file mode 100644
index 00000000000000..04bbe72a2b6e25
--- /dev/null
+++ b/clang/test/APINotes/Inputs/Frameworks/LayeredKit.framework/Modules/module.modulemap
@@ -0,0 +1,5 @@
+framework module LayeredKit {
+  umbrella header "LayeredKit.h"
+  export *
+  module * { export * }
+}
diff --git a/clang/test/APINotes/Inputs/Frameworks/LayeredKitImpl.framework/Headers/LayeredKitImpl.apinotes b/clang/test/APINotes/Inputs/Frameworks/LayeredKitImpl.framework/Headers/LayeredKitImpl.apinotes
new file mode 100644
index 00000000000000..bece28cfe60577
--- /dev/null
+++ b/clang/test/APINotes/Inputs/Frameworks/LayeredKitImpl.framework/Headers/LayeredKitImpl.apinotes
@@ -0,0 +1,9 @@
+Name: LayeredKitImpl
+Classes:
+- Name: PerfectlyNormalClass
+  Availability: none
+- Name: UpwardClass
+  Availability: none
+Protocols:
+- Name: UpwardProto
+  Availability: none
diff --git a/clang/test/APINotes/Inputs/Frameworks/LayeredKitImpl.framework/Headers/LayeredKitImpl.h b/clang/test/APINotes/Inputs/Frameworks/LayeredKitImpl.framework/Headers/LayeredKitImpl.h
new file mode 100644
index 00000000000000..99591d35803aa1
--- /dev/null
+++ b/clang/test/APINotes/Inputs/Frameworks/LayeredKitImpl.framework/Headers/LayeredKitImpl.h
@@ -0,0 +1,7 @@
+@protocol UpwardProto;
+@class UpwardClass;
+
+@interface PerfectlyNormalClass
+@end
+
+void doImplementationThings(UpwardClass *first, id <UpwardProto> second) __attribute((unavailable));
diff --git a/clang/test/APINotes/Inputs/Frameworks/LayeredKitImpl.framework/Modules/module.modulemap b/clang/test/APINotes/Inputs/Frameworks/LayeredKitImpl.framework/Modules/module.modulemap
new file mode 100644
index 00000000000000..58a6e55c1067f9
--- /dev/null
+++ b/clang/test/APINotes/Inputs/Frameworks/LayeredKitImpl.framework/Modules/module.modulemap
@@ -0,0 +1,5 @@
+framework module LayeredKitImpl {
+  umbrella header "LayeredKitImpl.h"
+  export *
+  module * { export * }
+}
diff --git a/clang/test/APINotes/Inputs/Frameworks/SimpleKit.framework/Modules/module.modulemap b/clang/test/APINotes/Inputs/Frameworks/SimpleKit.framework/Modules/module.modulemap
new file mode 100644
index 00000000000000..2d07e76c0a142a
--- /dev/null
+++ b/clang/test/APINotes/Inputs/Frameworks/SimpleKit.framework/Modules/module.modulemap
@@ -0,0 +1,5 @@
+framework module SimpleKit {
+  umbrella header "SimpleKit.h"
+  export *
+  module * { export * }
+}
diff --git a/clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/APINotes/SomeKit.apinotes b/clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/APINotes/SomeKit.apinotes
new file mode 100644
index 00000000000000..817af123fc77b6
--- /dev/null
+++ b/clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/APINotes/SomeKit.apinotes
@@ -0,0 +1,74 @@
+Name: SomeKit
+Classes:
+  - Name: A
+    Methods:
+      - Selector:        "transform:"
+        MethodKind:      Instance
+        Availability:    none
+        AvailabilityMsg: "anything but this"
+      - Selector: "transform:integer:"
+        MethodKind:      Instance
+        NullabilityOfRet: N
+        Nullability:      [ N, S ]
+    Properties:
+      - Name: intValue
+        PropertyKind:    Instance
+        Availability: none
+        AvailabilityMsg: "wouldn't work anyway"
+      - Name: nonnullAInstance
+        PropertyKind:    Instance
+        Nullability:     N
+      - Name: nonnullAClass
+        PropertyKind:    Class
+        Nullability:     N
+      - Name: nonnullABoth
+        Nullability:     N
+  - Name: B
+    Availability: none
+    AvailabilityMsg: "just don't"
+  - Name: C
+    Methods:
+      - Selector: "initWithA:"
+        MethodKind: Instance
+        DesignatedInit: true
+  - Name: OverriddenTypes
+    Methods:
+      - Selector: "methodToMangle:second:"
+        MethodKind: Instance
+        ResultType: 'char *'
+        Parameters:
+          - Position: 0
+            Type: 'SOMEKIT_DOUBLE *'
+          - Position: 1
+            Type: 'float *'
+    Properties:
+      - Name: intPropertyToMangle
+        PropertyKind: Instance
+        Type: 'double *'
+Functions:
+  - Name: global_int_fun
+    ResultType: 'char *'
+    Parameters:
+      - Position: 0
+        Type: 'double *'
+      - Position: 1
+        Type: 'float *'
+Globals:
+  - Name: global_int_ptr
+    Type: 'double *'
+SwiftVersions:
+  - Version: 3.0
+    Classes:
+      - Name: A
+        Methods:
+          - Selector: "transform:integer:"
+            MethodKind:      Instance
+            NullabilityOfRet: O
+            Nullability:      [ O, S ]
+        Properties:
+          - Name: explicitNonnullInstance
+            PropertyKind:    Instance
+            Nullability:     O
+          - Name: explicitNullableInstance
+            PropertyKind:    Instance
+            Nullability:     N
diff --git a/clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/APINotes/SomeKit_private.apinotes b/clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/APINotes/SomeKit_private.apinotes
new file mode 100644
index 00000000000000..28ede9dfa25c08
--- /dev/null
+++ b/clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/APINotes/SomeKit_private.apinotes
@@ -0,0 +1,15 @@
+Name: SomeKit
+Classes:
+  - Name: A
+    Methods:         
+      - Selector: "privateTransform:input:"
+        MethodKind:      Instance
+        NullabilityOfRet: N
+        Nullability:      [ N, S ]
+    Properties:
+      - Name: internalProperty
+        Nullability: N
+Protocols:
+  - Name: InternalProtocol
+    Availability: none
+    AvailabilityMsg: "not for you"
diff --git a/clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKitForNullAnnotation.h b/clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKitForNullAnnotation.h
new file mode 100644
index 00000000000000..d1eeb61991b480
--- /dev/null
+++ b/clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKitForNullAnnotation.h
@@ -0,0 +1,55 @@
+#ifndef SOMEKIT_H
+#define SOMEKIT_H
+
+#define ROOT_CLASS __attribute__((objc_root_class))
+
+ROOT_CLASS
+@interface A
+-(A*)transform:(A*)input;
+-(A*)transform:(A*)input integer:(int)integer;
+
+@property (nonatomic, readonly, retain) A* someA;
+@property (nonatomic, retain) A* someOtherA;
+
+@p...
[truncated]

The upstreamed version of this code didn't actually modify any types.
…ties

The upstreamed version of this code tried to add attributes before isClass/isInstance flag on an Objective-C property is parsed.

// Make it a property attribute if we can.
if (!isIndirectPointerType(Type))
if (!isIndirectPointerType(*Modified))
Property->setPropertyAttributes(
ObjCPropertyAttribute::kind_null_resettable);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add a test to ensure that the modified type is applied?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, we don't have a test for this, let me add one

// Regardless of setter/getter attribute, we save the default getter/setter
// selector names in anticipation of declaration of setter/getter methods.
PDecl->setGetterName(GetterSel, GetterNameLoc);
PDecl->setSetterName(SetterSel, SetterNameLoc);
PDecl->setPropertyAttributesAsWritten(
makePropertyAttributesAsWritten(AttributesAsWritten));

ProcessDeclAttributes(S, PDecl, FD.D);

if (Attributes & ObjCPropertyAttribute::kind_readonly)
PDecl->setPropertyAttributes(ObjCPropertyAttribute::kind_readonly);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add a test to ensure that the attributes are applied properly?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a test for this in this PR, properties.m – that's how I discovered this :)

Name: SomeBrokenLib
Functions:
- Name: do_something_with_pointers
Nu llabilityOfRet: O
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to comment that the space here is intentional? Particularly with this diff, it was difficult to verify until you get to the end.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, I added a comment

@@ -0,0 +1 @@
extern int FrameworkWithActualPrivateModule;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing newline

@@ -0,0 +1,2 @@
#include <FrameworkWithActualPrivateModule/FrameworkWithActualPrivateModule.h>
extern int FrameworkWithActualPrivateModule_Private;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing newline

// Build and check the unversioned module file.
// RUN: %clang_cc1 -fmodules -fblocks -fimplicit-module-maps -fmodules-cache-path=%t/ModulesCache/Unversioned -fdisable-module-hash -fapinotes-modules -fsyntax-only -I %S/Inputs/Headers -F %S/Inputs/Frameworks %s
// RUN: %clang_cc1 -ast-print %t/ModulesCache/Unversioned/VersionedKit.pcm | FileCheck -check-prefix=CHECK-UNVERSIONED %s
// RUN: %clang_cc1 -fmodules -fblocks -fimplicit-module-maps -fmodules-cache-path=%t/ModulesCache/Unversioned -fdisable-module-hash -fapinotes-modules -fsyntax-only -I %S/Inputs/Headers -F %S/Inputs/Frameworks %s -ast-dump -ast-dump-filter 'DUMP' | FileCheck -check-prefix=CHECK-DUMP -check-prefix=CHECK-UNVERSIONED-DUMP %s
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise

// RUN: %clang_cc1 -fmodules -fblocks -fimplicit-module-maps -fmodules-cache-path=%t/ModulesCache/Unversioned -fdisable-module-hash -fapinotes-modules -fsyntax-only -I %S/Inputs/Headers -F %S/Inputs/Frameworks %s -ast-dump -ast-dump-filter 'DUMP' | FileCheck -check-prefix=CHECK-DUMP -check-prefix=CHECK-UNVERSIONED-DUMP %s

// Build and check the versioned module file.
// RUN: %clang_cc1 -fmodules -fblocks -fimplicit-module-maps -fmodules-cache-path=%t/ModulesCache/Versioned -fdisable-module-hash -fapinotes-modules -fapinotes-swift-version=3 -fsyntax-only -I %S/Inputs/Headers -F %S/Inputs/Frameworks %s
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here too

// Build and check the versioned module file.
// RUN: %clang_cc1 -fmodules -fblocks -fimplicit-module-maps -fmodules-cache-path=%t/ModulesCache/Versioned -fdisable-module-hash -fapinotes-modules -fapinotes-swift-version=3 -fsyntax-only -I %S/Inputs/Headers -F %S/Inputs/Frameworks %s
// RUN: %clang_cc1 -ast-print %t/ModulesCache/Versioned/VersionedKit.pcm | FileCheck -check-prefix=CHECK-VERSIONED %s
// RUN: %clang_cc1 -fmodules -fblocks -fimplicit-module-maps -fmodules-cache-path=%t/ModulesCache/Versioned -fdisable-module-hash -fapinotes-modules -fapinotes-swift-version=3 -fsyntax-only -I %S/Inputs/Headers -F %S/Inputs/Frameworks %s -ast-dump -ast-dump-filter 'DUMP' | FileCheck -check-prefix=CHECK-DUMP -check-prefix=CHECK-VERSIONED-DUMP %s
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And here

@@ -0,0 +1,6 @@
// RUN: rm -rf %t
// RUN: not %clang_cc1 -fsyntax-only -fapinotes %s -I %S/Inputs/BrokenHeaders2 2>&1 | FileCheck %s
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra space after -fapinotes

@@ -0,0 +1,6 @@
// RUN: rm -rf %t
// RUN: %clang_cc1 -fsyntax-only -fapinotes %s -I %S/Inputs/BrokenHeaders -verify
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra space after -fapinotes

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
@egorzhdan
Copy link
Contributor Author

@compnerd I think I addressed all of your comments, let me know if this is good to go!

@egorzhdan
Copy link
Contributor Author

I'm going to land this patch now to unblock other work on APINotes, but I'm happy to address more review feedback post-merge.

@egorzhdan egorzhdan merged commit 932949d into llvm:main Mar 27, 2024
4 checks passed
@egorzhdan egorzhdan deleted the upstream-apinotes-tests branch March 27, 2024 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants