Skip to content

[InstallAPI] Collect global functions #83952

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

Merged
merged 2 commits into from
Mar 7, 2024
Merged

Conversation

cyndyishida
Copy link
Member

  • Include whether functions are inlinable as they impact whether to add them into the tbd file and for future verification.
  • Fix how clang arguments got passed along, previously spacing was passed along to CC1 causing search path inputs to look non-existent.

@cyndyishida cyndyishida requested review from zixu-w and ributzka March 5, 2024 03:22
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Mar 5, 2024
@llvmbot
Copy link
Member

llvmbot commented Mar 5, 2024

@llvm/pr-subscribers-clang

Author: Cyndy Ishida (cyndyishida)

Changes
  • Include whether functions are inlinable as they impact whether to add them into the tbd file and for future verification.
  • Fix how clang arguments got passed along, previously spacing was passed along to CC1 causing search path inputs to look non-existent.

Full diff: https://github.com/llvm/llvm-project/pull/83952.diff

10 Files Affected:

  • (modified) clang/include/clang/InstallAPI/Frontend.h (+4-1)
  • (modified) clang/include/clang/InstallAPI/Visitor.h (+3)
  • (modified) clang/lib/InstallAPI/Frontend.cpp (+1-1)
  • (modified) clang/lib/InstallAPI/Visitor.cpp (+78)
  • (added) clang/test/InstallAPI/functions.test (+78)
  • (modified) clang/tools/clang-installapi/Options.cpp (+3-1)
  • (modified) llvm/include/llvm/TextAPI/Record.h (+4-2)
  • (modified) llvm/include/llvm/TextAPI/RecordsSlice.h (+4-1)
  • (modified) llvm/lib/TextAPI/RecordsSlice.cpp (+3-3)
  • (modified) llvm/unittests/TextAPI/RecordTests.cpp (+1)
diff --git a/clang/include/clang/InstallAPI/Frontend.h b/clang/include/clang/InstallAPI/Frontend.h
index 8774321e990c13..cbc2b159ebd17a 100644
--- a/clang/include/clang/InstallAPI/Frontend.h
+++ b/clang/include/clang/InstallAPI/Frontend.h
@@ -50,12 +50,15 @@ class FrontendRecordsSlice : public llvm::MachO::RecordsSlice {
   /// \param D The pointer to the declaration from traversing AST.
   /// \param Access The intended access level of symbol.
   /// \param Flags The flags that describe attributes of the symbol.
+  /// \param Inlined Whether declaration is inlined, only applicable to
+  /// functions.
   /// \return The non-owning pointer to added record in slice.
   GlobalRecord *addGlobal(StringRef Name, RecordLinkage Linkage,
                           GlobalRecord::Kind GV,
                           const clang::AvailabilityInfo Avail, const Decl *D,
                           const HeaderType Access,
-                          SymbolFlags Flags = SymbolFlags::None);
+                          SymbolFlags Flags = SymbolFlags::None,
+                          bool Inlined = false);
 
   /// Add ObjC Class record with attributes from AST.
   ///
diff --git a/clang/include/clang/InstallAPI/Visitor.h b/clang/include/clang/InstallAPI/Visitor.h
index ff0a9957aa86bc..71d4d9894f4205 100644
--- a/clang/include/clang/InstallAPI/Visitor.h
+++ b/clang/include/clang/InstallAPI/Visitor.h
@@ -37,6 +37,9 @@ class InstallAPIVisitor final : public ASTConsumer,
   /// Collect global variables.
   bool VisitVarDecl(const VarDecl *D);
 
+  /// Collect global functions.
+  bool VisitFunctionDecl(const FunctionDecl *D);
+
   /// Collect Objective-C Interface declarations.
   /// Every Objective-C class has an interface declaration that lists all the
   /// ivars, properties, and methods of the class.
diff --git a/clang/lib/InstallAPI/Frontend.cpp b/clang/lib/InstallAPI/Frontend.cpp
index 240a80e1d3d82c..efc634d80dd218 100644
--- a/clang/lib/InstallAPI/Frontend.cpp
+++ b/clang/lib/InstallAPI/Frontend.cpp
@@ -19,7 +19,7 @@ namespace clang::installapi {
 GlobalRecord *FrontendRecordsSlice::addGlobal(
     StringRef Name, RecordLinkage Linkage, GlobalRecord::Kind GV,
     const clang::AvailabilityInfo Avail, const Decl *D, const HeaderType Access,
-    SymbolFlags Flags) {
+    SymbolFlags Flags, bool Inlined) {
 
   auto *GR = llvm::MachO::RecordsSlice::addGlobal(Name, Linkage, GV, Flags);
   FrontendRecords.insert({GR, FrontendAttrs{Avail, D, Access}});
diff --git a/clang/lib/InstallAPI/Visitor.cpp b/clang/lib/InstallAPI/Visitor.cpp
index fbe6f1dabe005d..1f2ef08e5aa252 100644
--- a/clang/lib/InstallAPI/Visitor.cpp
+++ b/clang/lib/InstallAPI/Visitor.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "clang/InstallAPI/Visitor.h"
+#include "clang/AST/ParentMapContext.h"
 #include "clang/Basic/Linkage.h"
 #include "clang/InstallAPI/Frontend.h"
 #include "llvm/ADT/SmallString.h"
@@ -27,6 +28,31 @@ static bool isExported(const NamedDecl *D) {
          (LV.getVisibility() == DefaultVisibility);
 }
 
+static bool isInlined(const FunctionDecl *D) {
+  bool HasInlineAttribute = false;
+  bool NoCXXAttr =
+      (!D->getASTContext().getLangOpts().CPlusPlus &&
+       !D->getASTContext().getTargetInfo().getCXXABI().isMicrosoft() &&
+       !D->hasAttr<DLLExportAttr>());
+
+  // Check all redeclarations to find an inline attribute or keyword.
+  for (const auto *RD : D->redecls()) {
+    if (!RD->isInlined())
+      continue;
+    HasInlineAttribute = true;
+    if (!(NoCXXAttr || RD->hasAttr<GNUInlineAttr>()))
+      continue;
+    if (RD->doesThisDeclarationHaveABody() &&
+        RD->isInlineDefinitionExternallyVisible())
+      return false;
+  }
+
+  if (!HasInlineAttribute)
+    return false;
+
+  return true;
+}
+
 static SymbolFlags getFlags(bool WeakDef, bool ThreadLocal) {
   SymbolFlags Result = SymbolFlags::None;
   if (WeakDef)
@@ -204,4 +230,56 @@ bool InstallAPIVisitor::VisitVarDecl(const VarDecl *D) {
   return true;
 }
 
+bool InstallAPIVisitor::VisitFunctionDecl(const FunctionDecl *D) {
+  if (const CXXMethodDecl *M = dyn_cast<CXXMethodDecl>(D)) {
+    // Skip member function in class templates.
+    if (M->getParent()->getDescribedClassTemplate() != nullptr)
+      return true;
+
+    // Skip methods in CXX RecordDecls.
+    for (auto P : D->getASTContext().getParents(*M)) {
+      if (P.get<CXXRecordDecl>())
+        return true;
+    }
+
+    // Skip CXX ConstructorDecls and DestructorDecls.
+    if (isa<CXXConstructorDecl>(M) || isa<CXXDestructorDecl>(M))
+      return true;
+  }
+
+  // Skip templated functions.
+  switch (D->getTemplatedKind()) {
+  case FunctionDecl::TK_NonTemplate:
+  case FunctionDecl::TK_DependentNonTemplate:
+    break;
+  case FunctionDecl::TK_MemberSpecialization:
+  case FunctionDecl::TK_FunctionTemplateSpecialization:
+    if (auto *TempInfo = D->getTemplateSpecializationInfo()) {
+      if (!TempInfo->isExplicitInstantiationOrSpecialization())
+        return true;
+    }
+    break;
+  case FunctionDecl::TK_FunctionTemplate:
+  case FunctionDecl::TK_DependentFunctionTemplateSpecialization:
+    return true;
+  }
+
+  auto Access = getAccessForDecl(D);
+  if (!Access)
+    return true;
+  auto Name = getMangledName(D);
+  const AvailabilityInfo Avail = AvailabilityInfo::createFromDecl(D);
+  const bool ExplicitInstantiation = D->getTemplateSpecializationKind() ==
+                                     TSK_ExplicitInstantiationDeclaration;
+  const bool WeakDef = ExplicitInstantiation || D->hasAttr<WeakAttr>();
+  const bool Inlined = isInlined(D);
+  const RecordLinkage Linkage = (Inlined || !isExported(D))
+                                    ? RecordLinkage::Internal
+                                    : RecordLinkage::Exported;
+  Ctx.Slice->addGlobal(Name, Linkage, GlobalRecord::Kind::Function, Avail, D,
+                       *Access, getFlags(WeakDef, /*ThreadLocal=*/false),
+                       Inlined);
+  return true;
+}
+
 } // namespace clang::installapi
diff --git a/clang/test/InstallAPI/functions.test b/clang/test/InstallAPI/functions.test
new file mode 100644
index 00000000000000..527965303cb351
--- /dev/null
+++ b/clang/test/InstallAPI/functions.test
@@ -0,0 +1,78 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s|DSTROOT|%/t|g" %t/inputs.json.in > %t/inputs.json
+
+// RUN: clang-installapi -target arm64-apple-macos13.1 \
+// RUN: -I%t/usr/include -I%t/usr/local/include \
+// RUN: -install_name @rpath/lib/libfunctions.dylib \
+// RUN: %t/inputs.json -o %t/outputs.tbd 2>&1 | FileCheck %s --allow-empty
+// RUN: llvm-readtapi -compare %t/outputs.tbd %t/expected.tbd 2>&1 | FileCheck %s --allow-empty
+
+// CHECK-NOT: error: 
+// CHECK-NOT: warning: 
+
+//--- usr/include/functions.h
+inline int inlined_func(void) { return 1;}
+int public(int a);
+
+//--- usr/local/include/private_functions.h
+__attribute__((visibility("hidden")))
+void hidden(void);
+
+//--- inputs.json.in
+{
+  "headers": [ {
+    "path" : "DSTROOT/usr/include/functions.h",
+    "type" : "public"
+  }, 
+  {
+    "path" : "DSTROOT/usr/local/include/private_functions.h",
+    "type" : "private"
+  }
+  ],
+  "version": "3"
+}
+
+//--- expected.tbd
+{
+  "main_library": {
+    "compatibility_versions": [
+      {
+        "version": "0"
+      }
+    ],
+    "current_versions": [
+      {
+        "version": "0"
+      }
+    ],
+    "exported_symbols": [
+      {
+        "text": {
+          "global": [
+            "_public"
+          ]
+        }
+      }
+    ],
+    "flags": [
+      {
+        "attributes": [
+          "not_app_extension_safe"
+        ]
+      }
+    ],
+    "install_names": [
+      {
+        "name": "@rpath/lib/libfunctions.dylib"
+      }
+    ],
+    "target_info": [
+      {
+        "min_deployment": "13.1",
+        "target": "arm64-macos"
+      }
+    ]
+  },
+  "tapi_tbd_version": 5
+}
diff --git a/clang/tools/clang-installapi/Options.cpp b/clang/tools/clang-installapi/Options.cpp
index 7d45e999448d9f..b9c36eab2ad3b7 100644
--- a/clang/tools/clang-installapi/Options.cpp
+++ b/clang/tools/clang-installapi/Options.cpp
@@ -112,7 +112,9 @@ Options::Options(DiagnosticsEngine &Diag, FileManager *FM,
   for (const Arg *A : ArgList) {
     if (A->isClaimed())
       continue;
-    FrontendArgs.emplace_back(A->getAsString(ArgList));
+
+    FrontendArgs.emplace_back(A->getSpelling());
+    llvm::copy(A->getValues(), std::back_inserter(FrontendArgs));
   }
   FrontendArgs.push_back("-fsyntax-only");
 }
diff --git a/llvm/include/llvm/TextAPI/Record.h b/llvm/include/llvm/TextAPI/Record.h
index 867d6a23588326..98639b064eaadd 100644
--- a/llvm/include/llvm/TextAPI/Record.h
+++ b/llvm/include/llvm/TextAPI/Record.h
@@ -103,8 +103,8 @@ class GlobalRecord : public Record {
   };
 
   GlobalRecord(StringRef Name, RecordLinkage Linkage, SymbolFlags Flags,
-               Kind GV)
-      : Record({Name, Linkage, Flags}), GV(GV) {}
+               Kind GV, bool Inlined)
+      : Record({Name, Linkage, Flags}), GV(GV), Inlined(Inlined) {}
 
   bool isFunction() const { return GV == Kind::Function; }
   bool isVariable() const { return GV == Kind::Variable; }
@@ -112,9 +112,11 @@ class GlobalRecord : public Record {
     if (GV == Kind::Unknown)
       GV = V;
   }
+  bool isInlined() const { return Inlined; }
 
 private:
   Kind GV;
+  bool Inlined = false;
 };
 
 // Define Objective-C instance variable records.
diff --git a/llvm/include/llvm/TextAPI/RecordsSlice.h b/llvm/include/llvm/TextAPI/RecordsSlice.h
index 57b23e5ea29e71..f934cf7607f1fd 100644
--- a/llvm/include/llvm/TextAPI/RecordsSlice.h
+++ b/llvm/include/llvm/TextAPI/RecordsSlice.h
@@ -53,10 +53,13 @@ class RecordsSlice {
   /// \param Linkage The linkage of symbol.
   /// \param GV The kind of global.
   /// \param Flags The flags that describe attributes of the symbol.
+  /// \param Inlined Whether declaration is inlined, only applicable to
+  /// functions.
   /// \return The non-owning pointer to added record in slice.
   GlobalRecord *addGlobal(StringRef Name, RecordLinkage Linkage,
                           GlobalRecord::Kind GV,
-                          SymbolFlags Flags = SymbolFlags::None);
+                          SymbolFlags Flags = SymbolFlags::None,
+                          bool Inlined = false);
 
   /// Add ObjC Class record.
   ///
diff --git a/llvm/lib/TextAPI/RecordsSlice.cpp b/llvm/lib/TextAPI/RecordsSlice.cpp
index db52a2cdd85c9c..111a1fa6eaf43b 100644
--- a/llvm/lib/TextAPI/RecordsSlice.cpp
+++ b/llvm/lib/TextAPI/RecordsSlice.cpp
@@ -171,8 +171,8 @@ ObjCIVarRecord *RecordsSlice::findObjCIVar(bool IsScopedName,
 }
 
 GlobalRecord *RecordsSlice::addGlobal(StringRef Name, RecordLinkage Linkage,
-                                      GlobalRecord::Kind GV,
-                                      SymbolFlags Flags) {
+                                      GlobalRecord::Kind GV, SymbolFlags Flags,
+                                      bool Inlined) {
   if (GV == GlobalRecord::Kind::Function)
     Flags |= SymbolFlags::Text;
   else if (GV == GlobalRecord::Kind::Variable)
@@ -182,7 +182,7 @@ GlobalRecord *RecordsSlice::addGlobal(StringRef Name, RecordLinkage Linkage,
   auto Result = Globals.insert({Name, nullptr});
   if (Result.second)
     Result.first->second =
-        std::make_unique<GlobalRecord>(Name, Linkage, Flags, GV);
+        std::make_unique<GlobalRecord>(Name, Linkage, Flags, GV, Inlined);
   else {
     updateLinkage(Result.first->second.get(), Linkage);
     updateFlags(Result.first->second.get(), Flags);
diff --git a/llvm/unittests/TextAPI/RecordTests.cpp b/llvm/unittests/TextAPI/RecordTests.cpp
index 37289eca1bdf6b..06bff2ccc48409 100644
--- a/llvm/unittests/TextAPI/RecordTests.cpp
+++ b/llvm/unittests/TextAPI/RecordTests.cpp
@@ -30,6 +30,7 @@ TEST(TAPIRecord, Simple) {
   EXPECT_FALSE(API.isWeakDefined());
   EXPECT_FALSE(API.isWeakReferenced());
   EXPECT_FALSE(API.isVariable());
+  EXPECT_FALSE(API.isInlined());
 }
 
 TEST(TAPIRecord, SimpleObjC) {

* Include whether functions are inlinable as they impact whether to add
  them into the tbd file and future verification.
* Fix how clang arguments got passed along, previously spacing was
  passed along to CC1 causing search paths to look non-existent.
@cyndyishida cyndyishida force-pushed the users/cyndyishida/installapip1 branch from be6d3d4 to 580af4d Compare March 5, 2024 15:47
@cyndyishida cyndyishida merged commit 50ae8a2 into main Mar 7, 2024
@cyndyishida cyndyishida deleted the users/cyndyishida/installapip1 branch March 7, 2024 23:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants