Provide TreeTransform::TransformAttr the transformed statement; NFC

[aaron.ballman]

Summary

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.

[aaron.ballman]

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

[haberman]

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

clang/lib/Sema/SemaStmt.cpp

if (!TheDecl->isGnuLocal()) {
     }
   }
   return LS;
 }
 
 StmtResult Sema::BuildAttributedStmt(SourceLocation AttrsLoc,
                                      ArrayRef<const Attr *> Attrs,
                                      Stmt *SubStmt) {
-  // FIXME: this code should move when a planned refactoring around statement
-  // attributes lands.
-  for (const auto *A : Attrs) {
-    if (A->getKind() == attr::MustTail) {
-      if (!checkAndRewriteMustTailAttr(SubStmt, *A)) {
-        return SubStmt;
-      }
-      setFunctionHasMustTail();
-    }
-  }
-
   return AttributedStmt::Create(Context, AttrsLoc, Attrs, SubStmt);
 }
 
 StmtResult Sema::ActOnAttributedStmt(const ParsedAttributesWithRange &Attrs,
                                      Stmt *SubStmt) {
   SmallVector<const Attr *, 1> SemanticAttrs;
   ProcessStmtAttributes(SubStmt, Attrs, SemanticAttrs);
   if (!SemanticAttrs.empty())

clang/lib/Sema/SemaStmtAttr.cpp

if (!CEF.foundCallExpr()) {
     return nullptr;
   }
 
   return ::new (S.Context) NoMergeAttr(S.Context, A);
 }
 
 static Attr *handleMustTailAttr(Sema &S, Stmt *St, const ParsedAttr &A,
                                 SourceRange Range) {
-  // Validation is in Sema::ActOnAttributedStmt().
+  MustTailAttr TmpAttr(S.Context, A);
+  if (!S.checkAndRewriteMustTailAttr(St, TmpAttr))
+    return nullptr;
+  S.setFunctionHasMustTail();
+
   return ::new (S.Context) MustTailAttr(S.Context, A);
 }
 
 static Attr *handleLikely(Sema &S, Stmt *St, const ParsedAttr &A,
                           SourceRange Range) {
 
   if (!S.getLangOpts().CPlusPlus20 && A.isCXX11Attribute() && !A.getScopeName())
     S.Diag(A.getLoc(), diag::ext_cxx20_attr) << A << Range;

clang/lib/Sema/SemaTemplateInstantiate.cpp

public:
 
     TemplateName
     TransformTemplateName(CXXScopeSpec &SS, TemplateName Name,
                           SourceLocation NameLoc,
                           QualType ObjectType = QualType(),
                           NamedDecl *FirstQualifierInScope = nullptr,
                           bool AllowInjectedClassName = false);
 
-    const LoopHintAttr *TransformLoopHintAttr(const LoopHintAttr *LH);
+    const LoopHintAttr *TransformLoopHintAttr(const LoopHintAttr *LH,
+                                              Stmt *TransformedStmt);
+    const MustTailAttr *TransformMustTailAttr(const MustTailAttr *MTA,
+                                              Stmt *TransformedStmt);
 
     ExprResult TransformPredefinedExpr(PredefinedExpr *E);
     ExprResult TransformDeclRefExpr(DeclRefExpr *E);
     ExprResult TransformCXXDefaultArgExpr(CXXDefaultArgExpr *E);
 
     ExprResult TransformTemplateParmRefExpr(DeclRefExpr *E,
                                             NonTypeTemplateParmDecl *D);
     ExprResult TransformSubstNonTypeTemplateParmPackExpr(
if (NTTP->isParameterPack()) {
 
     Arg = getPackSubstitutedTemplateArgument(getSema(), Arg);
   }
 
   return transformNonTypeTemplateParmRef(NTTP, E->getLocation(), Arg);
 }
 
 const LoopHintAttr *
-TemplateInstantiator::TransformLoopHintAttr(const LoopHintAttr *LH) {
+TemplateInstantiator::TransformLoopHintAttr(const LoopHintAttr *LH, Stmt *) {
   Expr *TransformedExpr = getDerived().TransformExpr(LH->getValue()).get();
 
   if (TransformedExpr == LH->getValue())
     return LH;
 
   // Generate error if there is a problem with the value.
   if (getSema().CheckLoopHintExpr(TransformedExpr, LH->getLocation()))
     return LH;
 
   // Create new LoopHintValueAttr with integral expression in place of the
   // non-type template parameter.
   return LoopHintAttr::CreateImplicit(getSema().Context, LH->getOption(),
                                       LH->getState(), TransformedExpr, *LH);
 }
 
+const MustTailAttr *
+TemplateInstantiator::TransformMustTailAttr(const MustTailAttr *MTA,
+                                            Stmt *TransformedStmt) {
+  // The attribute itself does not have arguments that need to be transformed,
+  // but the transformed return statement may require additional diagnostic
+  // checking and marking.
+  if (!getSema().checkAndRewriteMustTailAttr(TransformedStmt, *MTA))
+    return nullptr;
+  getSema().setFunctionHasMustTail();
+  return MTA;
+}
+
 ExprResult TemplateInstantiator::transformNonTypeTemplateParmRef(
                                                  NonTypeTemplateParmDecl *parm,
                                                  SourceLocation loc,
                                                  TemplateArgument arg) {
   ExprResult result;
 
   // Determine the substituted parameter type. We can usually infer this from
   // the template argument, but not always.

clang/lib/Sema/TreeTransform.h

public:
 
   /// Transform the given attribute.
   ///
   /// By default, this routine transforms a statement by delegating to the
   /// appropriate TransformXXXAttr function to transform a specific kind
   /// 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.
 ///
 /// Subclasses should override the transformation of attributes with a pragma
 /// 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.
   ///
   /// By default, this routine transforms an expression by delegating to the
   /// appropriate TransformXXXExpr function to build a new expression.
   /// Subclasses may override this function to transform expressions using some
   /// other mechanism.
   ///
QualType TreeTransform<Derived>::TransformAttributedType(
                                                 AttributedTypeLoc TL) {
   const AttributedType *oldType = TL.getTypePtr();
   QualType modifiedType = getDerived().TransformType(TLB, TL.getModifiedLoc());
   if (modifiedType.isNull())
     return QualType();
 
   // 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();
 
   QualType result = TL.getType();
 
   // FIXME: dependent operand expressions?
   if (getDerived().AlwaysRebuild() ||
       modifiedType != oldType->getModifiedType()) {
TreeTransform<Derived>::TransformLabelStmt(LabelStmt *S, StmtDiscardKind SDK) {
 
   // FIXME: Pass the real colon location in.
   return getDerived().RebuildLabelStmt(S->getIdentLoc(),
                                        cast<LabelDecl>(LD), SourceLocation(),
                                        SubStmt.get());
 }
 
 template <typename Derived>
-const Attr *TreeTransform<Derived>::TransformAttr(const Attr *R) {
-  if (!R)
-    return R;
+const Attr *TreeTransform<Derived>::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<X##Attr>(R));
+    return getDerived().Transform##X##Attr(cast<X##Attr>(A), TransformedStmt);
 #include "clang/Basic/AttrList.inc"
+#undef ATTR
   default:
-    return R;
+    return A;
   }
 }
 
 template <typename Derived>
 StmtResult
 TreeTransform<Derived>::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<const Attr *, 1> 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;
 
   // If transforming the attributes failed for all of the attributes in the
   // statement, don't make an AttributedStmt without attributes.
   if (Attrs.empty())
     return SubStmt;