[PATCH] D99983: Provide TreeTransform::TransformAttr the transformed statement; NFC

2021-04-21 Thread Josh Haberman via Phabricator via cfe-commits 
haberman accepted this revision.
haberman added a comment.
This revision is now accepted and ready to land.

This seems fine to me, but I'll defer to @rsmith for final review.


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

https://reviews.llvm.org/D99983

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

[PATCH] D99983: Provide TreeTransform::TransformAttr the transformed statement; NFC

2021-04-21 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman updated this revision to Diff 339319.
aaron.ballman added a comment.

Now that [[clang::musttail]] has landed, I can show the refactoring I had in 
mind by implementing it myself. WDYT of this approach?


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

https://reviews.llvm.org/D99983

Files:
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/TreeTransform.h

Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -379,8 +379,14 @@
   /// of attribute. Subclasses may override this function to transform
   /// attributed statements using some other mechanism.
   ///
+  /// The \c TransformedStmt parameter points to the substituted statement and
+  /// is non-const explicitly so that transformations of the attribute that
+  /// need to set state on the statement they apply to can do so if needed.
+  /// Note that \c TransformedStmt will be null when transforming type
+  /// attributes.
+  ///
   /// \returns the transformed attribute
-  const Attr *TransformAttr(const Attr *S);
+  const Attr *TransformAttr(const Attr *S, Stmt *TransformedStmt);
 
 /// Transform the specified attribute.
 ///
@@ -388,10 +394,12 @@
 /// spelling to transform expressions stored within the attribute.
 ///
 /// \returns the transformed attribute.
-#define ATTR(X)
-#define PRAGMA_SPELLING_ATTR(X)\
-  const X##Attr *Transform##X##Attr(const X##Attr *R) { return R; }
+#define ATTR(X)\
+  const X##Attr *Transform##X##Attr(const X##Attr *R, Stmt *TransformedStmt) { \
+return R;  \
+  }
 #include "clang/Basic/AttrList.inc"
+#undef ATTR
 
   /// Transform the given expression.
   ///
@@ -6745,7 +6753,8 @@
 
   // oldAttr can be null if we started with a QualType rather than a TypeLoc.
   const Attr *oldAttr = TL.getAttr();
-  const Attr *newAttr = oldAttr ? getDerived().TransformAttr(oldAttr) : nullptr;
+  const Attr *newAttr =
+  oldAttr ? getDerived().TransformAttr(oldAttr, nullptr) : nullptr;
   if (oldAttr && !newAttr)
 return QualType();
 
@@ -7298,19 +7307,20 @@
 }
 
 template 
-const Attr *TreeTransform::TransformAttr(const Attr *R) {
-  if (!R)
-return R;
+const Attr *TreeTransform::TransformAttr(const Attr *A,
+  Stmt *TransformedStmt) {
+  if (!A)
+return A;
 
-  switch (R->getKind()) {
+  switch (A->getKind()) {
 // Transform attributes with a pragma spelling by calling TransformXXXAttr.
-#define ATTR(X)
-#define PRAGMA_SPELLING_ATTR(X)\
+#define ATTR(X)\
   case attr::X:\
-return getDerived().Transform##X##Attr(cast(R));
+return getDerived().Transform##X##Attr(cast(A), TransformedStmt);
 #include "clang/Basic/AttrList.inc"
+#undef ATTR
   default:
-return R;
+return A;
   }
 }
 
@@ -7318,21 +7328,27 @@
 StmtResult
 TreeTransform::TransformAttributedStmt(AttributedStmt *S,
 StmtDiscardKind SDK) {
+  // Transform the attributed statement first so that we can pass the
+  // transformed version in to the attribute handler. This is necessary because
+  // attributes that need to perform semantic checking are more likely to need
+  // the transformed statement than statement semantic checking is likely to
+  // need the transformed attributes. In such a case, the TransformFooAttr()
+  // function can mutate the non-const transformed statement that it is passed.
+  StmtResult SubStmt = getDerived().TransformStmt(S->getSubStmt(), SDK);
+  if (SubStmt.isInvalid())
+return StmtError();
+
   bool AttrsChanged = false;
   SmallVector Attrs;
 
   // Visit attributes and keep track if any are transformed.
   for (const auto *I : S->getAttrs()) {
-const Attr *R = getDerived().TransformAttr(I);
+const Attr *R = getDerived().TransformAttr(I, SubStmt.get());
 AttrsChanged |= (I != R);
 if (R)
   Attrs.push_back(R);
   }
 
-  StmtResult SubStmt = getDerived().TransformStmt(S->getSubStmt(), SDK);
-  if (SubStmt.isInvalid())
-return StmtError();
-
   if (SubStmt.get() == S->getSubStmt() && !AttrsChanged)
 return S;
 
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -1074,7 +1074,10 @@
   NamedDecl *FirstQualifierInScope = nullptr,
   bool AllowInjectedClassName = false);
 
-const LoopHintAttr *Transfor

[PATCH] D99983: Provide TreeTransform::TransformAttr the transformed statement; NFC

2021-04-06 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman created this revision.
aaron.ballman added reviewers: rsmith, haberman.
aaron.ballman requested review of this revision.
Herald added a project: clang.

It is useful statement an attribute is being applied to when performing 
semantic processing of the attribute during template instantiation. This 
functionality is not currently needed by existing attributes, but is 
anticipated to be used by new attributes being worked on.

One design choice with this is whether to transform attributes then the 
attributed statement, or to transform the attributed statement and then the 
attributes. I elected to transform the attributed statement first because it 
seems less likely that the statement transformation would require the 
instanatiated attributes compared to when we transform attributes (those would 
benefit from knowing the fully instantiated statement). This patch passes a 
non-`const Stmt *` to `TransformAttr()` in case the attribute instantiation 
needs to modify the attributed statement for some reason.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99983

Files:
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/TreeTransform.h

Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -379,8 +379,14 @@
   /// of attribute. Subclasses may override this function to transform
   /// attributed statements using some other mechanism.
   ///
+  /// The \c TransformedStmt parameter points to the substituted statement and
+  /// is non-const explicitly so that transformations of the attribute that
+  /// need to set state on the statement they apply to can do so if needed.
+  /// Note that \c TransformedStmt will be null when transforming type
+  /// attributes.
+  ///
   /// \returns the transformed attribute
-  const Attr *TransformAttr(const Attr *S);
+  const Attr *TransformAttr(const Attr *A, Stmt *TransformedStmt);
 
 /// Transform the specified attribute.
 ///
@@ -390,7 +396,9 @@
 /// \returns the transformed attribute.
 #define ATTR(X)
 #define PRAGMA_SPELLING_ATTR(X)\
-  const X##Attr *Transform##X##Attr(const X##Attr *R) { return R; }
+  const X##Attr *Transform##X##Attr(const X##Attr *R, Stmt *TransformedStmt) { \
+return R;  \
+  }
 #include "clang/Basic/AttrList.inc"
 
   /// Transform the given expression.
@@ -6734,7 +6742,8 @@
 
   // oldAttr can be null if we started with a QualType rather than a TypeLoc.
   const Attr *oldAttr = TL.getAttr();
-  const Attr *newAttr = oldAttr ? getDerived().TransformAttr(oldAttr) : nullptr;
+  const Attr *newAttr =
+  oldAttr ? getDerived().TransformAttr(oldAttr, nullptr) : nullptr;
   if (oldAttr && !newAttr)
 return QualType();
 
@@ -7287,19 +7296,20 @@
 }
 
 template 
-const Attr *TreeTransform::TransformAttr(const Attr *R) {
-  if (!R)
-return R;
+const Attr *TreeTransform::TransformAttr(const Attr *A,
+  Stmt *TransformedStmt) {
+  if (!A)
+return A;
 
-  switch (R->getKind()) {
+  switch (A->getKind()) {
 // Transform attributes with a pragma spelling by calling TransformXXXAttr.
 #define ATTR(X)
 #define PRAGMA_SPELLING_ATTR(X)\
   case attr::X:\
-return getDerived().Transform##X##Attr(cast(R));
+return getDerived().Transform##X##Attr(cast(A), TransformedStmt);
 #include "clang/Basic/AttrList.inc"
   default:
-return R;
+return A;
   }
 }
 
@@ -7307,20 +7317,26 @@
 StmtResult
 TreeTransform::TransformAttributedStmt(AttributedStmt *S,
 StmtDiscardKind SDK) {
+  // Transform the attributed statement first so that we can pass the
+  // transformed version in to the attribute handler. This is necessary because
+  // attributes that need to perform semantic checking are more likely to need
+  // the transformed statement than statement semantic checking is likely to
+  // need the transformed attributes. In such a case, the TransformFooAttr()
+  // function can mutate the non-const transformed statement that it is passed.
+  StmtResult SubStmt = getDerived().TransformStmt(S->getSubStmt(), SDK);
+  if (SubStmt.isInvalid())
+return StmtError();
+
   bool AttrsChanged = false;
   SmallVector Attrs;
 
   // Visit attributes and keep track if any are transformed.
   for (const auto *I : S->getAttrs()) {
-const Attr *R = getDerived().TransformAttr(I);
+const Attr *R = getDerived().TransformAttr(I, SubStmt.get());
 AttrsChanged |= (I != R);
 Attrs.push_back(R);
   }
 
-  StmtResult SubStmt = getDerived().TransformStmt(S->getSubStmt(), SDK);
-  if (SubStmt.isInvalid())
-return StmtError();
-
   if (SubStmt.get() == S->getSubStmt() &