[PATCH] D75210: [Attr] Allow ParsedAttr to be constructor for any Attribute

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert abandoned this revision.
jdoerfert added a comment.

Code is good and usable but no users anymore. If anyone wants to pick this up, 
feel free!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75210



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

[PATCH] D75210: [Attr] Allow ParsedAttr to be constructor for any Attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D75210#1920206 , @jdoerfert wrote:

> The reason for this patch is not there anymore. I'm fine with postponing this 
> patch until there is a user again, thoughts?


If we don't have a use for it currently, I think we should postpone it until 
there is a need.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75210



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

[PATCH] D75210: [Attr] Allow ParsedAttr to be constructor for any Attribute

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert planned changes to this revision.
jdoerfert added a comment.

The reason for this patch is not there anymore. I'm fine with postponing this 
patch until there is a user again, thoughts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75210



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

[PATCH] D75210: [Attr] Allow ParsedAttr to be constructor for any Attribute

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM aside from the issue with keeping the comment up to date from Erich (but 
please only land this once D75779  is 
approved, as that's what carries the testing load for this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75210



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

[PATCH] D75210: [Attr] Allow ParsedAttr to be constructor for any Attribute

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert updated this revision to Diff 248845.
jdoerfert added a comment.

Pass parsed attribute kind explicitly


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75210

Files:
  clang/include/clang/Sema/ParsedAttr.h
  clang/lib/Sema/ParsedAttr.cpp

Index: clang/lib/Sema/ParsedAttr.cpp
===
--- clang/lib/Sema/ParsedAttr.cpp
+++ clang/lib/Sema/ParsedAttr.cpp
@@ -42,10 +42,10 @@
   else if (HasParsedType)
 return totalSizeToAlloc(0, 0, 0, 1, 0);
+detail::PropertyData, Attr *>(0, 0, 0, 1, 0, 0);
   return totalSizeToAlloc(NumArgs, 0, 0, 0, 0);
+  detail::PropertyData, Attr *>(NumArgs, 0, 0, 0, 0, 0);
 }
 
 AttributeFactory::AttributeFactory() {
Index: clang/include/clang/Sema/ParsedAttr.h
===
--- clang/include/clang/Sema/ParsedAttr.h
+++ clang/include/clang/Sema/ParsedAttr.h
@@ -14,6 +14,7 @@
 #ifndef LLVM_CLANG_SEMA_ATTRIBUTELIST_H
 #define LLVM_CLANG_SEMA_ATTRIBUTELIST_H
 
+#include "clang/AST/Attr.h"
 #include "clang/Basic/AttrSubjectMatchRules.h"
 #include "clang/Basic/AttributeCommonInfo.h"
 #include "clang/Basic/Diagnostic.h"
@@ -116,9 +117,10 @@
 ///
 class ParsedAttr final
 : public AttributeCommonInfo,
-  private llvm::TrailingObjects<
-  ParsedAttr, ArgsUnion, detail::AvailabilityData,
-  detail::TypeTagForDatatypeData, ParsedType, detail::PropertyData> {
+  private llvm::TrailingObjects {
   friend TrailingObjects;
 
   size_t numTrailingObjects(OverloadToken) const { return NumArgs; }
@@ -135,6 +137,7 @@
   size_t numTrailingObjects(OverloadToken) const {
 return IsProperty;
   }
+  size_t numTrailingObjects(OverloadToken) const { return HasAttrPtr; }
 
 private:
   IdentifierInfo *MacroII = nullptr;
@@ -143,7 +146,7 @@
 
   /// The number of expression arguments this attribute has.
   /// The expressions themselves are stored after the object.
-  unsigned NumArgs : 16;
+  unsigned NumArgs : 15;
 
   /// True if already diagnosed as invalid.
   mutable unsigned Invalid : 1;
@@ -166,6 +169,9 @@
   /// True if this has a ParsedType
   unsigned HasParsedType : 1;
 
+  /// True if this has an attribute pointer.
+  unsigned HasAttrPtr : 1;
+
   /// True if the processing cache is valid.
   mutable unsigned HasProcessingCache : 1;
 
@@ -207,7 +213,8 @@
 EllipsisLoc(ellipsisLoc), NumArgs(numArgs), Invalid(false),
 UsedAsTypeAttr(false), IsAvailability(false),
 IsTypeTagForDatatype(false), IsProperty(false), HasParsedType(false),
-HasProcessingCache(false), IsPragmaClangAttribute(false) {
+HasAttrPtr(false), HasProcessingCache(false),
+IsPragmaClangAttribute(false) {
 if (numArgs)
   memcpy(getArgsBuffer(), args, numArgs * sizeof(ArgsUnion));
   }
@@ -224,8 +231,9 @@
 syntaxUsed),
 NumArgs(1), Invalid(false), UsedAsTypeAttr(false), IsAvailability(true),
 IsTypeTagForDatatype(false), IsProperty(false), HasParsedType(false),
-HasProcessingCache(false), IsPragmaClangAttribute(false),
-UnavailableLoc(unavailable), MessageExpr(messageExpr) {
+HasAttrPtr(false), HasProcessingCache(false),
+IsPragmaClangAttribute(false), UnavailableLoc(unavailable),
+MessageExpr(messageExpr) {
 ArgsUnion PVal(Parm);
 memcpy(getArgsBuffer(), , sizeof(ArgsUnion));
 new (getAvailabilityData()) detail::AvailabilityData(
@@ -241,7 +249,7 @@
 syntaxUsed),
 NumArgs(3), Invalid(false), UsedAsTypeAttr(false),
 IsAvailability(false), IsTypeTagForDatatype(false), IsProperty(false),
-HasParsedType(false), HasProcessingCache(false),
+HasParsedType(false), HasAttrPtr(false), HasProcessingCache(false),
 IsPragmaClangAttribute(false) {
 ArgsUnion *Args = getArgsBuffer();
 Args[0] = Parm1;
@@ -258,7 +266,7 @@
 syntaxUsed),
 NumArgs(1), Invalid(false), UsedAsTypeAttr(false),
 IsAvailability(false), IsTypeTagForDatatype(true), IsProperty(false),
-HasParsedType(false), HasProcessingCache(false),
+HasParsedType(false), HasAttrPtr(false), HasProcessingCache(false),
 IsPragmaClangAttribute(false) {
 ArgsUnion PVal(ArgKind);
 memcpy(getArgsBuffer(), , sizeof(ArgsUnion));
@@ -276,7 +284,7 @@
 syntaxUsed),
 NumArgs(0), Invalid(false), UsedAsTypeAttr(false),
 IsAvailability(false), IsTypeTagForDatatype(false), IsProperty(false),
-HasParsedType(true), HasProcessingCache(false),
+HasParsedType(true), HasAttrPtr(false), HasProcessingCache(false),
 IsPragmaClangAttribute(false) {
 new (()) ParsedType(typeArg);
   }
@@ -290,11 +298,26 @@
 syntaxUsed),
 

[PATCH] D75210: [Attr] Allow ParsedAttr to be constructor for any Attribute

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D75210#1898012 , @aaron.ballman 
wrote:

> I think this direction is reasonable to go with, but would prefer to land 
> this with the code actually using this functionality (as there's not really a 
> good way to test it, otherwise).


Agreed. I'll put the other patch as a dependence up soon and this only goes in 
if the other one does.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75210



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

[PATCH] D75210: [Attr] Allow ParsedAttr to be constructor for any Attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I think this direction is reasonable to go with, but would prefer to land this 
with the code actually using this functionality (as there's not really a good 
way to test it, otherwise).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75210



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

[PATCH] D75210: [Attr] Allow ParsedAttr to be constructor for any Attribute

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert marked an inline comment as done.
jdoerfert added a comment.

In D75210#1894370 , @erichkeane wrote:

> I don't see anything glaring here, but I would love some better explanation 
> as to the goal here.  I don't see this used anywhere, the commit message is a 
> bit underwhelming in its reasoning, and there is no test/etc showing why I 
> would want this.


Agreed. The followup "user" patch is going to be put on phab very soon. I did 
rush this one out because I saw D31337  land 
today. Wanted to make sure people see what I hope to achieve here.

The reason for this is `omp begin/end declare variant` which I implement via 
`attribute push/pop`. The idea of both are basically the same, attach some 
information (=Attribute) on all function (definitions) in the scope. I have a 
new function definition matcher and the rest of the logic basically done, some 
overloading work is still missing.




Comment at: clang/include/clang/Sema/ParsedAttr.h:115
 /// 3: __attribute__(( format(printf, 1, 2) )). ParmName/Args/NumArgs all used.
 /// 4: __attribute__(( aligned(16) )). ParmName is unused, Args/Num used.
 ///

erichkeane wrote:
> Can you update this comment?
Will do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75210



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

[PATCH] D75210: [Attr] Allow ParsedAttr to be constructor for any Attribute

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

I don't see anything glaring here, but I would love some better explanation as 
to the goal here.  I don't see this used anywhere, the commit message is a bit 
underwhelming in its reasoning, and there is no test/etc showing why I would 
want this.




Comment at: clang/include/clang/Sema/ParsedAttr.h:115
 /// 3: __attribute__(( format(printf, 1, 2) )). ParmName/Args/NumArgs all used.
 /// 4: __attribute__(( aligned(16) )). ParmName is unused, Args/Num used.
 ///

Can you update this comment?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75210



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

[PATCH] D75210: [Attr] Allow ParsedAttr to be constructor for any Attribute

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert created this revision.
jdoerfert added reviewers: erichkeane, aaron.ballman, rjmccall, john.brawn.
Herald added a subscriber: bollu.
Herald added a project: clang.

ParsedAttr is used for `clang attribute push/pop` a utility that is
reusable in other contexts as well. This allows us to create "fake"
parsed attributes from other attributes, e.g., implicit ones.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75210

Files:
  clang/include/clang/Sema/ParsedAttr.h
  clang/lib/Sema/ParsedAttr.cpp

Index: clang/lib/Sema/ParsedAttr.cpp
===
--- clang/lib/Sema/ParsedAttr.cpp
+++ clang/lib/Sema/ParsedAttr.cpp
@@ -42,10 +42,10 @@
   else if (HasParsedType)
 return totalSizeToAlloc(0, 0, 0, 1, 0);
+detail::PropertyData, Attr *>(0, 0, 0, 1, 0, 0);
   return totalSizeToAlloc(NumArgs, 0, 0, 0, 0);
+  detail::PropertyData, Attr *>(NumArgs, 0, 0, 0, 0, 0);
 }
 
 AttributeFactory::AttributeFactory() {
Index: clang/include/clang/Sema/ParsedAttr.h
===
--- clang/include/clang/Sema/ParsedAttr.h
+++ clang/include/clang/Sema/ParsedAttr.h
@@ -116,9 +116,10 @@
 ///
 class ParsedAttr final
 : public AttributeCommonInfo,
-  private llvm::TrailingObjects<
-  ParsedAttr, ArgsUnion, detail::AvailabilityData,
-  detail::TypeTagForDatatypeData, ParsedType, detail::PropertyData> {
+  private llvm::TrailingObjects {
   friend TrailingObjects;
 
   size_t numTrailingObjects(OverloadToken) const { return NumArgs; }
@@ -135,6 +136,7 @@
   size_t numTrailingObjects(OverloadToken) const {
 return IsProperty;
   }
+  size_t numTrailingObjects(OverloadToken) const { return HasAttrPtr; }
 
 private:
   IdentifierInfo *MacroII = nullptr;
@@ -143,7 +145,7 @@
 
   /// The number of expression arguments this attribute has.
   /// The expressions themselves are stored after the object.
-  unsigned NumArgs : 16;
+  unsigned NumArgs : 15;
 
   /// True if already diagnosed as invalid.
   mutable unsigned Invalid : 1;
@@ -166,6 +168,9 @@
   /// True if this has a ParsedType
   unsigned HasParsedType : 1;
 
+  /// True if this has an attribute pointer.
+  unsigned HasAttrPtr : 1;
+
   /// True if the processing cache is valid.
   mutable unsigned HasProcessingCache : 1;
 
@@ -207,7 +212,8 @@
 EllipsisLoc(ellipsisLoc), NumArgs(numArgs), Invalid(false),
 UsedAsTypeAttr(false), IsAvailability(false),
 IsTypeTagForDatatype(false), IsProperty(false), HasParsedType(false),
-HasProcessingCache(false), IsPragmaClangAttribute(false) {
+HasAttrPtr(false), HasProcessingCache(false),
+IsPragmaClangAttribute(false) {
 if (numArgs)
   memcpy(getArgsBuffer(), args, numArgs * sizeof(ArgsUnion));
   }
@@ -224,8 +230,9 @@
 syntaxUsed),
 NumArgs(1), Invalid(false), UsedAsTypeAttr(false), IsAvailability(true),
 IsTypeTagForDatatype(false), IsProperty(false), HasParsedType(false),
-HasProcessingCache(false), IsPragmaClangAttribute(false),
-UnavailableLoc(unavailable), MessageExpr(messageExpr) {
+HasAttrPtr(false), HasProcessingCache(false),
+IsPragmaClangAttribute(false), UnavailableLoc(unavailable),
+MessageExpr(messageExpr) {
 ArgsUnion PVal(Parm);
 memcpy(getArgsBuffer(), , sizeof(ArgsUnion));
 new (getAvailabilityData()) detail::AvailabilityData(
@@ -241,7 +248,7 @@
 syntaxUsed),
 NumArgs(3), Invalid(false), UsedAsTypeAttr(false),
 IsAvailability(false), IsTypeTagForDatatype(false), IsProperty(false),
-HasParsedType(false), HasProcessingCache(false),
+HasParsedType(false), HasAttrPtr(false), HasProcessingCache(false),
 IsPragmaClangAttribute(false) {
 ArgsUnion *Args = getArgsBuffer();
 Args[0] = Parm1;
@@ -258,7 +265,7 @@
 syntaxUsed),
 NumArgs(1), Invalid(false), UsedAsTypeAttr(false),
 IsAvailability(false), IsTypeTagForDatatype(true), IsProperty(false),
-HasParsedType(false), HasProcessingCache(false),
+HasParsedType(false), HasAttrPtr(false), HasProcessingCache(false),
 IsPragmaClangAttribute(false) {
 ArgsUnion PVal(ArgKind);
 memcpy(getArgsBuffer(), , sizeof(ArgsUnion));
@@ -276,7 +283,7 @@
 syntaxUsed),
 NumArgs(0), Invalid(false), UsedAsTypeAttr(false),
 IsAvailability(false), IsTypeTagForDatatype(false), IsProperty(false),
-HasParsedType(true), HasProcessingCache(false),
+HasParsedType(true), HasAttrPtr(false), HasProcessingCache(false),
 IsPragmaClangAttribute(false) {
 new (()) ParsedType(typeArg);
   }
@@ -290,11 +297,25 @@
 syntaxUsed),
 NumArgs(0), Invalid(false), UsedAsTypeAttr(false),