[clang][ASTImporter] Add import of thread safety attributes.

[balazske]

Summary

Attributes of "C/C++ Thread safety attributes" section in Attr.td
are added to ASTImporter. The not added attributes from this section
do not need special import handling.

[steakhal]

I appreciate the effort in working on the attribute imports.

Personally, I don't really like this change. IMO AttrImporter::createImportedAttr() should do the magic, and apply the necessary glue for the import.
Or we could even create a dedicated function, which would 'just do the right thing' and import the Attr, without the current noise of calling the individual imports than gluing the stuff to the constructor dispatch.

The proposed implementation involves too much code repetition for me.

[balazske]

The code can be made better in a way that only lines similar to

Expected<Attr *> ToAttrOrErr = AI.createImportedAttr(
    From, AI.importArg(From->getSuccessValue()).value(),
    AI.importArrayArg(From->args(), From->args_size()).value(),
    From->args_size());

are needed. If there is an AttrVisitor the switch can be removed. No one import function is possible that works with every Attr, in the same way as there is a separate function for import of every Stmt (Decl and others are more complicated) there must be something for every Attr (if it has custom arguments). Even now the import of an Attr needs much less code than import of a Stmt (that could be done in similar way). There are cases that need special handling, like AlignedAttr. If the current code is included we can see enough cases to make a better generalization, but after splitting the ASTImporter.cpp into multiple files.

[martong]

I agree with @steakhal , that perhaps we could come up with a somewhat more compact and elegant implementation in the future. Nevertheless, I think this patch in its current form is already valuable and a nice work, it does what it needs to do.

[balazske]

If the ASTImporter and parts of ASTNodeImporter are made public API then it can be possible to add generation of import code to the .td files, this should be the best solution. In similar way, import of Stmt and Expr nodes can be generated if possible. These imports are mechanical tasks that can be automated, probably with some exceptions.

[martong]

In D110528#3026921, @balazske wrote:

If the ASTImporter and parts of ASTNodeImporter are made public API then it can be possible to add generation of import code to the .td files, this should be the best solution. In similar way, import of Stmt and Expr nodes can be generated if possible. These imports are mechanical tasks that can be automated, probably with some exceptions.

Okay, let's keep that to a follow-up patch.

clang/lib/AST/ASTImporter.cpp

Expected<Attr *> ToAttrOrErr = AI.createImportedAttr(
         From, AI.importArrayArg(From->args(), From->args_size()).value(),
         From->args_size());
     if (ToAttrOrErr)
       ToAttr = *ToAttrOrErr;
     else
       return ToAttrOrErr.takeError();
     break;
   }
+  case attr::AcquireCapability: {
+    const auto *From = cast<AcquireCapabilityAttr>(FromAttr);
+    AttrImporter AI(*this);
+    Expected<Attr *> ToAttrOrErr = AI.createImportedAttr(
+        From, AI.importArrayArg(From->args(), From->args_size()).value(),
+        From->args_size());
+    if (ToAttrOrErr)
+      ToAttr = *ToAttrOrErr;
+    else
+      return ToAttrOrErr.takeError();
+    break;
+  }
+  case attr::TryAcquireCapability: {
+    const auto *From = cast<TryAcquireCapabilityAttr>(FromAttr);
+    AttrImporter AI(*this);
+    Expected<Attr *> ToAttrOrErr = AI.createImportedAttr(
+        From, AI.importArg(From->getSuccessValue()).value(),
+        AI.importArrayArg(From->args(), From->args_size()).value(),
+        From->args_size());
+    if (ToAttrOrErr)
+      ToAttr = *ToAttrOrErr;
+    else
+      return ToAttrOrErr.takeError();
+    break;
+  }
+  case attr::ReleaseCapability: {
+    const auto *From = cast<ReleaseCapabilityAttr>(FromAttr);
+    AttrImporter AI(*this);
+    Expected<Attr *> ToAttrOrErr = AI.createImportedAttr(
+        From, AI.importArrayArg(From->args(), From->args_size()).value(),
+        From->args_size());
+    if (ToAttrOrErr)
+      ToAttr = *ToAttrOrErr;
+    else
+      return ToAttrOrErr.takeError();
+    break;
+  }
+  case attr::RequiresCapability: {
+    const auto *From = cast<RequiresCapabilityAttr>(FromAttr);
+    AttrImporter AI(*this);
+    Expected<Attr *> ToAttrOrErr = AI.createImportedAttr(
+        From, AI.importArrayArg(From->args(), From->args_size()).value(),
+        From->args_size());
+    if (ToAttrOrErr)
+      ToAttr = *ToAttrOrErr;
+    else
+      return ToAttrOrErr.takeError();
+    break;
+  }
+  case attr::GuardedBy: {
+    const auto *From = cast<GuardedByAttr>(FromAttr);
+    AttrImporter AI(*this);
+    Expected<Attr *> ToAttrOrErr =
+        AI.createImportedAttr(From, AI.importArg(From->getArg()).value());
+    if (ToAttrOrErr)
+      ToAttr = *ToAttrOrErr;
+    else
+      return ToAttrOrErr.takeError();
+    break;
+  }
+  case attr::PtGuardedBy: {
+    const auto *From = cast<PtGuardedByAttr>(FromAttr);
+    AttrImporter AI(*this);
+    Expected<Attr *> ToAttrOrErr =
+        AI.createImportedAttr(From, AI.importArg(From->getArg()).value());
+    if (ToAttrOrErr)
+      ToAttr = *ToAttrOrErr;
+    else
+      return ToAttrOrErr.takeError();
+    break;
+  }
+  case attr::AcquiredAfter: {
+    const auto *From = cast<AcquiredAfterAttr>(FromAttr);
+    AttrImporter AI(*this);
+    Expected<Attr *> ToAttrOrErr = AI.createImportedAttr(
+        From, AI.importArrayArg(From->args(), From->args_size()).value(),
+        From->args_size());
+    if (ToAttrOrErr)
+      ToAttr = *ToAttrOrErr;
+    else
+      return ToAttrOrErr.takeError();
+    break;
+  }
+  case attr::AcquiredBefore: {
+    const auto *From = cast<AcquiredBeforeAttr>(FromAttr);
+    AttrImporter AI(*this);
+    Expected<Attr *> ToAttrOrErr = AI.createImportedAttr(
+        From, AI.importArrayArg(From->args(), From->args_size()).value(),
+        From->args_size());
+    if (ToAttrOrErr)
+      ToAttr = *ToAttrOrErr;
+    else
+      return ToAttrOrErr.takeError();
+    break;
+  }
+  case attr::AssertExclusiveLock: {
+    const auto *From = cast<AssertExclusiveLockAttr>(FromAttr);
+    AttrImporter AI(*this);
+    Expected<Attr *> ToAttrOrErr = AI.createImportedAttr(
+        From, AI.importArrayArg(From->args(), From->args_size()).value(),
+        From->args_size());
+    if (ToAttrOrErr)
+      ToAttr = *ToAttrOrErr;
+    else
+      return ToAttrOrErr.takeError();
+    break;
+  }
+  case attr::AssertSharedLock: {
+    const auto *From = cast<AssertSharedLockAttr>(FromAttr);
+    AttrImporter AI(*this);
+    Expected<Attr *> ToAttrOrErr = AI.createImportedAttr(
+        From, AI.importArrayArg(From->args(), From->args_size()).value(),
+        From->args_size());
+    if (ToAttrOrErr)
+      ToAttr = *ToAttrOrErr;
+    else
+      return ToAttrOrErr.takeError();
+    break;
+  }
+  case attr::ExclusiveTrylockFunction: {
+    const auto *From = cast<ExclusiveTrylockFunctionAttr>(FromAttr);
+    AttrImporter AI(*this);
+    Expected<Attr *> ToAttrOrErr = AI.createImportedAttr(
+        From, AI.importArg(From->getSuccessValue()).value(),
+        AI.importArrayArg(From->args(), From->args_size()).value(),
+        From->args_size());
+    if (ToAttrOrErr)
+      ToAttr = *ToAttrOrErr;
+    else
+      return ToAttrOrErr.takeError();
+    break;
+  }
+  case attr::SharedTrylockFunction: {
+    const auto *From = cast<SharedTrylockFunctionAttr>(FromAttr);
+    AttrImporter AI(*this);
+    Expected<Attr *> ToAttrOrErr = AI.createImportedAttr(
+        From, AI.importArg(From->getSuccessValue()).value(),
+        AI.importArrayArg(From->args(), From->args_size()).value(),
+        From->args_size());
+    if (ToAttrOrErr)
+      ToAttr = *ToAttrOrErr;
+    else
+      return ToAttrOrErr.takeError();
+    break;
+  }
+  case attr::LockReturned: {
+    const auto *From = cast<LockReturnedAttr>(FromAttr);
+    AttrImporter AI(*this);
+    Expected<Attr *> ToAttrOrErr =
+        AI.createImportedAttr(From, AI.importArg(From->getArg()).value());
+    if (ToAttrOrErr)
+      ToAttr = *ToAttrOrErr;
+    else
+      return ToAttrOrErr.takeError();
+    break;
+  }
+  case attr::LocksExcluded: {
+    const auto *From = cast<LocksExcludedAttr>(FromAttr);
+    AttrImporter AI(*this);
+    Expected<Attr *> ToAttrOrErr = AI.createImportedAttr(
+        From, AI.importArrayArg(From->args(), From->args_size()).value(),
+        From->args_size());
+    if (ToAttrOrErr)
+      ToAttr = *ToAttrOrErr;
+    else
+      return ToAttrOrErr.takeError();
+    break;
+  }
 
   default:
     // FIXME: 'clone' copies every member but some of them should be imported.
     // Handle other Attrs that have parameters that should be imported.
     ToAttr = FromAttr->clone(ToContext);
     ToAttr->setRange(ToRange);
     break;
   }

clang/unittests/AST/ASTImporterTest.cpp

TEST_P(ASTImporterOptionSpecificTestBase, ImportFormatAttr) {
   EXPECT_EQ(FromAttr->isPackExpansion(), ToAttr->isPackExpansion());
   EXPECT_EQ(FromAttr->isImplicit(), ToAttr->isImplicit());
   EXPECT_EQ(FromAttr->getSyntax(), ToAttr->getSyntax());
   EXPECT_EQ(FromAttr->getAttributeSpellingListIndex(),
             ToAttr->getAttributeSpellingListIndex());
   EXPECT_EQ(FromAttr->getType()->getName(), ToAttr->getType()->getName());
 }
 
+TEST_P(ImportAttributes, ImportGuardedVar) {
+  GuardedVarAttr *FromAttr, *ToAttr;
+  importAttr<VarDecl>("int test __attribute__((guarded_var));", FromAttr,
+                      ToAttr);
+}
+
+TEST_P(ImportAttributes, ImportPtGuardedVar) {
+  PtGuardedVarAttr *FromAttr, *ToAttr;
+  importAttr<VarDecl>("int *test __attribute__((pt_guarded_var));", FromAttr,
+                      ToAttr);
+}
+
+TEST_P(ImportAttributes, ImportScopedLockable) {
+  ScopedLockableAttr *FromAttr, *ToAttr;
+  importAttr<CXXRecordDecl>("struct __attribute__((scoped_lockable)) test {};",
+                            FromAttr, ToAttr);
+}
+
+TEST_P(ImportAttributes, ImportCapability) {
+  CapabilityAttr *FromAttr, *ToAttr;
+  importAttr<CXXRecordDecl>(
+      "struct __attribute__((capability(\"cap\"))) test {};", FromAttr, ToAttr);
+  EXPECT_EQ(FromAttr->getName(), ToAttr->getName());
+}
+
 TEST_P(ImportAttributes, ImportAssertCapability) {
   AssertCapabilityAttr *FromAttr, *ToAttr;
   importAttr<FunctionDecl>(
       "void test(int A1, int A2) __attribute__((assert_capability(A1, A2)));",
       FromAttr, ToAttr);
   checkImportVariadicArg(FromAttr->args(), ToAttr->args());
 }
 
+TEST_P(ImportAttributes, ImportAcquireCapability) {
+  AcquireCapabilityAttr *FromAttr, *ToAttr;
+  importAttr<FunctionDecl>(
+      "void test(int A1, int A2) __attribute__((acquire_capability(A1, A2)));",
+      FromAttr, ToAttr);
+  checkImportVariadicArg(FromAttr->args(), ToAttr->args());
+}
+
+TEST_P(ImportAttributes, ImportTryAcquireCapability) {
+  TryAcquireCapabilityAttr *FromAttr, *ToAttr;
+  importAttr<FunctionDecl>(
+      "void test(int A1, int A2) __attribute__((try_acquire_capability(1, A1, "
+      "A2)));",
+      FromAttr, ToAttr);
+  checkImported(FromAttr->getSuccessValue(), ToAttr->getSuccessValue());
+  checkImportVariadicArg(FromAttr->args(), ToAttr->args());
+}
+
+TEST_P(ImportAttributes, ImportReleaseCapability) {
+  ReleaseCapabilityAttr *FromAttr, *ToAttr;
+  importAttr<FunctionDecl>(
+      "void test(int A1, int A2) __attribute__((release_capability(A1, A2)));",
+      FromAttr, ToAttr);
+  checkImportVariadicArg(FromAttr->args(), ToAttr->args());
+}
+
+TEST_P(ImportAttributes, ImportRequiresCapability) {
+  RequiresCapabilityAttr *FromAttr, *ToAttr;
+  importAttr<FunctionDecl>(
+      "void test(int A1, int A2) __attribute__((requires_capability(A1, A2)));",
+      FromAttr, ToAttr);
+  checkImportVariadicArg(FromAttr->args(), ToAttr->args());
+}
+
+TEST_P(ImportAttributes, ImportNoThreadSafetyAnalysis) {
+  NoThreadSafetyAnalysisAttr *FromAttr, *ToAttr;
+  importAttr<FunctionDecl>(
+      "void test() __attribute__((no_thread_safety_analysis));", FromAttr,
+      ToAttr);
+}
+
+TEST_P(ImportAttributes, ImportGuardedBy) {
+  GuardedByAttr *FromAttr, *ToAttr;
+  importAttr<VarDecl>(
+      R"(
+      int G;
+      int test __attribute__((guarded_by(G)));
+      )",
+      FromAttr, ToAttr);
+  checkImported(FromAttr->getArg(), ToAttr->getArg());
+}
+
+TEST_P(ImportAttributes, ImportPtGuardedBy) {
+  PtGuardedByAttr *FromAttr, *ToAttr;
+  importAttr<VarDecl>(
+      R"(
+      int G;
+      int *test __attribute__((pt_guarded_by(G)));
+      )",
+      FromAttr, ToAttr);
+  checkImported(FromAttr->getArg(), ToAttr->getArg());
+}
+
+TEST_P(ImportAttributes, ImportAcquiredAfter) {
+  AcquiredAfterAttr *FromAttr, *ToAttr;
+  importAttr<VarDecl>(
+      R"(
+      struct __attribute__((lockable)) L {};
+      L A1;
+      L A2;
+      L test __attribute__((acquired_after(A1, A2)));
+      )",
+      FromAttr, ToAttr);
+  checkImportVariadicArg(FromAttr->args(), ToAttr->args());
+}
+
+TEST_P(ImportAttributes, ImportAcquiredBefore) {
+  AcquiredBeforeAttr *FromAttr, *ToAttr;
+  importAttr<VarDecl>(
+      R"(
+      struct __attribute__((lockable)) L {};
+      L A1;
+      L A2;
+      L test __attribute__((acquired_before(A1, A2)));
+      )",
+      FromAttr, ToAttr);
+  checkImportVariadicArg(FromAttr->args(), ToAttr->args());
+}
+
+TEST_P(ImportAttributes, ImportAssertExclusiveLock) {
+  AssertExclusiveLockAttr *FromAttr, *ToAttr;
+  importAttr<FunctionDecl>("void test(int A1, int A2) "
+                           "__attribute__((assert_exclusive_lock(A1, A2)));",
+                           FromAttr, ToAttr);
+  checkImportVariadicArg(FromAttr->args(), ToAttr->args());
+}
+
+TEST_P(ImportAttributes, ImportAssertSharedLock) {
+  AssertSharedLockAttr *FromAttr, *ToAttr;
+  importAttr<FunctionDecl>(
+      "void test(int A1, int A2) __attribute__((assert_shared_lock(A1, A2)));",
+      FromAttr, ToAttr);
+  checkImportVariadicArg(FromAttr->args(), ToAttr->args());
+}
+
+TEST_P(ImportAttributes, ImportExclusiveTrylockFunction) {
+  ExclusiveTrylockFunctionAttr *FromAttr, *ToAttr;
+  importAttr<FunctionDecl>(
+      "void test(int A1, int A2) __attribute__((exclusive_trylock_function(1, "
+      "A1, A2)));",
+      FromAttr, ToAttr);
+  checkImported(FromAttr->getSuccessValue(), ToAttr->getSuccessValue());
+  checkImportVariadicArg(FromAttr->args(), ToAttr->args());
+}
+
+TEST_P(ImportAttributes, ImportSharedTrylockFunction) {
+  SharedTrylockFunctionAttr *FromAttr, *ToAttr;
+  importAttr<FunctionDecl>(
+      "void test(int A1, int A2) __attribute__((shared_trylock_function(1, A1, "
+      "A2)));",
+      FromAttr, ToAttr);
+  checkImported(FromAttr->getSuccessValue(), ToAttr->getSuccessValue());
+  checkImportVariadicArg(FromAttr->args(), ToAttr->args());
+}
+
+TEST_P(ImportAttributes, ImportLockReturned) {
+  LockReturnedAttr *FromAttr, *ToAttr;
+  importAttr<FunctionDecl>(
+      "void test(int A1) __attribute__((lock_returned(A1)));", FromAttr,
+      ToAttr);
+  checkImported(FromAttr->getArg(), ToAttr->getArg());
+}
+
+TEST_P(ImportAttributes, ImportLocksExcluded) {
+  LocksExcludedAttr *FromAttr, *ToAttr;
+  importAttr<FunctionDecl>(
+      "void test(int A1, int A2) __attribute__((locks_excluded(A1, A2)));",
+      FromAttr, ToAttr);
+  checkImportVariadicArg(FromAttr->args(), ToAttr->args());
+}
+
 template <typename T>
 auto ExtendWithOptions(const T &Values, const std::vector<std::string> &Args) {
   auto Copy = Values;
   for (std::vector<std::string> &ArgV : Copy) {
     for (const std::string &Arg : Args) {
       ArgV.push_back(Arg);
     }
   }