[PATCH] D59628: Add support for __attribute__((objc_class_stub))

[Slava Pestov via Phabricator via cfe-commits]

slavapestov updated this revision to Diff 202084.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59628/new/

https://reviews.llvm.org/D59628

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/ObjCRuntime.h
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/test/CodeGenObjC/class-stubs.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaObjC/class-stub-attr-unsupported.m
  clang/test/SemaObjC/class-stub-attr.m
  clang/utils/TableGen/ClangAttrEmitter.cpp

Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -1922,6 +1922,30 @@
   return true;
 }
 
+static std::string GenerateTestExpression(ArrayRef LangOpts) {
+  std::string Test;
+
+  for (auto *E : LangOpts) {
+if (!Test.empty())
+  Test += " || ";
+
+const StringRef Code = E->getValueAsString("CustomCode");
+if (!Code.empty()) {
+  Test += "(";
+  Test += Code;
+  Test += ")";
+} else {
+  Test += "LangOpts.";
+  Test += E->getValueAsString("Name");
+}
+  }
+
+  if (Test.empty())
+return "true";
+
+  return Test;
+}
+
 std::string
 PragmaClangAttributeSupport::generateStrictConformsTo(const Record &Attr,
   raw_ostream &OS) {
@@ -1948,19 +1972,8 @@
   // rules if the specific language options are specified.
   std::vector LangOpts = Rule.getLangOpts();
   OS << "  MatchRules.push_back(std::make_pair(" << Rule.getEnumValue()
- << ", /*IsSupported=*/";
-  if (!LangOpts.empty()) {
-for (auto I = LangOpts.begin(), E = LangOpts.end(); I != E; ++I) {
-  const StringRef Part = (*I)->getValueAsString("Name");
-  if ((*I)->getValueAsBit("Negated"))
-OS << "!";
-  OS << "LangOpts." << Part;
-  if (I + 1 != E)
-OS << " || ";
-}
-  } else
-OS << "true";
-  OS << "));\n";
+ << ", /*IsSupported=*/" << GenerateTestExpression(LangOpts)
+ << "));\n";
 }
   }
   OS << "}\n\n";
@@ -3431,23 +3444,12 @@
   if (LangOpts.empty())
 return "defaultDiagnoseLangOpts";
 
-  // Generate the test condition, as well as a unique function name for the
-  // diagnostic test. The list of options should usually be short (one or two
-  // options), and the uniqueness isn't strictly necessary (it is just for
-  // codegen efficiency).
-  std::string FnName = "check", Test;
-  for (auto I = LangOpts.begin(), E = LangOpts.end(); I != E; ++I) {
-const StringRef Part = (*I)->getValueAsString("Name");
-if ((*I)->getValueAsBit("Negated")) {
-  FnName += "Not";
-  Test += "!";
-}
-Test += "S.LangOpts.";
-Test +=  Part;
-if (I + 1 != E)
-  Test += " || ";
-FnName += Part;
-  }
+  // Generate a unique function name for the diagnostic test. The list of
+  // options should usually be short (one or two options), and the
+  // uniqueness isn't strictly necessary (it is just for codegen efficiency).
+  std::string FnName = "check";
+  for (auto I = LangOpts.begin(), E = LangOpts.end(); I != E; ++I)
+FnName += (*I)->getValueAsString("Name");
   FnName += "LangOpts";
 
   // If this code has already been generated, simply return the previous
@@ -3458,7 +3460,8 @@
 return *I;
 
   OS << "static bool " << FnName << "(Sema &S, const ParsedAttr &Attr) {\n";
-  OS << "  if (" << Test << ")\n";
+  OS << "  auto &LangOpts = S.LangOpts;\n";
+  OS << "  if (" << GenerateTestExpression(LangOpts) << ")\n";
   OS << "return true;\n\n";
   OS << "  S.Diag(Attr.getLoc(), diag::warn_attribute_ignored) ";
   OS << "<< Attr.getName();\n";
Index: clang/test/SemaObjC/class-stub-attr.m
===
--- /dev/null
+++ clang/test/SemaObjC/class-stub-attr.m
@@ -0,0 +1,27 @@
+// RUN: %clang -target x86_64-apple-darwin -fsyntax-only -Xclang -verify %s
+// RUN: %clang -target x86_64-apple-darwin -x objective-c++ -fsyntax-only -Xclang -verify %s
+
+@interface NSObject
+@end
+
+__attribute__((objc_class_stub))
+@interface MissingSubclassingRestrictedAttribute : NSObject // expected-error {{'objc_class_stub' attribute cannot be specified on a class that does not have the 'objc_subclassing_restricted' attribute}}
+@end
+
+__attribute__((objc_class_stub))
+__attribute__((objc_subclassing_restricted))
+@interface ValidClassStubAttribute : NSObject
+@end
+
+@implementation ValidClassStubAttribute // expected-error {{cannot declare implementation of a class declared with the 'objc_class_stub' attribute}}
+@end
+
+@implementation ValidClassStubAttribute (MyCategory)
+@end
+
+__attribute__((objc_class_stub(123))) // expected-error {{'objc_clas

[PATCH] D59628: Add support for __attribute__((objc_class_stub))

[Slava Pestov via Phabricator via cfe-commits]

slavapestov marked 2 inline comments as done.
slavapestov added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:297
 def CUDA : LangOpt<"CUDA">;
-def COnly : LangOpt<"CPlusPlus", 1>;
+def COnly : LangOpt<"COnly", "!LangOpts.CPlusPlus">;
 def CPlusPlus : LangOpt<"CPlusPlus">;

aaron.ballman wrote:
> This compiles? I would have assumed that it would have to be `def COnly : 
> LangOpt<"COnly", [{!LangOpts.CPlusPlus}]>;`
> 
> (If it compiles as-is, that's fine, I just wasn't aware that you could use 
> string syntax for code blocks.)
It appears that both work! Going with your suggested version.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59628/new/

https://reviews.llvm.org/D59628



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59628: Add support for __attribute__((objc_class_stub))

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM!




Comment at: clang/include/clang/Basic/Attr.td:297
 def CUDA : LangOpt<"CUDA">;
-def COnly : LangOpt<"CPlusPlus", 1>;
+def COnly : LangOpt<"COnly", "!LangOpts.CPlusPlus">;
 def CPlusPlus : LangOpt<"CPlusPlus">;

This compiles? I would have assumed that it would have to be `def COnly : 
LangOpt<"COnly", [{!LangOpts.CPlusPlus}]>;`

(If it compiles as-is, that's fine, I just wasn't aware that you could use 
string syntax for code blocks.)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59628/new/

https://reviews.llvm.org/D59628



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59628: Add support for __attribute__((objc_class_stub))

[John McCall via Phabricator via cfe-commits]

rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!  You should wait for Aaron's approval, too.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59628/new/

https://reviews.llvm.org/D59628



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59628: Add support for __attribute__((objc_class_stub))

[Slava Pestov via Phabricator via cfe-commits]

slavapestov updated this revision to Diff 200808.
slavapestov marked 6 inline comments as done.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59628/new/

https://reviews.llvm.org/D59628

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/ObjCRuntime.h
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/test/CodeGenObjC/class-stubs.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaObjC/class-stub-attr-unsupported.m
  clang/test/SemaObjC/class-stub-attr.m
  clang/utils/TableGen/ClangAttrEmitter.cpp

Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -1922,6 +1922,30 @@
   return true;
 }
 
+static std::string GenerateTestExpression(ArrayRef LangOpts) {
+  std::string Test;
+
+  for (auto *E : LangOpts) {
+if (!Test.empty())
+  Test += " || ";
+
+const StringRef Code = E->getValueAsString("CustomCode");
+if (!Code.empty()) {
+  Test += "(";
+  Test += Code;
+  Test += ")";
+} else {
+  Test += "LangOpts.";
+  Test += E->getValueAsString("Name");
+}
+  }
+
+  if (Test.empty())
+return "true";
+
+  return Test;
+}
+
 std::string
 PragmaClangAttributeSupport::generateStrictConformsTo(const Record &Attr,
   raw_ostream &OS) {
@@ -1948,19 +1972,8 @@
   // rules if the specific language options are specified.
   std::vector LangOpts = Rule.getLangOpts();
   OS << "  MatchRules.push_back(std::make_pair(" << Rule.getEnumValue()
- << ", /*IsSupported=*/";
-  if (!LangOpts.empty()) {
-for (auto I = LangOpts.begin(), E = LangOpts.end(); I != E; ++I) {
-  const StringRef Part = (*I)->getValueAsString("Name");
-  if ((*I)->getValueAsBit("Negated"))
-OS << "!";
-  OS << "LangOpts." << Part;
-  if (I + 1 != E)
-OS << " || ";
-}
-  } else
-OS << "true";
-  OS << "));\n";
+ << ", /*IsSupported=*/" << GenerateTestExpression(LangOpts)
+ << "));\n";
 }
   }
   OS << "}\n\n";
@@ -3431,23 +3444,12 @@
   if (LangOpts.empty())
 return "defaultDiagnoseLangOpts";
 
-  // Generate the test condition, as well as a unique function name for the
-  // diagnostic test. The list of options should usually be short (one or two
-  // options), and the uniqueness isn't strictly necessary (it is just for
-  // codegen efficiency).
-  std::string FnName = "check", Test;
-  for (auto I = LangOpts.begin(), E = LangOpts.end(); I != E; ++I) {
-const StringRef Part = (*I)->getValueAsString("Name");
-if ((*I)->getValueAsBit("Negated")) {
-  FnName += "Not";
-  Test += "!";
-}
-Test += "S.LangOpts.";
-Test +=  Part;
-if (I + 1 != E)
-  Test += " || ";
-FnName += Part;
-  }
+  // Generate a unique function name for the diagnostic test. The list of
+  // options should usually be short (one or two options), and the
+  // uniqueness isn't strictly necessary (it is just for codegen efficiency).
+  std::string FnName = "check";
+  for (auto I = LangOpts.begin(), E = LangOpts.end(); I != E; ++I)
+FnName += (*I)->getValueAsString("Name");
   FnName += "LangOpts";
 
   // If this code has already been generated, simply return the previous
@@ -3458,7 +3460,8 @@
 return *I;
 
   OS << "static bool " << FnName << "(Sema &S, const ParsedAttr &Attr) {\n";
-  OS << "  if (" << Test << ")\n";
+  OS << "  auto &LangOpts = S.LangOpts;\n";
+  OS << "  if (" << GenerateTestExpression(LangOpts) << ")\n";
   OS << "return true;\n\n";
   OS << "  S.Diag(Attr.getLoc(), diag::warn_attribute_ignored) ";
   OS << "<< Attr.getName();\n";
Index: clang/test/SemaObjC/class-stub-attr.m
===
--- /dev/null
+++ clang/test/SemaObjC/class-stub-attr.m
@@ -0,0 +1,27 @@
+// RUN: %clang -target x86_64-apple-darwin -fsyntax-only -Xclang -verify %s
+// RUN: %clang -target x86_64-apple-darwin -x objective-c++ -fsyntax-only -Xclang -verify %s
+
+@interface NSObject
+@end
+
+__attribute__((objc_class_stub))
+@interface MissingSubclassingRestrictedAttribute : NSObject // expected-error {{'objc_class_stub' attribute cannot be specified on a class that does not have the 'objc_subclassing_restricted' attribute}}
+@end
+
+__attribute__((objc_class_stub))
+__attribute__((objc_subclassing_restricted))
+@interface ValidClassStubAttribute : NSObject
+@end
+
+@implementation ValidClassStubAttribute // expected-error {{cannot declare implementation of a class declared with the 'objc_class_stub' attribute}}
+@end
+
+@implementation ValidClassStubAttribute (MyCategory)
+@end
+
+__attribute__((objc_cl

[PATCH] D59628: Add support for __attribute__((objc_class_stub))

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:292
+  // "LangOpts" bound.
+  string CustomCode = customCode;
 }

If this is code, should it be using a `code` type rather than a `string` type?



Comment at: clang/include/clang/Basic/AttrDocs.td:1101
+implementations defined for them. This attribute is intended for use in
+Swift generated headers for classes defined in Swift.
+

Swift generated -> Swift-generated



Comment at: clang/lib/CodeGen/CGObjCMac.cpp:736
+llvm::Type *params[] = { Int8PtrPtrTy };
+auto F = CGM.CreateRuntimeFunction(
+llvm::FunctionType::get(ClassnfABIPtrTy, params, false),

Please spell the type out explicitly rather than use `auto` here.



Comment at: clang/lib/CodeGen/CGObjCMac.cpp:7325
NotForDefinition);
+  assert(ClassGV->getType() == ObjCTypes.ClassnfABIPtrTy);
 }

Can you add a message to this assertion so that when it triggers, users get 
something more helpful to report?



Comment at: clang/lib/CodeGen/CGObjCMac.cpp:7366
   if (!Entry) {
-auto ClassGV = GetClassGlobal(ID, /*metaclass*/ false, NotForDefinition);
+auto ClassGV = GetClassGlobalForClassRef(ID);
 std::string SectionName =

Can you spell out the type here, since you're changing the initialization 
already?



Comment at: clang/lib/CodeGen/CGObjCMac.cpp:7410-7411
   if (ID->isWeakImported()) {
-auto ClassGV = GetClassGlobal(ID, /*metaclass*/ false, NotForDefinition);
+llvm::Constant *ClassGV = GetClassGlobal(ID, /*metaclass*/ false,
+ NotForDefinition);
 (void)ClassGV;

I'm not opposed to this change, but it seems unrelated to the patch. Feel free 
to commit separately as an NFC.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59628/new/

https://reviews.llvm.org/D59628



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59628: Add support for __attribute__((objc_class_stub))

[Slava Pestov via Phabricator via cfe-commits]

slavapestov updated this revision to Diff 199905.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59628/new/

https://reviews.llvm.org/D59628

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/ObjCRuntime.h
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/test/CodeGenObjC/class-stubs.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaObjC/class-stub-attr-unsupported.m
  clang/test/SemaObjC/class-stub-attr.m
  clang/utils/TableGen/ClangAttrEmitter.cpp

Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -1922,6 +1922,30 @@
   return true;
 }
 
+static std::string GenerateTestExpression(ArrayRef LangOpts) {
+  std::string Test;
+
+  for (auto *E : LangOpts) {
+if (!Test.empty())
+  Test += " || ";
+
+const StringRef Code = E->getValueAsString("CustomCode");
+if (!Code.empty()) {
+  Test += "(";
+  Test += Code;
+  Test += ")";
+} else {
+  Test += "LangOpts.";
+  Test += E->getValueAsString("Name");
+}
+  }
+
+  if (Test.empty())
+return "true";
+
+  return Test;
+}
+
 std::string
 PragmaClangAttributeSupport::generateStrictConformsTo(const Record &Attr,
   raw_ostream &OS) {
@@ -1948,19 +1972,8 @@
   // rules if the specific language options are specified.
   std::vector LangOpts = Rule.getLangOpts();
   OS << "  MatchRules.push_back(std::make_pair(" << Rule.getEnumValue()
- << ", /*IsSupported=*/";
-  if (!LangOpts.empty()) {
-for (auto I = LangOpts.begin(), E = LangOpts.end(); I != E; ++I) {
-  const StringRef Part = (*I)->getValueAsString("Name");
-  if ((*I)->getValueAsBit("Negated"))
-OS << "!";
-  OS << "LangOpts." << Part;
-  if (I + 1 != E)
-OS << " || ";
-}
-  } else
-OS << "true";
-  OS << "));\n";
+ << ", /*IsSupported=*/" << GenerateTestExpression(LangOpts)
+ << "));\n";
 }
   }
   OS << "}\n\n";
@@ -3431,23 +3444,12 @@
   if (LangOpts.empty())
 return "defaultDiagnoseLangOpts";
 
-  // Generate the test condition, as well as a unique function name for the
-  // diagnostic test. The list of options should usually be short (one or two
-  // options), and the uniqueness isn't strictly necessary (it is just for
-  // codegen efficiency).
-  std::string FnName = "check", Test;
-  for (auto I = LangOpts.begin(), E = LangOpts.end(); I != E; ++I) {
-const StringRef Part = (*I)->getValueAsString("Name");
-if ((*I)->getValueAsBit("Negated")) {
-  FnName += "Not";
-  Test += "!";
-}
-Test += "S.LangOpts.";
-Test +=  Part;
-if (I + 1 != E)
-  Test += " || ";
-FnName += Part;
-  }
+  // Generate a unique function name for the diagnostic test. The list of
+  // options should usually be short (one or two options), and the
+  // uniqueness isn't strictly necessary (it is just for codegen efficiency).
+  std::string FnName = "check";
+  for (auto I = LangOpts.begin(), E = LangOpts.end(); I != E; ++I)
+FnName += (*I)->getValueAsString("Name");
   FnName += "LangOpts";
 
   // If this code has already been generated, simply return the previous
@@ -3458,7 +3460,8 @@
 return *I;
 
   OS << "static bool " << FnName << "(Sema &S, const ParsedAttr &Attr) {\n";
-  OS << "  if (" << Test << ")\n";
+  OS << "  auto &LangOpts = S.LangOpts;\n";
+  OS << "  if (" << GenerateTestExpression(LangOpts) << ")\n";
   OS << "return true;\n\n";
   OS << "  S.Diag(Attr.getLoc(), diag::warn_attribute_ignored) ";
   OS << "<< Attr.getName();\n";
Index: clang/test/SemaObjC/class-stub-attr.m
===
--- /dev/null
+++ clang/test/SemaObjC/class-stub-attr.m
@@ -0,0 +1,27 @@
+// RUN: %clang -target x86_64-apple-darwin -fsyntax-only -Xclang -verify %s
+// RUN: %clang -target x86_64-apple-darwin -x objective-c++ -fsyntax-only -Xclang -verify %s
+
+@interface NSObject
+@end
+
+__attribute__((objc_class_stub))
+@interface MissingSubclassingRestrictedAttribute : NSObject // expected-error {{'objc_class_stub' attribute cannot be specified on a class that does not have the 'objc_subclassing_restricted' attribute}}
+@end
+
+__attribute__((objc_class_stub))
+__attribute__((objc_subclassing_restricted))
+@interface ValidClassStubAttribute : NSObject
+@end
+
+@implementation ValidClassStubAttribute // expected-error {{cannot declare implementation of a class declared with the 'objc_class_stub' attribute}}
+@end
+
+@implementation ValidClassStubAttribute (MyCategory)
+@end
+
+__attribute__((objc_class_stub(123))) // expected-error {{'objc_clas

[PATCH] D59628: Add support for __attribute__((objc_class_stub))

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Thanks!  Down to minor stuff now.




Comment at: clang/include/clang/Basic/Attr.td:291
+  string CustomCode = customCode;
 }
 def MicrosoftExt : LangOpt<"MicrosoftExt">;

Please add a comment here explaining how to use `CustomCode`.

Can we just remove `Negated` and define `COnly` as `!LangOpts.CPlusPlus`?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59628/new/

https://reviews.llvm.org/D59628



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59628: Add support for __attribute__((objc_class_stub))

[Slava Pestov via Phabricator via cfe-commits]

slavapestov updated this revision to Diff 199885.
slavapestov added a comment.

Updated patch to address review feedback.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59628/new/

https://reviews.llvm.org/D59628

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/ObjCRuntime.h
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/test/CodeGenObjC/class-stubs.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaObjC/class-stub-attr-unsupported.m
  clang/test/SemaObjC/class-stub-attr.m
  clang/utils/TableGen/ClangAttrEmitter.cpp

Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -1922,6 +1922,34 @@
   return true;
 }
 
+static std::string GenerateTestExpression(ArrayRef LangOpts) {
+  std::string Test;
+
+  for (auto *E : LangOpts) {
+if (!Test.empty())
+  Test += " || ";
+
+if (E->getValueAsBit("Negated")) {
+  Test += "!";
+}
+
+const StringRef Code = E->getValueAsString("CustomCode");
+if (!Code.empty()) {
+  Test += "(";
+  Test += Code;
+  Test += ")";
+} else {
+  Test += "LangOpts.";
+  Test += E->getValueAsString("Name");
+}
+  }
+
+  if (Test.empty())
+return "true";
+
+  return Test;
+}
+
 std::string
 PragmaClangAttributeSupport::generateStrictConformsTo(const Record &Attr,
   raw_ostream &OS) {
@@ -1948,19 +1976,8 @@
   // rules if the specific language options are specified.
   std::vector LangOpts = Rule.getLangOpts();
   OS << "  MatchRules.push_back(std::make_pair(" << Rule.getEnumValue()
- << ", /*IsSupported=*/";
-  if (!LangOpts.empty()) {
-for (auto I = LangOpts.begin(), E = LangOpts.end(); I != E; ++I) {
-  const StringRef Part = (*I)->getValueAsString("Name");
-  if ((*I)->getValueAsBit("Negated"))
-OS << "!";
-  OS << "LangOpts." << Part;
-  if (I + 1 != E)
-OS << " || ";
-}
-  } else
-OS << "true";
-  OS << "));\n";
+ << ", /*IsSupported=*/" << GenerateTestExpression(LangOpts)
+ << "));\n";
 }
   }
   OS << "}\n\n";
@@ -3431,22 +3448,14 @@
   if (LangOpts.empty())
 return "defaultDiagnoseLangOpts";
 
-  // Generate the test condition, as well as a unique function name for the
-  // diagnostic test. The list of options should usually be short (one or two
-  // options), and the uniqueness isn't strictly necessary (it is just for
-  // codegen efficiency).
-  std::string FnName = "check", Test;
+  // Generate a unique function name for the diagnostic test. The list of
+  // options should usually be short (one or two options), and the
+  // uniqueness isn't strictly necessary (it is just for codegen efficiency).
+  std::string FnName = "check";
   for (auto I = LangOpts.begin(), E = LangOpts.end(); I != E; ++I) {
-const StringRef Part = (*I)->getValueAsString("Name");
-if ((*I)->getValueAsBit("Negated")) {
+if ((*I)->getValueAsBit("Negated"))
   FnName += "Not";
-  Test += "!";
-}
-Test += "S.LangOpts.";
-Test +=  Part;
-if (I + 1 != E)
-  Test += " || ";
-FnName += Part;
+FnName += (*I)->getValueAsString("Name");
   }
   FnName += "LangOpts";
 
@@ -3458,7 +3467,8 @@
 return *I;
 
   OS << "static bool " << FnName << "(Sema &S, const ParsedAttr &Attr) {\n";
-  OS << "  if (" << Test << ")\n";
+  OS << "  auto &LangOpts = S.LangOpts;\n";
+  OS << "  if (" << GenerateTestExpression(LangOpts) << ")\n";
   OS << "return true;\n\n";
   OS << "  S.Diag(Attr.getLoc(), diag::warn_attribute_ignored) ";
   OS << "<< Attr.getName();\n";
Index: clang/test/SemaObjC/class-stub-attr.m
===
--- /dev/null
+++ clang/test/SemaObjC/class-stub-attr.m
@@ -0,0 +1,27 @@
+// RUN: %clang -target x86_64-apple-darwin -fsyntax-only -Xclang -verify %s
+// RUN: %clang -target x86_64-apple-darwin -x objective-c++ -fsyntax-only -Xclang -verify %s
+
+@interface NSObject
+@end
+
+__attribute__((objc_class_stub))
+@interface MissingSubclassingRestrictedAttribute : NSObject // expected-error {{'objc_class_stub' attribute cannot be specified on a class that does not have the 'objc_subclassing_restricted' attribute}}
+@end
+
+__attribute__((objc_class_stub))
+__attribute__((objc_subclassing_restricted))
+@interface ValidClassStubAttribute : NSObject
+@end
+
+@implementation ValidClassStubAttribute // expected-error {{cannot declare implementation of a class declared with the 'objc_class_stub' attribute}}
+@end
+
+@implementation ValidClassStubAttribute (MyCategory)
+@end
+
+__attribute__((objc_class_s

[PATCH] D59628: Add support for __attribute__((objc_class_stub))

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: include/clang/Basic/Attr.td:290
 
-class LangOpt {
+class LangOpt {
   string Name = name;

I think there's a grand total of one use of `negated`, so you might as well 
rewrite it to use `customCode`; see below.



Comment at: include/clang/Basic/Attr.td:306
+def ObjCNonFragileRuntime : LangOpt<"ObjCNonFragileRuntime", 0,
+"LangOpts.ObjCRuntime.allowsClassStubs()">;
 

This is not an appropriate name for this `LangOpt`.  How about 
`ObjCAllowsClassStubs`?



Comment at: include/clang/Basic/AttrDocs.td:1100
+implementations defined for them. This attribute is intended for use in
+Swift generated headers for classes defined in Swift.
+}];

We should add something here to make it clear what the evolution story with 
this attribute is.  Presumably you can't add it to an existing class without 
breaking ABI.  Are there circumstances under which we're willing to guarantee 
that it can be removed (under a sufficiently-recent deployment target)?



Comment at: utils/TableGen/ClangAttrEmitter.cpp:3442
+if (!Code.empty()) {
+  Test += "S.";
+  Test += Code;

I think the contract with `CustomCode` ought to be that `LangOpts` will be 
bound as an unqualified name, so that the custom code can be an arbitrary 
expression in terms of that.  That will allow e.g. compound and negated 
`LangOpt`s to be defined, like `LangOpts.OpenCL && LangOpts.CPlusPlus`.  So you 
need to generate `auto &LangOpts = S.LangOpts;` at the top of the function, at 
least when custom code is present; and you should probably add parens around 
the code just to be safe.

Alternatively, you could make the contract that `CustomCode` should contain 
`%0` or some other string that'll be expanded to an expression for the language 
options, but that sounds pretty complex compared to just binding `LangOpts` in 
the context.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59628/new/

https://reviews.llvm.org/D59628



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59628: Add support for __attribute__((objc_class_stub))

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/Basic/AttrDocs.td:1116
+def ObjCClassStubDocs : Documentation {
+let Category = DocCatFunction;
+let Content = [{

aaron.ballman wrote:
> slavapestov wrote:
> > aaron.ballman wrote:
> > > This seems like the wrong category -- the attribute doesn't apply to 
> > > functions.
> > Would DocCatType make more sense? Would you like me to change 
> > ObjCRuntimeVisible and a handful of other miscategorized attributes too?
> Oye, it looks like we have a lot of inconsistent categories currently. It's 
> not really a type attribute, it's a declaration attribute, but we don't have 
> such a notion yet. Go ahead and leave this as `DocCatType` for now; I'll see 
> about cleaning this up in a follow-up.
I added `DocCatDecl` in r357585, which I think would be the correct category to 
use here.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59628/new/

https://reviews.llvm.org/D59628



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59628: Add support for __attribute__((objc_class_stub))

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/Basic/Attr.td:293
   bit Negated = negated;
+  string CustomCode = customCode;
 }

I think the type here should be `code` instead of `string` since the user is 
passing in code snippets, no?



Comment at: lib/Sema/SemaDeclObjC.cpp:4129-4130
+
+if (IntfDecl->hasAttr()) {
+  if (!IntfDecl->hasAttr())
+Diag(IntfDecl->getLocation(), 
diag::err_class_stub_subclassing_mismatch);

Combine these `if` statements?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59628/new/

https://reviews.llvm.org/D59628



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59628: Add support for __attribute__((objc_class_stub))

[Slava Pestov via Phabricator via cfe-commits]

slavapestov updated this revision to Diff 193407.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59628/new/

https://reviews.llvm.org/D59628

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/ObjCRuntime.h
  lib/CodeGen/CGObjCMac.cpp
  lib/Sema/SemaDeclAttr.cpp
  lib/Sema/SemaDeclObjC.cpp
  test/CodeGenObjC/class-stubs.m
  test/Misc/pragma-attribute-supported-attributes-list.test
  test/SemaObjC/class-stub-attr-unsupported.m
  test/SemaObjC/class-stub-attr.m
  utils/TableGen/ClangAttrEmitter.cpp

Index: utils/TableGen/ClangAttrEmitter.cpp
===
--- utils/TableGen/ClangAttrEmitter.cpp
+++ utils/TableGen/ClangAttrEmitter.cpp
@@ -1969,10 +1969,15 @@
  << ", /*IsSupported=*/";
   if (!LangOpts.empty()) {
 for (auto I = LangOpts.begin(), E = LangOpts.end(); I != E; ++I) {
-  const StringRef Part = (*I)->getValueAsString("Name");
   if ((*I)->getValueAsBit("Negated"))
 OS << "!";
-  OS << "LangOpts." << Part;
+  const StringRef Code = (*I)->getValueAsString("CustomCode");
+  if (!Code.empty()) {
+OS << Code;
+  } else {
+const StringRef Name = (*I)->getValueAsString("Name");
+OS << "LangOpts." << Name;
+  }
   if (I + 1 != E)
 OS << " || ";
 }
@@ -2957,15 +2962,15 @@
 OS << "case AttrSyntax::" << Variety << ": {\n";
 // C++11-style attributes are further split out based on the Scope.
 for (auto I = List.cbegin(), E = List.cend(); I != E; ++I) {
-  if (I != List.cbegin())
-OS << " else ";
-  if (I->first.empty())
-OS << "if (ScopeName == \"\") {\n";
-  else
-OS << "if (ScopeName == \"" << I->first << "\") {\n";
-  OS << "  return llvm::StringSwitch(Name)\n";
-  GenerateHasAttrSpellingStringSwitch(I->second, OS, Spelling, I->first);
-  OS << "}";
+  if (I != List.cbegin())
+OS << " else ";
+  if (I->first.empty())
+OS << "if (ScopeName == \"\") {\n";
+  else
+OS << "if (ScopeName == \"" << I->first << "\") {\n";
+  OS << "  return llvm::StringSwitch(Name)\n";
+  GenerateHasAttrSpellingStringSwitch(I->second, OS, Spelling, I->first);
+  OS << "}";
 }
 OS << "\n} break;\n";
   };
@@ -3426,16 +3431,22 @@
   // codegen efficiency).
   std::string FnName = "check", Test;
   for (auto I = LangOpts.begin(), E = LangOpts.end(); I != E; ++I) {
-const StringRef Part = (*I)->getValueAsString("Name");
 if ((*I)->getValueAsBit("Negated")) {
   FnName += "Not";
   Test += "!";
 }
-Test += "S.LangOpts.";
-Test +=  Part;
+const StringRef Name = (*I)->getValueAsString("Name");
+FnName += Name;
+const StringRef Code = (*I)->getValueAsString("CustomCode");
+if (!Code.empty()) {
+  Test += "S.";
+  Test += Code;
+} else {
+  Test += "S.LangOpts.";
+  Test += Name;
+}
 if (I + 1 != E)
   Test += " || ";
-FnName += Part;
   }
   FnName += "LangOpts";
 
Index: test/SemaObjC/class-stub-attr.m
===
--- /dev/null
+++ test/SemaObjC/class-stub-attr.m
@@ -0,0 +1,27 @@
+// RUN: %clang -target x86_64-apple-darwin -fsyntax-only -Xclang -verify %s
+// RUN: %clang -target x86_64-apple-darwin -x objective-c++ -fsyntax-only -Xclang -verify %s
+
+@interface NSObject
+@end
+
+__attribute__((objc_class_stub))
+@interface MissingSubclassingRestrictedAttribute : NSObject // expected-error {{'objc_class_stub' attribute cannot be specified on a class that does not have the 'objc_subclassing_restricted' attribute}}
+@end
+
+__attribute__((objc_class_stub))
+__attribute__((objc_subclassing_restricted))
+@interface ValidClassStubAttribute : NSObject
+@end
+
+@implementation ValidClassStubAttribute // expected-error {{cannot declare implementation of a class declared with the 'objc_class_stub' attribute}}
+@end
+
+@implementation ValidClassStubAttribute (MyCategory)
+@end
+
+__attribute__((objc_class_stub(123))) // expected-error {{'objc_class_stub' attribute takes no arguments}}
+@interface InvalidClassStubAttribute : NSObject
+@end
+
+__attribute__((objc_class_stub)) // expected-error {{'objc_class_stub' attribute only applies to Objective-C interfaces}}
+int cannotHaveObjCClassStubAttribute() {}
Index: test/SemaObjC/class-stub-attr-unsupported.m
===
--- /dev/null
+++ test/SemaObjC/class-stub-attr-unsupported.m
@@ -0,0 +1,10 @@
+// RUN: %clang -target i386-apple-darwin -fsyntax-only -Xclang -verify %s
+// RUN: %clang -target i386-apple-darwin -x objective-c++ -fsyntax-only -Xclang -verify %s
+
+@interface NSObject
+@end
+
+__attribute__((objc_class_stub)) // expected-warning {{'objc_class_stub' attribute ignored}}
+__attribute__((objc_su

[PATCH] D59628: Add support for __attribute__((objc_class_stub))

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/Sema/SemaDeclObjC.cpp:4131-4133
+  if (!getLangOpts().ObjCRuntime.allowsClassStubs()) {
+Diag(IntfDecl->getLocation(), diag::err_class_stub_not_supported);
+  }

aaron.ballman wrote:
> This should be done in Attr.td. You'll need to add a new LangOpt subclass 
> (around line 298 or so are examples), and then add it to the `LangOpts` array 
> when defining the attribute. Then you can remove this code as well as the new 
> diagnostic.
I don't think there's a way to run arbitrary code in a `LangOpt` right now, but 
it should be relatively straightforward to generalize `ClangAttrEmitter` to 
handle this.  Just add an optional `Code` property to `LangOpt` that's 
expressed in terms of an assumed variable `LangOpts`, so that `Attr.td` could 
have a line like:

```
  def ObjCClassStubsAllowed : LangOpt<"ObjCClassStubsAllowed", 
"LangOpts.ObjCRuntime.allowsClassStubs()">;
```

`ClangAttrEmitter` would take that expression, parenthesize it, and use it 
where it currently expands to `"LangOpts." + Name`.  It should be possible to 
remove the `Negated` field in favor of this.

I guess that's probably worth doing vs. just having some hard-coded logic.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59628/new/

https://reviews.llvm.org/D59628



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59628: Add support for __attribute__((objc_class_stub))

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D59628#1446626 , @slavapestov wrote:

> I don't know what the etiquette is around here about pinging reviewers for a 
> re-review, but this CL is ready for another look. Your feedback is much 
> appreciated!


Thanks for letting us know you think you've addressed all the feedback. We 
usually recommend pinging once a week if there's no activity on a review 
thread, but it's no problem to ping sooner when you have more information.




Comment at: include/clang/Basic/Attr.td:1802
+  let Documentation = [ObjCClassStubDocs];
+}
+

slavapestov wrote:
> aaron.ballman wrote:
> > Does this attribute make sense outside of ObjC? e.g., should it require an 
> > ObjC compiland? If it should only be used in ObjC, then it should have a 
> > `LangOpts` field.
> The other ObjC attributes, such as ObjCSubclassingRestricted, do not have a 
> LangOpts. Do you want me to add a LangOpts field to those too? Or is it 
> unnecessary since they're already restricted to InterfaceDecls?
> The other ObjC attributes, such as ObjCSubclassingRestricted, do not have a 
> LangOpts. Do you want me to add a LangOpts field to those too? Or is it 
> unnecessary since they're already restricted to InterfaceDecls?

Don't feel obligated, but if you wanted to post a follow-up patch that adds 
this to the other ObjC attribute, I think it would be a good change to make. 
It's not strictly required, but I think the diagnostics are a bit more clear 
when we restrict the language options.



Comment at: include/clang/Basic/AttrDocs.td:1116
+def ObjCClassStubDocs : Documentation {
+let Category = DocCatFunction;
+let Content = [{

slavapestov wrote:
> aaron.ballman wrote:
> > This seems like the wrong category -- the attribute doesn't apply to 
> > functions.
> Would DocCatType make more sense? Would you like me to change 
> ObjCRuntimeVisible and a handful of other miscategorized attributes too?
Oye, it looks like we have a lot of inconsistent categories currently. It's not 
really a type attribute, it's a declaration attribute, but we don't have such a 
notion yet. Go ahead and leave this as `DocCatType` for now; I'll see about 
cleaning this up in a follow-up.



Comment at: include/clang/Basic/ObjCRuntime.h:437-443
+case FragileMacOSX: return false;
+case GCC: return false;
+case MacOSX: return true;
+case GNUstep: return false;
+case ObjFW: return false;
+case iOS: return true;
+case WatchOS: return true;

I'd prefer if you grouped these cases and did it like:
```
switch (getKind()) {
case FragileMacOSX:
case GCC:
case GNUstep:
case ObjFW:
  return false;
case MacOSX:
case iOS:
case WatchOS:
  return true;
}
```



Comment at: lib/CodeGen/CGObjCMac.cpp:7269
+CGObjCNonFragileABIMac::GetClassGlobalForClassRef(const ObjCInterfaceDecl *ID) 
{
+  auto *ClassGV = GetClassGlobal(ID, /*metaclass*/ false, NotForDefinition);
+

Please don't use `auto` here, the type is not spelled out in the initialization.



Comment at: lib/CodeGen/CGObjCMac.cpp:7347
   if (!Entry) {
-auto ClassGV = GetClassGlobal(ID, /*metaclass*/ false, NotForDefinition);
-Entry = new llvm::GlobalVariable(CGM.getModule(), 
ObjCTypes.ClassnfABIPtrTy,
+auto ClassGV = GetClassGlobalForClassRef(ID);
+Entry = new llvm::GlobalVariable(CGM.getModule(), ClassGV->getType(),

Don't use `auto` here either.



Comment at: lib/Sema/SemaDeclObjC.cpp:4097
 
+  if (IDecl->hasAttr()) {
+Diag(IC->getLocation(), diag::err_implementation_of_class_stub);

Elide braces.



Comment at: lib/Sema/SemaDeclObjC.cpp:4131-4133
+  if (!getLangOpts().ObjCRuntime.allowsClassStubs()) {
+Diag(IntfDecl->getLocation(), diag::err_class_stub_not_supported);
+  }

This should be done in Attr.td. You'll need to add a new LangOpt subclass 
(around line 298 or so are examples), and then add it to the `LangOpts` array 
when defining the attribute. Then you can remove this code as well as the new 
diagnostic.



Comment at: lib/Sema/SemaDeclObjC.cpp:4134
+  }
+  if (!IntfDecl->hasAttr()) {
+Diag(IntfDecl->getLocation(), 
diag::err_class_stub_subclassing_mismatch);

Elide braces.



Comment at: test/CodeGenObjC/class-stubs.m:2
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -Wno-objc-root-class 
-emit-llvm -o - %s | \
+// RUN: FileCheck %s
+

Hmm, this looks suspicious -- I think it's more clear to just bump this to the 
previous line rather than wonder if the RUN: command will confuse things.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59628/new/

https://reviews.llvm.org/D59628



___
cfe-commits mailing list
cfe-commit

[PATCH] D59628: Add support for __attribute__((objc_class_stub))

[Slava Pestov via Phabricator via cfe-commits]

slavapestov added a comment.

I don't know what the etiquette is around here about pinging reviewers for a 
re-review, but this CL is ready for another look. Your feedback is much 
appreciated!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59628/new/

https://reviews.llvm.org/D59628



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59628: Add support for __attribute__((objc_class_stub))

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/CodeGen/CGObjCMac.cpp:7274
+// Classrefs pointing at Objective-C stub classes have the least
+// significant bit set to 1.
+auto *Tag = llvm::ConstantInt::get(CGM.IntPtrTy, 1);

slavapestov wrote:
> rjmccall wrote:
> > This isn't for an arbitrary class ref, it's for the global class list.  I'd 
> > say something like "the global class list sets the LSB to 1 on any class 
> > stubs".
> Can you explain what the difference is? My understanding is that the thing 
> you pass to objc_loadClassref() (or load from directly) is a "classref". This 
> is for classes you reference, not classes you define.
Oh, I see what you're saying, nevermind.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59628/new/

https://reviews.llvm.org/D59628



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59628: Add support for __attribute__((objc_class_stub))

[Slava Pestov via Phabricator via cfe-commits]

slavapestov updated this revision to Diff 192378.
slavapestov marked 2 inline comments as done.
slavapestov added a comment.

Second revision adds the following:

- Validation of the attribute in Sema
- Sema tests for the above
- Correct behavior of super method calls of a class with the attribute
- More comprehensive CodeGen tests


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59628/new/

https://reviews.llvm.org/D59628

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/ObjCRuntime.h
  lib/CodeGen/CGObjCMac.cpp
  lib/Sema/SemaDeclAttr.cpp
  lib/Sema/SemaDeclObjC.cpp
  test/CodeGenObjC/class-stubs.m
  test/Misc/pragma-attribute-supported-attributes-list.test
  test/SemaObjC/class-stub-attr-unsupported.m
  test/SemaObjC/class-stub-attr.m

Index: test/SemaObjC/class-stub-attr.m
===
--- /dev/null
+++ test/SemaObjC/class-stub-attr.m
@@ -0,0 +1,27 @@
+// RUN: %clang -target x86_64-apple-darwin -fsyntax-only -Xclang -verify %s
+// RUN: %clang -target x86_64-apple-darwin -x objective-c++ -fsyntax-only -Xclang -verify %s
+
+@interface NSObject
+@end
+
+__attribute__((objc_class_stub))
+@interface MissingSubclassingRestrictedAttribute : NSObject // expected-error {{'objc_class_stub' attribute cannot be specified on a class that does not have the 'objc_subclassing_restricted' attribute}}
+@end
+
+__attribute__((objc_class_stub))
+__attribute__((objc_subclassing_restricted))
+@interface ValidClassStubAttribute : NSObject
+@end
+
+@implementation ValidClassStubAttribute // expected-error {{cannot declare implementation of a class declared with the 'objc_class_stub' attribute}}
+@end
+
+@implementation ValidClassStubAttribute (MyCategory)
+@end
+
+__attribute__((objc_class_stub(123))) // expected-error {{'objc_class_stub' attribute takes no arguments}}
+@interface InvalidClassStubAttribute : NSObject
+@end
+
+__attribute__((objc_class_stub)) // expected-error {{'objc_class_stub' attribute only applies to Objective-C interfaces}}
+int cannotHaveObjCClassStubAttribute() {}
Index: test/SemaObjC/class-stub-attr-unsupported.m
===
--- /dev/null
+++ test/SemaObjC/class-stub-attr-unsupported.m
@@ -0,0 +1,10 @@
+// RUN: %clang -target i386-apple-darwin -fsyntax-only -Xclang -verify %s
+// RUN: %clang -target i386-apple-darwin -x objective-c++ -fsyntax-only -Xclang -verify %s
+
+@interface NSObject
+@end
+
+__attribute__((objc_class_stub))
+__attribute__((objc_subclassing_restricted))
+@interface StubClass : NSObject // expected-error {{'objc_class_stub' attribute is not supported for this target}}
+@end
Index: test/Misc/pragma-attribute-supported-attributes-list.test
===
--- test/Misc/pragma-attribute-supported-attributes-list.test
+++ test/Misc/pragma-attribute-supported-attributes-list.test
@@ -95,6 +95,7 @@
 // CHECK-NEXT: ObjCBridge (SubjectMatchRule_record, SubjectMatchRule_type_alias)
 // CHECK-NEXT: ObjCBridgeMutable (SubjectMatchRule_record)
 // CHECK-NEXT: ObjCBridgeRelated (SubjectMatchRule_record)
+// CHECK-NEXT: ObjCClassStub (SubjectMatchRule_objc_interface)
 // CHECK-NEXT: ObjCCompleteDefinition (SubjectMatchRule_objc_interface)
 // CHECK-NEXT: ObjCDesignatedInitializer (SubjectMatchRule_objc_method)
 // CHECK-NEXT: ObjCException (SubjectMatchRule_objc_interface)
Index: test/CodeGenObjC/class-stubs.m
===
--- /dev/null
+++ test/CodeGenObjC/class-stubs.m
@@ -0,0 +1,81 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -Wno-objc-root-class -emit-llvm -o - %s | \
+// RUN: FileCheck %s
+
+// -- classref for the message send in main()
+//
+// The class is declared with objc_class_stub, so LSB of the class pointer
+// must be set to 1.
+//
+// CHECK-LABEL: @"OBJC_CLASSLIST_REFERENCES_$_" = private global i8* getelementptr (i8, i8* bitcast (%struct._class_t* @"OBJC_CLASS_$_Base" to i8*), i32 1), section "__DATA,__objc_classrefs,regular,no_dead_strip", align 8
+
+// -- classref for the super message send in anotherClassMethod()
+//
+// Metaclasses do not use the "stub" mechanism and are referenced statically.
+//
+// CHECK-LABEL: @"OBJC_CLASSLIST_SUP_REFS_$_" = private global %struct._class_t* @"OBJC_METACLASS_$_Derived", section "__DATA,__objc_superrefs,regular,no_dead_strip", align 8
+
+// -- classref for the super message send in anotherInstanceMethod()
+//
+// The class is declared with objc_class_stub, so LSB of the class pointer
+// must be set to 1.
+//
+// CHECK-LABEL: @"OBJC_CLASSLIST_SUP_REFS_$_.1" = private global i8* getelementptr (i8, i8* bitcast (%struct._class_t* @"OBJC_CLASS_$_Derived" to i8*), i32 1), section "__DATA,__objc_superrefs,regular,no_dead_strip", align 8
+
+__attribute__((objc_class_stub))
+__attribute__((objc_subclassing_restricted))
+@interface 

[PATCH] D59628: Add support for __attribute__((objc_class_stub))

[Slava Pestov via Phabricator via cfe-commits]

slavapestov marked 11 inline comments as done.
slavapestov added a comment.

Thanks for the comments everyone. In the next version of the patch, I fixed 
most of the review feedback and also addressed an issue where super message 
sends were not calling `objc_loadClassRef()`.




Comment at: include/clang/Basic/Attr.td:1802
+  let Documentation = [ObjCClassStubDocs];
+}
+

aaron.ballman wrote:
> Does this attribute make sense outside of ObjC? e.g., should it require an 
> ObjC compiland? If it should only be used in ObjC, then it should have a 
> `LangOpts` field.
The other ObjC attributes, such as ObjCSubclassingRestricted, do not have a 
LangOpts. Do you want me to add a LangOpts field to those too? Or is it 
unnecessary since they're already restricted to InterfaceDecls?



Comment at: include/clang/Basic/AttrDocs.td:1116
+def ObjCClassStubDocs : Documentation {
+let Category = DocCatFunction;
+let Content = [{

aaron.ballman wrote:
> This seems like the wrong category -- the attribute doesn't apply to 
> functions.
Would DocCatType make more sense? Would you like me to change 
ObjCRuntimeVisible and a handful of other miscategorized attributes too?



Comment at: lib/CodeGen/CGObjCMac.cpp:7274
+// Classrefs pointing at Objective-C stub classes have the least
+// significant bit set to 1.
+auto *Tag = llvm::ConstantInt::get(CGM.IntPtrTy, 1);

rjmccall wrote:
> This isn't for an arbitrary class ref, it's for the global class list.  I'd 
> say something like "the global class list sets the LSB to 1 on any class 
> stubs".
Can you explain what the difference is? My understanding is that the thing you 
pass to objc_loadClassref() (or load from directly) is a "classref". This is 
for classes you reference, not classes you define.



Comment at: lib/CodeGen/CGObjCMac.cpp:7285
 
-Entry = new llvm::GlobalVariable(CGM.getModule(), 
ObjCTypes.ClassnfABIPtrTy,
+Entry = new llvm::GlobalVariable(CGM.getModule(), ClassGV->getType(),
  false, llvm::GlobalValue::PrivateLinkage,

jordan_rose wrote:
> Is there a concern here in the non-stub case if GetClassGlobal no longer 
> produces a ObjCTypes.ClassnfABIPtrTy? (Probably not, but thought I'd check 
> [again].)
You raise a good point. I brought back the assert in the next iteration of the 
patch.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59628/new/

https://reviews.llvm.org/D59628



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59628: Add support for __attribute__((objc_class_stub))

[Jordan Rose via Phabricator via cfe-commits]

jordan_rose added inline comments.



Comment at: lib/CodeGen/CGObjCMac.cpp:7285
 
-Entry = new llvm::GlobalVariable(CGM.getModule(), 
ObjCTypes.ClassnfABIPtrTy,
+Entry = new llvm::GlobalVariable(CGM.getModule(), ClassGV->getType(),
  false, llvm::GlobalValue::PrivateLinkage,

Is there a concern here in the non-stub case if GetClassGlobal no longer 
produces a ObjCTypes.ClassnfABIPtrTy? (Probably not, but thought I'd check 
[again].)


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59628/new/

https://reviews.llvm.org/D59628



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59628: Add support for __attribute__((objc_class_stub))

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

You should also add Sema tests that ensure the attribute applies to the 
expected AST nodes, is diagnosed appropriately when applied to something it 
shouldn't be applied to, accepts no args, etc. Basically, all of the semantic 
places we could warn on should have test coverage, as well as demonstrations of 
correct usage.




Comment at: include/clang/Basic/Attr.td:1798
 
+def ObjCClassStub: Attr {
+  let Spellings = [Clang<"objc_class_stub">];

Formatting is a smidge off here -- should be `ObjCClassStub : Attr` with the 
extra whitespace.



Comment at: include/clang/Basic/Attr.td:1802
+  let Documentation = [ObjCClassStubDocs];
+}
+

Does this attribute make sense outside of ObjC? e.g., should it require an ObjC 
compiland? If it should only be used in ObjC, then it should have a `LangOpts` 
field.



Comment at: include/clang/Basic/AttrDocs.td:1116
+def ObjCClassStubDocs : Documentation {
+let Category = DocCatFunction;
+let Content = [{

This seems like the wrong category -- the attribute doesn't apply to functions.



Comment at: include/clang/Basic/AttrDocs.td:1118
+let Content = [{
+This attribute specifies that the Objective-C class to which it applies has 
dynamically-allocated metadata. Classes annotated with this attribute cannot be 
subclassed.
+}];

rjmccall wrote:
> You should probably check that the user doesn't try to subclass classes 
> annotated with this attribute, then. :)
Try to keep the docs wrapped to the usual 80-col limit.

I think this could use a bit more exposition, or links to explain stuff. Based 
on the small amount here, I still have no idea why I would use this attribute.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59628/new/

https://reviews.llvm.org/D59628



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59628: Add support for __attribute__((objc_class_stub))

[John McCall via Phabricator via cfe-commits]

rjmccall requested changes to this revision.
rjmccall added inline comments.
This revision now requires changes to proceed.



Comment at: include/clang/Basic/AttrDocs.td:1118
+let Content = [{
+This attribute specifies that the Objective-C class to which it applies has 
dynamically-allocated metadata. Classes annotated with this attribute cannot be 
subclassed.
+}];

You should probably check that the user doesn't try to subclass classes 
annotated with this attribute, then. :)



Comment at: lib/CodeGen/CGObjCMac.cpp:735
+  llvm::Constant *getLoadClassrefFn() const {
+// FIXME: Other attributes?
+

It should be safe to make it `readnone`, which could theoretically optimize 
something like doing a class message-send in a loop.



Comment at: lib/CodeGen/CGObjCMac.cpp:7274
+// Classrefs pointing at Objective-C stub classes have the least
+// significant bit set to 1.
+auto *Tag = llvm::ConstantInt::get(CGM.IntPtrTy, 1);

This isn't for an arbitrary class ref, it's for the global class list.  I'd say 
something like "the global class list sets the LSB to 1 on any class stubs".



Comment at: lib/CodeGen/CGObjCMac.cpp:7278
+ClassGV = llvm::ConstantExpr::getIntToPtr(ClassGV,
+  ObjCTypes.Int8PtrTy);
+  }

It's more typical to do this with a GEP, although the add should still work.



Comment at: lib/Sema/SemaDeclAttr.cpp:7126
 break;
+  case ParsedAttr::AT_ObjCClassStub:
+handleSimpleAttribute(S, D, AL);

You should emit an error if `!getLangOpts().ObjCRuntime.allowsClassStubs()`, 
which should be straightforward to implement.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59628/new/

https://reviews.llvm.org/D59628



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59628: Add support for __attribute__((objc_class_stub))

[Slava Pestov via Phabricator via cfe-commits]

slavapestov created this revision.
slavapestov added reviewers: rjmccall, dexonsmith, aaron.ballman.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
dexonsmith edited reviewers, added: erik.pilkington, arphaman; removed: 
dexonsmith.
dexonsmith added a comment.

+Erik and Alex, can you take a look at this?


This CL lands the Clang-side support for a new Objective-C runtime feature that 
adds support for dynamically-allocated metadata.

What follows is a basic description of this mechanism:

In Swift, the class metadata contains an Objective-C class object as a prefix, 
followed by a vtable, generic parameters, and field offsets. If the inheritance 
hierarchy of a class crosses a resilience boundary, we cannot statically emit 
the entire metadata. Instead, it is lazily built at runtime when first needed.

In order for such classes to be accessed from Clang, the Objective-C runtime 
will support a new mechanism whereby Swift can instead emit a "class stub" 
object. The class stub object is used by Clang in places where a class 
reference is normally emitted:

- Classrefs emitted when messaging the class
- Categories

A class stub contains an "isa pointer" which is a scalar value 1 followed by a 
function pointer which realizes the real class metadata. A reference to a class 
stub from a classref also has its least significant pointer set to 1.

The Objective-C runtime distinguishes a class stub from a real class using this 
fake "isa pointer". Instead of directly loading from a classref, Clang instead 
calls `objc_loadClassref()`; this function checks if the classref contains a 
class stub, and if so, it calls the function pointer and stores the result back 
into the classref to fast path future loads.

Note that Clang already supports a objc_runtime_visible attribute. The 
objc_class_stub attribute can be thought of as a generalization of 
objc_runtime_visible. Whereas objc_runtime_visible replaces the classref load 
with a runtime lookup of a class by string, the class stub mechanism is more 
efficient because the result of the lookup is cached. Also, 
objc_runtime_visible does not support categories to be defined on the class, 
whereas objc_class_stub does.


Repository:
  rC Clang

https://reviews.llvm.org/D59628

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  lib/CodeGen/CGObjCMac.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/CodeGenObjC/class-stubs.m

Index: test/CodeGenObjC/class-stubs.m
===
--- /dev/null
+++ test/CodeGenObjC/class-stubs.m
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -Wno-objc-root-class -emit-llvm -o - %s | \
+// RUN: FileCheck %s
+
+__attribute__((objc_class_stub))
+@interface A
++ (void) classMethod;
+@end
+
+int main() {
+  [A classMethod];
+}
+
+// CHECK-LABEL: @"OBJC_CLASSLIST_REFERENCES_$_" = private global i8* inttoptr (i64 add (i64 ptrtoint (%struct._class_t* @"OBJC_CLASS_$_A" to i64), i64 1) to i8*), section "__DATA,__objc_classrefs,regular,no_dead_strip", align 8
+
+
+// CHECK-LABEL: define i32 @main()
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   [[CLASS:%.*]] = call %struct._class_t* @objc_loadClassref(i8** @"OBJC_CLASSLIST_REFERENCES_$_")
+// CHECK-NEXT:   [[SELECTOR:%.*]] = load i8*, i8** @OBJC_SELECTOR_REFERENCES_
+// CHECK-NEXT:   [[RECEIVER:%.*]] = bitcast %struct._class_t* [[CLASS]] to i8*
+// CHECK-NEXT:   call void bitcast (i8* (i8*, i8*, ...)* @objc_msgSend to void (i8*, i8*)*)(i8* [[RECEIVER]], i8* [[SELECTOR]])
+// CHECK-NEXT:   ret i32 0
+
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -7123,6 +7123,9 @@
   case ParsedAttr::AT_ObjCSubclassingRestricted:
 handleSimpleAttribute(S, D, AL);
 break;
+  case ParsedAttr::AT_ObjCClassStub:
+handleSimpleAttribute(S, D, AL);
+break;
   case ParsedAttr::AT_ObjCCompleteDefinition:
 handleSimpleAttribute(S, D, AL);
 break;
Index: lib/CodeGen/CGObjCMac.cpp
===
--- lib/CodeGen/CGObjCMac.cpp
+++ lib/CodeGen/CGObjCMac.cpp
@@ -726,6 +726,25 @@
  "objc_begin_catch");
   }
 
+  /// Class objc_loadClassref (void *)
+  ///
+  /// Loads from a classref. For Objective-C stub classes, this invokes the
+  /// initialization callback stored inside the stub. For all other classes
+  /// this simply dereferences the pointer.
+  llvm::Constant *getLoadClassrefFn() const {
+// FIXME: Other attributes?
+
+// Add the non-lazy-bind attribute, since objc_loadClassref is likely to
+// be called a lot.
+llvm::Type *params[] = { Int8PtrPtrTy };
+return CGM.CreateRuntimeFunction(
+llvm::FunctionType::get(ClassnfABIPtrTy, params, false),
+"objc_loadClassref",
+llvm::AttributeList::get(CGM.getLLVMContext(),
+ llvm::At

[PATCH] D59628: Add support for __attribute__((objc_class_stub))

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith edited reviewers, added: erik.pilkington, arphaman; removed: 
dexonsmith.
dexonsmith added a comment.

+Erik and Alex, can you take a look at this?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59628/new/

https://reviews.llvm.org/D59628



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits