[PATCH] Support constant expressions, including non-type template parameters, in pragma loop hints

Aaron Ballman aaron at aaronballman.com
Wed Jul 30 10:51:23 PDT 2014


On Tue, Jul 29, 2014 at 8:47 PM, Tyler Nowicki <tnowicki at apple.com> wrote:
> And the last piece of the constant expression patch is to parse multiple
> tokens, add the Expr variable to the loop hint attribute, handle template
> instantiation, get the value at codegen, and of course tests.
>
> Apply the attached patch with -p1 on top of loop_hint_attr_state-svn.patch.
>
> Thanks again for reviewing these while you are on vacation!

Just got off of vacation today, so now back to my regularly-scheduled
nitpicking. ;-)

Comments below.

> diff --git a/include/clang/AST/Attr.h b/include/clang/AST/Attr.h
> index fc48816..787843e 100644
> --- a/include/clang/AST/Attr.h
> +++ b/include/clang/AST/Attr.h
> @@ -16,6 +16,7 @@
>
>  #include "clang/AST/AttrIterator.h"
>  #include "clang/AST/Decl.h"
> +#include "clang/AST/Expr.h"

This include should not be required here.

>  #include "clang/AST/Type.h"
>  #include "clang/Basic/AttrKinds.h"
>  #include "clang/Basic/LLVM.h"
> diff --git a/include/clang/Basic/Attr.td b/include/clang/Basic/Attr.td
> index 98bc216..e5779a7 100644
> --- a/include/clang/Basic/Attr.td
> +++ b/include/clang/Basic/Attr.td
> @@ -1809,7 +1809,7 @@ def LoopHint : Attr {
>                EnumArgument<"State", "StateType",
>                             ["default", "enable", "disable"],
>                             ["Default", "Enable", "Disable"]>,
> -              DefaultIntArgument<"Value", 1>];
> +              ExprArgument<"Value">];
>
>    let AdditionalMembers = [{
>    static const char *getOptionName(int Option) {
> @@ -1849,7 +1849,7 @@ def LoopHint : Attr {
>      OS << "(";
>      if (option == VectorizeWidth || option == InterleaveCount ||
>          option == UnrollCount)
> -      OS << value;
> +      value->printPretty(OS, nullptr, Policy);
>      else if (state == Default)
>        return "";
>      else if (state == Enable)
> diff --git a/include/clang/Basic/DiagnosticParseKinds.td b/include/clang/Basic/DiagnosticParseKinds.td
> index 6b73b3b..c481832 100644
> --- a/include/clang/Basic/DiagnosticParseKinds.td
> +++ b/include/clang/Basic/DiagnosticParseKinds.td
> @@ -925,8 +925,8 @@ def err_pragma_invalid_keyword : Error<
>  def err_pragma_loop_invalid_option : Error<
>    "%select{invalid|missing}0 option%select{ %1|}0; expected vectorize, "
>    "vectorize_width, interleave, interleave_count, unroll, or unroll_count">;
> -def err_pragma_loop_numeric_value : Error<
> -  "invalid argument; expected a positive integer value">;
> +def err_pragma_loop_invalid_value : Error<
> +  "%select{invalid|missing}0 argument; expected a positive 32-bit integer value">;
>
>  // Pragma unroll support.
>  def warn_pragma_unroll_cuda_value_in_parens : Warning<
> diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
> index 87032b8..2a9dd06 100644
> --- a/include/clang/Basic/DiagnosticSemaKinds.td
> +++ b/include/clang/Basic/DiagnosticSemaKinds.td
> @@ -539,8 +539,10 @@ def err_pragma_pop_visibility_mismatch : Error<
>    "#pragma visibility pop with no matching #pragma visibility push">;
>  def note_surrounding_namespace_starts_here : Note<
>    "surrounding namespace with visibility attribute starts here">;
> -def err_pragma_loop_invalid_value : Error<
> -  "invalid argument; expected a positive integer value">;
> +def err_pragma_loop_invalid_argument : Error<
> +  "invalid argument %0; expected a positive 32-bit integer value">;
> +def err_pragma_loop_invalid_expression : Error<
> +  "invalid expression; expected an integer constant expression">;
>  def err_pragma_loop_compatibility : Error<
>    "%select{incompatible|duplicate}0 directives '%1' and '%2'">;
>  def err_pragma_loop_precedes_nonloop : Error<
> diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
> index 477a03c..9ebaa9a 100644
> --- a/include/clang/Sema/Sema.h
> +++ b/include/clang/Sema/Sema.h
> @@ -3446,6 +3446,9 @@ public:
>                                   PredefinedExpr::IdentType IT);
>    ExprResult ActOnPredefinedExpr(SourceLocation Loc, tok::TokenKind Kind);
>    ExprResult ActOnIntegerConstant(SourceLocation Loc, uint64_t Val);
> +
> +  bool CheckLoopHintExpr(Expr *E, SourceLocation Loc, bool AllowValueDependent);
> +
>    ExprResult ActOnNumericConstant(const Token &Tok, Scope *UDLScope = nullptr);
>    ExprResult ActOnCharacterConstant(const Token &Tok,
>                                      Scope *UDLScope = nullptr);
> diff --git a/lib/CodeGen/CGStmt.cpp b/lib/CodeGen/CGStmt.cpp
> index d5db35b..d2ff21d 100644
> --- a/lib/CodeGen/CGStmt.cpp
> +++ b/lib/CodeGen/CGStmt.cpp
> @@ -589,8 +589,6 @@ void CodeGenFunction::EmitCondBrHints(llvm::LLVMContext &Context,
>
>      LoopHintAttr::OptionType Option = LH->getOption();
>      int State = LH->getState();
> -    int ValueInt = LH->getValue();
> -
>      const char *MetadataName;
>      switch (Option) {
>      case LoopHintAttr::Vectorize:
> @@ -610,6 +608,15 @@ void CodeGenFunction::EmitCondBrHints(llvm::LLVMContext &Context,
>        MetadataName = "llvm.loop.unroll.count";
>        break;
>      }
> +
> +    Expr *ValueExpr = LH->getValue();
> +    int ValueInt = 1;
> +    if (ValueExpr) {
> +      llvm::APSInt ValueAPS =
> +          ValueExpr->EvaluateKnownConstInt(CGM.getContext());
> +      ValueInt = ValueAPS.getSExtValue();

Typecast required since this is a 64-to-32-bit truncation.

> +    }
> +
>      llvm::Value *Value;
>      llvm::MDString *Name;
>      switch (Option) {
> diff --git a/lib/Parse/ParsePragma.cpp b/lib/Parse/ParsePragma.cpp
> index 9322384..b22be9f 100644
> --- a/lib/Parse/ParsePragma.cpp
> +++ b/lib/Parse/ParsePragma.cpp
> @@ -718,15 +718,28 @@ bool Parser::HandlePragmaMSInitSeg(StringRef PragmaName,
>  struct PragmaLoopHintInfo {
>    Token PragmaName;
>    Token Option;
> -  Token Value;
> -  bool HasValue;
> +  Token *Args;
> +  size_t ArgSize;

These aren't just arguments to the loop hint, they're a series of
tokens that need further processing. Perhaps Toks instead?

> +  PragmaLoopHintInfo() : Args(nullptr), ArgSize(0) {}
>  };
>
> +std::string PragmaLoopHintString(Token PragmaName, Token Option) {

Function should be marked static.

> +  std::string PragmaString;
> +  if (PragmaName.getIdentifierInfo()->getName() == "loop") {
> +    PragmaString = "clang loop ";
> +    PragmaString += Option.getIdentifierInfo()->getName();
> +  } else {
> +    assert(PragmaName.getIdentifierInfo()->getName() == "unroll" &&
> +           "Unexpected pragma name");
> +    PragmaString = "unroll";
> +  }
> +  return PragmaString;
> +}
> +
>  bool Parser::HandlePragmaLoopHint(LoopHint &Hint) {
>    assert(Tok.is(tok::annot_pragma_loop_hint));
>    PragmaLoopHintInfo *Info =
>        static_cast<PragmaLoopHintInfo *>(Tok.getAnnotationValue());
> -  ConsumeToken(); // The annotation token.
>
>    IdentifierInfo *PragmaNameInfo = Info->PragmaName.getIdentifierInfo();
>    Hint.PragmaNameLoc = IdentifierLoc::create(
> @@ -736,13 +749,22 @@ bool Parser::HandlePragmaLoopHint(LoopHint &Hint) {
>    Hint.OptionLoc = IdentifierLoc::create(
>        Actions.Context, Info->Option.getLocation(), OptionInfo);
>
> +  Token *Args = Info->Args;
> +  size_t ArgSize = Info->ArgSize;
> +
>    bool PragmaUnroll = PragmaNameInfo->getName() == "unroll";
>    bool PragmaNoUnroll = PragmaNameInfo->getName() == "nounroll";
> -  if (!Info->HasValue && (PragmaUnroll | PragmaNoUnroll)) {
> +  if (ArgSize == 0 && (PragmaUnroll | PragmaNoUnroll)) {
> +    ConsumeToken(); // The annotation token.
>      Hint.Range = SourceRange(Info->PragmaName.getLocation());
>      return true;
>    }
>
> +  // The constant expression is always followed by an eof token, which increases
> +  // the ArgSize by 1.
> +  assert(ArgSize > 0 &&
> +         "LoopHintInfo::Values must contain at least one token.");

Assert text doesn't match reality (LoopHintInfo::Values should be
PragmaLoopHintInfo::Args/Toks/Whatever).

> +
>    bool StateOption = false;
>    if (OptionInfo) // Pragma unroll does not specify an option.
>      StateOption = llvm::StringSwitch<bool>(OptionInfo->getName())
> @@ -751,32 +773,62 @@ bool Parser::HandlePragmaLoopHint(LoopHint &Hint) {
>                        .Case("unroll", true)
>                        .Default(false);
>
> +  // Verify loop hint has an argument.
> +  if (Args[0].is(tok::eof)) {
> +    ConsumeToken(); // The annotation token.
> +    Diag(Args[0].getLocation(), StateOption
> +                                    ? diag::err_pragma_invalid_keyword
> +                                    : diag::err_pragma_loop_invalid_value)
> +        << /*MissingArgument=*/true << /*FullKeyword=*/PragmaUnroll;
> +    return false;
> +  }
> +
>    // Validate the argument.
>    if (StateOption) {
> +    ConsumeToken(); // The annotation token.
>      bool OptionUnroll = OptionInfo->isStr("unroll");
> -    SourceLocation StateLoc = Info->Value.getLocation();
> -    IdentifierInfo *StateInfo = Info->Value.getIdentifierInfo();
> +    SourceLocation StateLoc = Args[0].getLocation();
> +    IdentifierInfo *StateInfo = Args[0].getIdentifierInfo();
>      if (!StateInfo || ((OptionUnroll ? !StateInfo->isStr("full")
>                                       : !StateInfo->isStr("enable")) &&
>                         !StateInfo->isStr("disable"))) {
> -      Diag(StateLoc, diag::err_pragma_invalid_keyword)
> -          << /*MissingArgument=*/false << /*FullKeyword=*/OptionUnroll;
> +      Diag(Args[0].getLocation(), diag::err_pragma_invalid_keyword)
> +          << /*MissingArgument=*/(ArgSize == 1) << /*FullKeyword=*/OptionUnroll;
>        return false;
>      }
> +    if (ArgSize > 2)
> +      Diag(Tok.getLocation(), diag::warn_pragma_extra_tokens_at_eol)
> +          << PragmaLoopHintString(Info->PragmaName, Info->Option);
>      Hint.StateLoc = IdentifierLoc::create(Actions.Context, StateLoc, StateInfo);
>    } else {
> -    // FIXME: We should allow non-type template parameters for the loop hint
> -    // value. See bug report #19610
> -    if (Info->Value.is(tok::numeric_constant))
> -      Hint.ValueExpr = Actions.ActOnNumericConstant(Info->Value).get();
> -    else {
> -      Diag(Info->Value.getLocation(), diag::err_pragma_loop_numeric_value);
> -      return false;
> +    // Enter constant expression including eof terminator into token stream.
> +    PP.EnterTokenStream(Args, ArgSize, /*DisableMacroExpansion=*/false,
> +                        /*OwnsTokens=*/false);
> +    ConsumeToken(); // The annotation token.
> +
> +    ExprResult R = ParseConstantExpression();
> +
> +    // Tokens following an error in an ill-formed constant expression will
> +    // remain in the token stream and must be removed.
> +    if (Tok.isNot(tok::eof)) {
> +      Diag(Tok.getLocation(), diag::warn_pragma_extra_tokens_at_eol)
> +          << PragmaLoopHintString(Info->PragmaName, Info->Option);
> +      while (Tok.isNot(tok::eof))
> +        ConsumeToken();
>      }
> +
> +    ConsumeToken(); // Consume the constant expression eof terminator.
> +
> +    if (Actions.CheckLoopHintExpr(R.get(), Args[0].getLocation(),
> +                                  /*AllowValueDependent=*/true))
> +      return false;
> +
> +    // Argument is a constant expression with an integer type.
> +    Hint.ValueExpr = R.get();
>    }
>
> -  Hint.Range =
> -      SourceRange(Info->PragmaName.getLocation(), Info->Value.getLocation());
> +  Hint.Range = SourceRange(Info->PragmaName.getLocation(),
> +                           Info->Args[ArgSize - 1].getLocation());
>    return true;
>  }
>
> @@ -1782,32 +1834,13 @@ void PragmaOptimizeHandler::HandlePragma(Preprocessor &PP,
>  static bool ParseLoopHintValue(Preprocessor &PP, Token &Tok, Token PragmaName,
>                                 Token Option, bool ValueInParens,
>                                 PragmaLoopHintInfo &Info) {
> -  if (ValueInParens) {
> -    if (Tok.is(tok::r_paren)) {
> -      // Nothing between the parentheses.
> -      std::string PragmaString;
> -      if (PragmaName.getIdentifierInfo()->getName() == "loop") {
> -        PragmaString = "clang loop ";
> -        PragmaString += Option.getIdentifierInfo()->getName();
> -      } else {
> -        assert(PragmaName.getIdentifierInfo()->getName() == "unroll" &&
> -               "Unexpected pragma name");
> -        PragmaString = "unroll";
> -      }
> -      // Don't try to emit what the pragma is expecting with the diagnostic
> -      // because the logic is non-trivial and we give expected values in sema
> -      // diagnostics if an invalid argument is given.  Here, just note that the
> -      // pragma is missing an argument.
> -      PP.Diag(Tok.getLocation(), diag::err_pragma_missing_argument)
> -          << PragmaString << /*Expected=*/false;
> -      return true;
> -    }
> +  SmallVector<Token, 1> ValueList;
> +  // Read constant expression.
> +  while (Tok.isNot(tok::r_paren) && Tok.isNot(tok::eod)) {
> +    ValueList.push_back(Tok);
> +    PP.Lex(Tok);
>    }
>
> -  // FIXME: Value should be stored and parsed as a constant expression.
> -  Token Value = Tok;
> -  PP.Lex(Tok);
> -
>    if (ValueInParens) {
>      // Read ')'
>      if (Tok.isNot(tok::r_paren)) {
> @@ -1817,10 +1850,20 @@ static bool ParseLoopHintValue(Preprocessor &PP, Token &Tok, Token PragmaName,
>      PP.Lex(Tok);
>    }
>
> +  Token EoF;
> +  EoF.startToken();
> +  EoF.setKind(tok::eof);
> +  EoF.setLocation(Tok.getLocation());
> +  ValueList.push_back(EoF); // Terminates expression for parsing.
> +
> +  Token *TokenArray = (Token *)PP.getPreprocessorAllocator().Allocate(
> +      ValueList.size() * sizeof(Token), llvm::alignOf<Token>());
> +  std::copy(ValueList.begin(), ValueList.end(), TokenArray);
> +  Info.Args = TokenArray;
> +  Info.ArgSize = ValueList.size();
> +
>    Info.PragmaName = PragmaName;
>    Info.Option = Option;
> -  Info.Value = Value;
> -  Info.HasValue = true;
>    return false;
>  }
>
> @@ -1961,7 +2004,6 @@ void PragmaUnrollHintHandler::HandlePragma(Preprocessor &PP,
>    if (Tok.is(tok::eod)) {
>      // nounroll or unroll pragma without an argument.
>      Info->PragmaName = PragmaName;
> -    Info->HasValue = false;
>    } else if (PragmaName.getIdentifierInfo()->getName() == "nounroll") {
>      PP.Diag(Tok.getLocation(), diag::warn_pragma_extra_tokens_at_eol)
>          << "nounroll";
> @@ -1980,7 +2022,7 @@ void PragmaUnrollHintHandler::HandlePragma(Preprocessor &PP,
>      // In CUDA, the argument to '#pragma unroll' should not be contained in
>      // parentheses.
>      if (PP.getLangOpts().CUDA && ValueInParens)
> -      PP.Diag(Info->Value.getLocation(),
> +      PP.Diag(Info->Args[0].getLocation(),
>                diag::warn_pragma_unroll_cuda_value_in_parens);
>
>      if (Tok.isNot(tok::eod)) {
> diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
> index ea5ad77..0c0fc54 100644
> --- a/lib/Sema/SemaExpr.cpp
> +++ b/lib/Sema/SemaExpr.cpp
> @@ -42,6 +42,7 @@
>  #include "clang/Sema/ScopeInfo.h"
>  #include "clang/Sema/SemaFixItUtils.h"
>  #include "clang/Sema/Template.h"
> +#include "llvm/Support/raw_ostream.h"
>  using namespace clang;
>  using namespace sema;
>
> @@ -3045,6 +3046,45 @@ static Expr *BuildFloatingLiteral(Sema &S, NumericLiteralParser &Literal,
>    return FloatingLiteral::Create(S.Context, Val, isExact, Ty, Loc);
>  }
>
> +bool Sema::CheckLoopHintExpr(Expr *E, SourceLocation Loc,
> +                             bool AllowValueDependent) {
> +  if (!E) {
> +    Diag(Loc, diag::err_pragma_loop_invalid_expression);
> +    return true;
> +  }
> +
> +  if (E->isValueDependent()) {
> +    if (AllowValueDependent)
> +      return false;
> +    Diag(E->getExprLoc(), diag::err_pragma_loop_invalid_expression);
> +    return true;
> +  }
> +
> +  QualType QT = E->getType();
> +  if (!QT->isIntegerType() || QT->isBooleanType() || QT->isCharType()) {
> +    Diag(E->getExprLoc(), diag::err_pragma_loop_invalid_argument) << QT;
> +    return true;
> +  }
> +
> +  llvm::APSInt ValueAPS;
> +  ExprResult R = VerifyIntegerConstantExpression(
> +      E, &ValueAPS, diag::err_pragma_loop_invalid_expression);
> +
> +  if (!R.isUsable())
> +    return true;
> +
> +  if (!ValueAPS.isStrictlyPositive() || ValueAPS.getBitWidth() > 32) {
> +    std::string Value;
> +    llvm::raw_string_ostream OS(Value);
> +    OS << '\'' << ValueAPS << '\'';
> +
> +    Diag(E->getExprLoc(), diag::err_pragma_loop_invalid_argument) << OS.str();

Why not simply use APSInt::toString instead of doing the
raw_string_ostream dance?

> +    return true;
> +  }
> +
> +  return false;
> +}
> +
>  ExprResult Sema::ActOnNumericConstant(const Token &Tok, Scope *UDLScope) {
>    // Fast path for a single digit (which is quite common).  A single digit
>    // cannot have a trigraph, escaped newline, radix prefix, or suffix.
> diff --git a/lib/Sema/SemaStmtAttr.cpp b/lib/Sema/SemaStmtAttr.cpp
> index 1da7d37..ed23a32 100644
> --- a/lib/Sema/SemaStmtAttr.cpp
> +++ b/lib/Sema/SemaStmtAttr.cpp
> @@ -18,6 +18,7 @@
>  #include "clang/Sema/Lookup.h"
>  #include "clang/Sema/LoopHint.h"
>  #include "clang/Sema/ScopeInfo.h"
> +#include "clang/Sema/Sema.h"

This include is not required (SemaInternal.h already includes Sema.h).

>  #include "llvm/ADT/StringExtras.h"
>
>  using namespace clang;
> @@ -88,21 +89,16 @@ static Attr *handleLoopHintAttr(Sema &S, Stmt *St, const AttributeList &A,
>      Spelling = LoopHintAttr::Pragma_clang_loop;
>    }
>
> -  int ValueInt = 1;
>    LoopHintAttr::StateType State = LoopHintAttr::Default;
>    if (PragmaNoUnroll) {
>      State = LoopHintAttr::Disable;
>    } else if (Option == LoopHintAttr::VectorizeWidth ||
>               Option == LoopHintAttr::InterleaveCount ||
>               Option == LoopHintAttr::UnrollCount) {
> -    // FIXME: We should support template parameters for the loop hint value.
> -    // See bug report #19610.
> -    llvm::APSInt ValueAPS;
> -    if (!ValueExpr || !ValueExpr->isIntegerConstantExpr(ValueAPS, S.Context) ||
> -        (ValueInt = ValueAPS.getSExtValue()) < 1) {
> -      S.Diag(A.getRange().getEnd(), diag::err_pragma_loop_invalid_value);
> +    assert(ValueExpr && "Attribute must have a valid value expression.");
> +    if (S.CheckLoopHintExpr(ValueExpr, St->getLocStart(),
> +                            /*AllowAllowValueDependent=*/true))
>        return nullptr;
> -    }
>    } else if (Option == LoopHintAttr::Vectorize ||
>               Option == LoopHintAttr::Interleave ||
>               Option == LoopHintAttr::Unroll) {
> @@ -117,7 +113,7 @@ static Attr *handleLoopHintAttr(Sema &S, Stmt *St, const AttributeList &A,
>    }
>
>    return LoopHintAttr::CreateImplicit(S.Context, Spelling, Option, State,
> -                                      ValueInt, A.getRange());
> +                                      ValueExpr, A.getRange());
>  }
>
>  static void
> diff --git a/lib/Sema/SemaTemplateInstantiate.cpp b/lib/Sema/SemaTemplateInstantiate.cpp
> index f447804..99c128a 100644
> --- a/lib/Sema/SemaTemplateInstantiate.cpp
> +++ b/lib/Sema/SemaTemplateInstantiate.cpp
> @@ -767,6 +767,8 @@ namespace {
>                            QualType ObjectType = QualType(),
>                            NamedDecl *FirstQualifierInScope = nullptr);
>
> +    const LoopHintAttr *TransformLoopHintAttr(const LoopHintAttr *LH);
> +
>      ExprResult TransformPredefinedExpr(PredefinedExpr *E);
>      ExprResult TransformDeclRefExpr(DeclRefExpr *E);
>      ExprResult TransformCXXDefaultArgExpr(CXXDefaultArgExpr *E);
> @@ -1127,10 +1129,27 @@ TemplateInstantiator::TransformTemplateParmRefExpr(DeclRefExpr *E,
>    return transformNonTypeTemplateParmRef(NTTP, E->getLocation(), Arg);
>  }
>
> +const LoopHintAttr *
> +TemplateInstantiator::TransformLoopHintAttr(const LoopHintAttr *LH) {
> +  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(),
> +                                  /*AllowValueDependent=*/false))
> +    return LH;
> +
> +  // Create new LoopHintValueAttr with integral expression in place of the
> +  // non-type template parameter.
> +  return LoopHintAttr::CreateImplicit(
> +      getSema().Context, LH->getSemanticSpelling(), LH->getOption(),
> +      LH->getState(), TransformedExpr, LH->getRange());
> +}
> +
>  ExprResult TemplateInstantiator::transformNonTypeTemplateParmRef(
> -                                                 NonTypeTemplateParmDecl *parm,
> -                                                 SourceLocation loc,
> -                                                 TemplateArgument arg) {
> +    NonTypeTemplateParmDecl *parm, SourceLocation loc, TemplateArgument arg) {

This looks like a drive-by that shouldn't be included in this patch.

>    ExprResult result;
>    QualType type;
>
> diff --git a/lib/Sema/TreeTransform.h b/lib/Sema/TreeTransform.h
> index c1a7a48..a83c20c 100644
> --- a/lib/Sema/TreeTransform.h
> +++ b/lib/Sema/TreeTransform.h
> @@ -327,6 +327,27 @@ public:
>    /// \returns the transformed OpenMP clause.
>    OMPClause *TransformOMPClause(OMPClause *S);
>
> +  /// \brief 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.
> +  ///
> +  /// \returns the transformed attribute
> +  const Attr *TransformAttr(const Attr *S);
> +
> +/// \brief 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; }
> +#include "clang/Basic/AttrList.inc"
> +
>    /// \brief Transform the given expression.
>    ///
>    /// By default, this routine transforms an expression by delegating to the
> @@ -5519,26 +5540,48 @@ TreeTransform<Derived>::TransformLabelStmt(LabelStmt *S) {
>    if (!LD)
>      return StmtError();
>
> -
>    // FIXME: Pass the real colon location in.
> -  return getDerived().RebuildLabelStmt(S->getIdentLoc(),
> -                                       cast<LabelDecl>(LD), SourceLocation(),
> -                                       SubStmt.get());
> +  return getDerived().RebuildLabelStmt(S->getIdentLoc(), cast<LabelDecl>(LD),
> +                                       SourceLocation(), SubStmt.get());
>  }
>
> -template<typename Derived>
> -StmtResult
> -TreeTransform<Derived>::TransformAttributedStmt(AttributedStmt *S) {
> +template <typename Derived>
> +const Attr *TreeTransform<Derived>::TransformAttr(const Attr *R) {
> +  if (!R)
> +    return R;
> +
> +  switch (R->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<X##Attr>(R));
> +#include "clang/Basic/AttrList.inc"
> +  default:
> +    return R;
> +  }
> +}
> +
> +template <typename Derived>
> +StmtResult TreeTransform<Derived>::TransformAttributedStmt(AttributedStmt *S) {
> +  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);
> +    AttrsChanged |= (I != R);
> +    Attrs.push_back(R);
> +  }
> +
>    StmtResult SubStmt = getDerived().TransformStmt(S->getSubStmt());
>    if (SubStmt.isInvalid())
>      return StmtError();
>
> -  // TODO: transform attributes
> -  if (SubStmt.get() == S->getSubStmt() /* && attrs are the same */)
> +  if (SubStmt.get() == S->getSubStmt() && !AttrsChanged)
>      return S;
>
> -  return getDerived().RebuildAttributedStmt(S->getAttrLoc(),
> -                                            S->getAttrs(),
> +  return getDerived().RebuildAttributedStmt(S->getAttrLoc(), Attrs,
>                                              SubStmt.get());
>  }
>
> diff --git a/test/CodeGen/pragma-loop.cpp b/test/CodeGen/pragma-loop.cpp
> index b75c7ee..87f2d06 100644
> --- a/test/CodeGen/pragma-loop.cpp
> +++ b/test/CodeGen/pragma-loop.cpp

This file should actually be in CodeGenCXX (especially since it now
has templates, etc). I hadn't noticed this before, sorry!

> @@ -28,11 +28,15 @@ void do_test(int *List, int Length) {
>    } while (i < Length);
>  }
>
> +// Test for scoped enumerations.
> +enum Tuner { Interleave = 4,
> +             Unroll = 8 };
> +
>  // Verify for loop is recognized after sequence of pragma clang loop directives.
>  void for_test(int *List, int Length) {
>  #pragma clang loop interleave(enable)
> -#pragma clang loop interleave_count(4)
> -#pragma clang loop unroll_count(8)
> +#pragma clang loop interleave_count(Tuner::Interleave)
> +#pragma clang loop unroll_count(Tuner::Unroll)
>    for (int i = 0; i < Length; i++) {
>      // CHECK: br i1 {{.*}}, label {{.*}}, label {{.*}}, !llvm.loop ![[LOOP_3:.*]]
>      List[i] = i * 2;
> @@ -74,28 +78,68 @@ void for_define_test(int *List, int Length, int Value) {
>    }
>  }
>
> +// Verify constant expressions are handled correctly.
> +void for_contant_expression_test(int *List, int Length) {
> +#pragma clang loop vectorize_width(1 + 4)
> +  for (int i = 0; i < Length; i++) {
> +    // CHECK: br i1 {{.*}}, label {{.*}}, label {{.*}}, !llvm.loop ![[LOOP_7:.*]]
> +    List[i] = i;
> +  }
> +
> +#pragma clang loop vectorize_width(3 + VECWIDTH)
> +  for (int i = 0; i < Length; i++) {
> +    // CHECK: br i1 {{.*}}, label {{.*}}, label {{.*}}, !llvm.loop ![[LOOP_8:.*]]
> +    List[i] += i;
> +  }
> +}
> +
>  // Verify metadata is generated when template is used.
>  template <typename A>
>  void for_template_test(A *List, int Length, A Value) {
> -
>  #pragma clang loop vectorize_width(8) interleave_count(8) unroll_count(8)
>    for (int i = 0; i < Length; i++) {
> -    // CHECK: br i1 {{.*}}, label {{.*}}, label {{.*}}, !llvm.loop ![[LOOP_7:.*]]
> +    // CHECK: br i1 {{.*}}, label {{.*}}, label {{.*}}, !llvm.loop ![[LOOP_9:.*]]
>      List[i] = i * Value;
>    }
>  }
>
>  // Verify define is resolved correctly when template is used.
> -template <typename A>
> +template <typename A, typename T>
>  void for_template_define_test(A *List, int Length, A Value) {
> -#pragma clang loop vectorize_width(VECWIDTH) interleave_count(INTCOUNT)
> -#pragma clang loop unroll_count(UNROLLCOUNT)
> +  const T VWidth = VECWIDTH;
> +  const T ICount = INTCOUNT;
> +  const T UCount = UNROLLCOUNT;
> +#pragma clang loop vectorize_width(VWidth) interleave_count(ICount)
> +#pragma clang loop unroll_count(UCount)
>    for (int i = 0; i < Length; i++) {
> -    // CHECK: br i1 {{.*}}, label {{.*}}, label {{.*}}, !llvm.loop ![[LOOP_8:.*]]
> +    // CHECK: br i1 {{.*}}, label {{.*}}, label {{.*}}, !llvm.loop ![[LOOP_10:.*]]
>      List[i] = i * Value;
>    }
>  }
>
> +// Verify templates and constant expressions are handled correctly.
> +template <typename A, int V, int I, int U>
> +void for_template_constant_expression_test(A *List, int Length) {
> +#pragma clang loop vectorize_width(V) interleave_count(I) unroll_count(U)
> +  for (int i = 0; i < Length; i++) {
> +    // CHECK: br i1 {{.*}}, label {{.*}}, label {{.*}}, !llvm.loop ![[LOOP_11:.*]]
> +    List[i] = i;
> +  }
> +
> +#pragma clang loop vectorize_width(V * 2 + VECWIDTH) interleave_count(I * 2 + INTCOUNT) unroll_count(U * 2 + UNROLLCOUNT)
> +  for (int i = 0; i < Length; i++) {
> +    // CHECK: br i1 {{.*}}, label {{.*}}, label {{.*}}, !llvm.loop ![[LOOP_12:.*]]
> +    List[i] += i;
> +  }
> +
> +  const int Scale = 4;
> +#pragma clang loop vectorize_width(Scale *V) interleave_count(Scale *I) unroll_count(Scale *U)
> +  for (int i = 0; i < Length; i++) {
> +    // CHECK: br i1 {{.*}}, label {{.*}}, label {{.*}}, !llvm.loop ![[LOOP_13:.*]]
> +    List[i] += i;
> +  }
> +}
> +
>  #undef VECWIDTH
>  #undef INTCOUNT
>  #undef UNROLLCOUNT
> @@ -105,7 +149,8 @@ void template_test(double *List, int Length) {
>    double Value = 10;
>
>    for_template_test<double>(List, Length, Value);
> -  for_template_define_test<double>(List, Length, Value);
> +  for_template_define_test<double, int>(List, Length, Value);
> +  for_template_constant_expression_test<double, 2, 4, 8>(List, Length);
>  }
>
>  // CHECK: ![[LOOP_1]] = metadata !{metadata ![[LOOP_1]], metadata ![[UNROLL_FULL:.*]], metadata ![[WIDTH_4:.*]], metadata ![[INTERLEAVE_4:.*]], metadata ![[INTENABLE_1:.*]]}
> @@ -124,6 +169,17 @@ void template_test(double *List, int Length) {
>  // CHECK: ![[LOOP_5]] = metadata !{metadata ![[LOOP_5]], metadata ![[UNROLL_DISABLE:.*]], metadata ![[WIDTH_1:.*]]}
>  // CHECK: ![[WIDTH_1]] = metadata !{metadata !"llvm.loop.vectorize.width", i32 1}
>  // CHECK: ![[LOOP_6]] = metadata !{metadata ![[LOOP_6]], metadata ![[UNROLL_8:.*]], metadata ![[INTERLEAVE_2:.*]], metadata ![[WIDTH_2:.*]]}
> -// CHECK: ![[LOOP_7]] = metadata !{metadata ![[LOOP_7]], metadata ![[UNROLL_8:.*]], metadata ![[INTERLEAVE_8:.*]], metadata ![[WIDTH_8:.*]]}
> +// CHECK: ![[LOOP_7]] = metadata !{metadata ![[LOOP_7]], metadata ![[WIDTH_5:.*]]}
> +// CHECK: ![[WIDTH_5]] = metadata !{metadata !"llvm.loop.vectorize.width", i32 5}
> +// CHECK: ![[LOOP_8]] = metadata !{metadata ![[LOOP_8]], metadata ![[WIDTH_5:.*]]}
> +// CHECK: ![[LOOP_9]] = metadata !{metadata ![[LOOP_9]], metadata ![[UNROLL_8:.*]], metadata ![[INTERLEAVE_8:.*]], metadata ![[WIDTH_8:.*]]}
>  // CHECK: ![[INTERLEAVE_8]] = metadata !{metadata !"llvm.loop.interleave.count", i32 8}
> -// CHECK: ![[LOOP_8]] = metadata !{metadata ![[LOOP_8]], metadata ![[UNROLL_8:.*]], metadata ![[INTERLEAVE_2:.*]], metadata ![[WIDTH_2:.*]]}
> +// CHECK: ![[LOOP_10]] = metadata !{metadata ![[LOOP_10]], metadata ![[UNROLL_8:.*]], metadata ![[INTERLEAVE_2:.*]], metadata ![[WIDTH_2:.*]]}
> +// CHECK: ![[LOOP_11]] = metadata !{metadata ![[LOOP_11]], metadata ![[UNROLL_8:.*]], metadata ![[INTERLEAVE_4:.*]], metadata ![[WIDTH_2:.*]]}
> +// CHECK: ![[LOOP_12]] = metadata !{metadata ![[LOOP_12]], metadata ![[UNROLL_24:.*]], metadata ![[INTERLEAVE_10:.*]], metadata ![[WIDTH_6:.*]]}
> +// CHECK: ![[UNROLL_24]] = metadata !{metadata !"llvm.loop.unroll.count", i32 24}
> +// CHECK: ![[INTERLEAVE_10]] = metadata !{metadata !"llvm.loop.interleave.count", i32 10}
> +// CHECK: ![[WIDTH_6]] = metadata !{metadata !"llvm.loop.vectorize.width", i32 6}
> +// CHECK: ![[LOOP_13]] = metadata !{metadata ![[LOOP_13]], metadata ![[UNROLL_32:.*]], metadata ![[INTERLEAVE_16:.*]], metadata ![[WIDTH_8:.*]]}
> +// CHECK: ![[UNROLL_32]] = metadata !{metadata !"llvm.loop.unroll.count", i32 32}
> +// CHECK: ![[INTERLEAVE_16]] = metadata !{metadata !"llvm.loop.interleave.count", i32 16}
> diff --git a/test/Misc/ast-print-pragmas.cpp b/test/Misc/ast-print-pragmas.cpp
> index 25421fc..23f533f 100644
> --- a/test/Misc/ast-print-pragmas.cpp
> +++ b/test/Misc/ast-print-pragmas.cpp
> @@ -38,3 +38,18 @@ void test(int *List, int Length) {
>      i++;
>    }
>  }
> +
> +template <int V, int I>
> +void test_nontype_template_param(int *List, int Length) {
> +#pragma clang loop vectorize_width(V) interleave_count(I)
> +  for (int i = 0; i < Length; i++) {
> +    List[i] = i;
> +  }
> +}
> +
> +// CHECK: #pragma clang loop interleave_count(I)
> +// CHECK: #pragma clang loop vectorize_width(V)
> +
> +void test_templates(int *List, int Length) {
> +  test_nontype_template_param<2, 4>(List, Length);
> +}
> diff --git a/test/PCH/pragma-loop.cpp b/test/PCH/pragma-loop.cpp
> index 3eb1f60..2640020 100644
> --- a/test/PCH/pragma-loop.cpp
> +++ b/test/PCH/pragma-loop.cpp
> @@ -16,6 +16,8 @@
>  // CHECK: #pragma unroll
>  // CHECK: #pragma unroll (32)
>  // CHECK: #pragma nounroll
> +// CHECK: #pragma clang loop interleave_count(I)
> +// CHECK: #pragma clang loop vectorize_width(V)
>
>  #ifndef HEADER
>  #define HEADER
> @@ -81,6 +83,15 @@ public:
>        i++;
>      }
>    }
> +
> +  template <int V, int I>
> +  inline void run7(int *List, int Length) {
> +#pragma clang loop vectorize_width(V)
> +#pragma clang loop interleave_count(I)
> +    for (int i = 0; i < Length; i++) {
> +      List[i] = i;
> +    }
> +  }
>  };
>  #else
>
> @@ -95,6 +106,7 @@ void test() {
>    pt.run4(List, 100);
>    pt.run5(List, 100);
>    pt.run6(List, 100);
> +  pt.run7<2, 4>(List, 100);
>  }
>
>  #endif
> diff --git a/test/Parser/pragma-loop.cpp b/test/Parser/pragma-loop.cpp
> index 742a5d7..40e128d 100644
> --- a/test/Parser/pragma-loop.cpp
> +++ b/test/Parser/pragma-loop.cpp
> @@ -3,6 +3,81 @@
>  // Note that this puts the expected lines before the directives to work around
>  // limitations in the -verify mode.
>
> +template <int V, int I>
> +void test_nontype_template_param(int *List, int Length) {
> +#pragma clang loop vectorize_width(V) interleave_count(I)
> +  for (int i = 0; i < Length; i++) {
> +    List[i] = i;
> +  }
> +
> +#pragma clang loop vectorize_width(V + 4) interleave_count(I + 4)
> +  for (int i = 0; i < Length; i++) {
> +    List[i] = i;
> +  }
> +}
> +
> +template <int V>
> +void test_nontype_template_vectorize(int *List, int Length) {
> +  /* expected-error {{invalid argument '-1'; expected a positive 32-bit integer value}} */ #pragma clang loop vectorize_width(V)
> +  for (int i = 0; i < Length; i++) {
> +    List[i] = i;
> +  }
> +
> +  /* expected-error {{invalid argument '0'; expected a positive 32-bit integer value}} */ #pragma clang loop vectorize_width(V / 2)
> +  for (int i = 0; i < Length; i++) {
> +    List[i] += i;
> +  }
> +}
> +
> +template <int I>
> +void test_nontype_template_interleave(int *List, int Length) {
> +  /* expected-error {{invalid argument '-1'; expected a positive 32-bit integer value}} */ #pragma clang loop interleave_count(I)
> +  for (int i = 0; i < Length; i++) {
> +    List[i] = i;
> +  }
> +
> +  /* expected-error {{invalid argument '0'; expected a positive 32-bit integer value}} */ #pragma clang loop interleave_count(2 % I)
> +  for (int i = 0; i < Length; i++) {
> +    List[i] = i;
> +  }
> +}
> +
> +template <char V>
> +void test_nontype_template_char(int *List, int Length) {
> +  /* expected-error {{invalid argument 'char'; expected a positive 32-bit integer value}} */ #pragma clang loop vectorize_width(V)
> +  for (int i = 0; i < Length; i++) {
> +    List[i] = i;
> +  }
> +}
> +
> +template <bool V>
> +void test_nontype_template_bool(int *List, int Length) {
> +  /* expected-error {{invalid argument 'bool'; expected a positive 32-bit integer value}} */ #pragma clang loop vectorize_width(V)
> +  for (int i = 0; i < Length; i++) {
> +    List[i] = i;
> +  }
> +}
> +
> +template <int V, int I>
> +void test_nontype_template_badarg(int *List, int Length) {
> +  /* expected-error {{invalid expression; expected an integer constant expression}} expected-error {{use of undeclared identifier 'Vec'}} */ #pragma clang loop vectorize_width(Vec) interleave_count(I)
> +  /* expected-error {{invalid expression; expected an integer constant expression}} expected-error {{use of undeclared identifier 'Int'}} */ #pragma clang loop vectorize_width(V) interleave_count(Int)
> +  for (int i = 0; i < Length; i++) {
> +    List[i] = i;
> +  }
> +}
> +
> +template <typename T>
> +void test_type_template_vectorize(int *List, int Length) {
> +  const T Value = -1;
> +  /* expected-error {{invalid argument '-1'; expected a positive 32-bit integer value}} */ #pragma clang loop vectorize_width(Value)
> +  for (int i = 0; i < Length; i++) {
> +    List[i] = i;
> +  }
> +}
> +
> +void BadFunctionDecl();
> +
>  void test(int *List, int Length) {
>    int i = 0;
>
> @@ -43,6 +118,8 @@ void test(int *List, int Length) {
>      VList[j] = List[j];
>    }
>
> +  test_nontype_template_param<4, 8>(List, Length);
> +
>  /* expected-error {{expected '('}} */ #pragma clang loop vectorize
>  /* expected-error {{expected '('}} */ #pragma clang loop interleave
>  /* expected-error {{expected '('}} */ #pragma clang loop unroll
> @@ -55,40 +132,51 @@ void test(int *List, int Length) {
>  /* expected-error {{expected ')'}} */ #pragma clang loop interleave_count(4
>  /* expected-error {{expected ')'}} */ #pragma clang loop unroll_count(4
>
> -/* expected-error {{missing argument to '#pragma clang loop vectorize'}} */ #pragma clang loop vectorize()
> -/* expected-error {{missing argument to '#pragma clang loop interleave_count'}} */ #pragma clang loop interleave_count()
> -/* expected-error {{missing argument to '#pragma clang loop unroll'}} */ #pragma clang loop unroll()
> +/* expected-error {{missing argument; expected 'enable' or 'disable'}} */ #pragma clang loop vectorize()
> +/* expected-error {{missing argument; expected a positive 32-bit integer value}} */ #pragma clang loop interleave_count()
> +/* expected-error {{missing argument; expected 'enable' or 'disable'}} */ #pragma clang loop unroll()
>
>  /* expected-error {{missing option}} */ #pragma clang loop
>  /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop badkeyword
>  /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop badkeyword(enable)
>  /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop vectorize(enable) badkeyword(4)
>  /* expected-warning {{extra tokens at end of '#pragma clang loop'}} */ #pragma clang loop vectorize(enable) ,
> -
>    while (i-4 < Length) {
>      List[i] = i;
>    }
>
> -/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop vectorize_width(0)
> -/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop interleave_count(0)
> -/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop unroll_count(0)
> +/* expected-error {{invalid argument '0'; expected a positive 32-bit integer value}} */ #pragma clang loop vectorize_width(0)
> +/* expected-error {{invalid argument '0'; expected a positive 32-bit integer value}} */ #pragma clang loop interleave_count(0)
> +/* expected-error {{invalid argument '0'; expected a positive 32-bit integer value}} */ #pragma clang loop unroll_count(0)
> +
> +/* expected-error {{invalid expression; expected an integer constant expression}} */ /* expected-note {{division by zero}} */ #pragma clang loop vectorize_width(10 / 0)
> +/* expected-error {{invalid argument '0'; expected a positive 32-bit integer value}} */ #pragma clang loop interleave_count(10 / 5 - 2)
>    while (i-5 < Length) {
>      List[i] = i;
>    }
>
> -/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop vectorize_width(3000000000)
> -/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop interleave_count(3000000000)
> -/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop unroll_count(3000000000)
> +test_nontype_template_vectorize<4>(List, Length);
> +/* expected-note {{in instantiation of function template specialization}} */ test_nontype_template_vectorize<-1>(List, Length);
> +test_nontype_template_interleave<8>(List, Length);
> +/* expected-note {{in instantiation of function template specialization}} */ test_nontype_template_interleave<-1>(List, Length);
> +
> +/* expected-note {{in instantiation of function template specialization}} */ test_nontype_template_char<'A'>(List, Length); // Loop hint arg cannot be a char.
> +/* expected-note {{in instantiation of function template specialization}} */ test_nontype_template_bool<true>(List, Length);  // Or a bool.
> +/* expected-note {{in instantiation of function template specialization}} */ test_type_template_vectorize<int>(List, Length); // Or a template type.
> +
> +/* expected-error {{invalid argument '3000000000'; expected a positive 32-bit integer value}} */ #pragma clang loop vectorize_width(3000000000)
> +/* expected-error {{invalid argument '3000000000'; expected a positive 32-bit integer value}} */ #pragma clang loop interleave_count(3000000000)
> +/* expected-error {{invalid argument '3000000000'; expected a positive 32-bit integer value}} */ #pragma clang loop unroll_count(3000000000)
>    while (i-6 < Length) {
>      List[i] = i;
>    }
>
> -/* expected-error {{expected ')'}} */ #pragma clang loop vectorize_width(1 +) 1
> +/* expected-warning {{extra tokens at end of '#pragma clang loop'}} */ #pragma clang loop vectorize_width(1 +) 1
>  /* expected-warning {{extra tokens at end of '#pragma clang loop'}} */ #pragma clang loop vectorize_width(1) +1
>
> -/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop vectorize_width(badvalue)
> -/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop interleave_count(badvalue)
> -/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop unroll_count(badvalue)
> +/* expected-error {{invalid expression; expected an integer constant expression}} expected-error {{use of undeclared identifier 'badvalue'}} */ #pragma clang loop vectorize_width(badvalue)
> +/* expected-error {{invalid expression; expected an integer constant expression}} expected-error {{use of undeclared identifier 'badvalue'}} */ #pragma clang loop interleave_count(badvalue)
> +/* expected-error {{invalid expression; expected an integer constant expression}} expected-error {{use of undeclared identifier 'badvalue'}} */ #pragma clang loop unroll_count(badvalue)
>    while (i-6 < Length) {
>      List[i] = i;
>    }
> @@ -105,9 +193,9 @@ void test(int *List, int Length) {
>  /* expected-error {{invalid argument; expected 'enable' or 'disable'}} */ #pragma clang loop vectorize(()
>  /* expected-error {{invalid argument; expected 'enable' or 'disable'}} */ #pragma clang loop interleave(*)
>  /* expected-error {{invalid argument; expected 'full' or 'disable'}} */ #pragma clang loop unroll(=)
> -/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop vectorize_width(^)
> -/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop interleave_count(/)
> -/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop unroll_count(==)
> +/* expected-error {{invalid expression; expected an integer constant expression}} expected-error {{type name requires a specifier or qualifier}} expected-error {{expected expression}} */ #pragma clang loop vectorize_width(^)
> +/* expected-error {{invalid expression; expected an integer constant expression}} expected-error {{expected expression}} expected-error {{expected expression}} */ #pragma clang loop interleave_count(/)
> +/* expected-error {{invalid expression; expected an integer constant expression}} expected-error {{expected expression}} expected-error {{expected expression}} */ #pragma clang loop unroll_count(==)
>    while (i-8 < Length) {
>      List[i] = i;
>    }

I'd like to see a test added for constant expressions that are a
nonsense parsing error, just to make sure we recover properly.

> diff --git a/test/Parser/pragma-unroll.cpp b/test/Parser/pragma-unroll.cpp
> index 5d28dae..835d56a 100644
> --- a/test/Parser/pragma-unroll.cpp
> +++ b/test/Parser/pragma-unroll.cpp
> @@ -27,7 +27,7 @@ void test(int *List, int Length) {
>    }
>
>  /* expected-error {{expected ')'}} */ #pragma unroll(4
> -/* expected-error {{missing argument to '#pragma unroll'}} */ #pragma unroll()
> +/* expected-error {{missing argument; expected a positive 32-bit integer value}} */ #pragma unroll()
>  /* expected-warning {{extra tokens at end of '#pragma unroll'}} */ #pragma unroll 1 2
>    while (i-6 < Length) {
>      List[i] = i;
> @@ -38,12 +38,12 @@ void test(int *List, int Length) {
>      List[i] = i;
>    }
>
> -/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma unroll(()
> -/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma unroll -
> -/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma unroll(0)
> -/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma unroll 0
> -/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma unroll(3000000000)
> -/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma unroll 3000000000
> +/* expected-error {{expected expression}} expected-error {{invalid expression; expected an integer constant expression}} */ #pragma unroll(()
> +/* expected-error {{expected expression}} expected-error {{invalid expression; expected an integer constant expression}} */ #pragma unroll -
> +/* expected-error {{invalid argument '0'; expected a positive 32-bit integer value}} */ #pragma unroll(0)
> +/* expected-error {{invalid argument '0'; expected a positive 32-bit integer value}} */ #pragma unroll 0
> +/* expected-error {{invalid argument '3000000000'; expected a positive 32-bit integer value}} */ #pragma unroll(3000000000)
> +/* expected-error {{invalid argument '3000000000'; expected a positive 32-bit integer value}} */ #pragma unroll 3000000000
>    while (i-8 < Length) {
>      List[i] = i;
>    }
> diff --git a/utils/TableGen/ClangAttrEmitter.cpp b/utils/TableGen/ClangAttrEmitter.cpp
> index 1790dcb..705e644 100644
> --- a/utils/TableGen/ClangAttrEmitter.cpp
> +++ b/utils/TableGen/ClangAttrEmitter.cpp
> @@ -1606,8 +1606,16 @@ static void EmitAttrList(raw_ostream &OS, StringRef Class,
>    }
>  }
>
> -namespace clang {
> +// Determines if an attribute has a Pragma spelling.
> +static bool AttrHasPragmaSpelling(const Record *R) {
> +  std::vector<FlattenedSpelling> Spellings = GetFlattenedSpellings(*R);
> +  return std::find_if(Spellings.begin(), Spellings.end(),
> +                      [](const FlattenedSpelling &S) {
> +           return S.variety() == "Pragma";
> +         }) != Spellings.end();
> +}
>
> +namespace clang {
>  // Emits the enumeration list for attributes.
>  void EmitClangAttrList(RecordKeeper &Records, raw_ostream &OS) {
>    emitSourceFileHeader("List of all attributes that Clang recognizes", OS);
> @@ -1633,14 +1641,25 @@ void EmitClangAttrList(RecordKeeper &Records, raw_ostream &OS) {
>          " INHERITABLE_PARAM_ATTR(NAME)\n";
>    OS << "#endif\n\n";
>
> +  OS << "#ifndef PRAGMA_SPELLING_ATTR\n";
> +  OS << "#define PRAGMA_SPELLING_ATTR(NAME)\n";
> +  OS << "#endif\n\n";
> +
> +  OS << "#ifndef LAST_PRAGMA_SPELLING_ATTR\n";
> +  OS << "#define LAST_PRAGMA_SPELLING_ATTR(NAME) PRAGMA_SPELLING_ATTR(NAME)\n";
> +  OS << "#endif\n\n";
> +
>    Record *InhClass = Records.getClass("InheritableAttr");
>    Record *InhParamClass = Records.getClass("InheritableParamAttr");
> -  std::vector<Record*> Attrs = Records.getAllDerivedDefinitions("Attr"),
> -                       NonInhAttrs, InhAttrs, InhParamAttrs;
> +  std::vector<Record *> Attrs = Records.getAllDerivedDefinitions("Attr"),
> +                        NonInhAttrs, InhAttrs, InhParamAttrs, PragmaAttrs;
>    for (auto *Attr : Attrs) {
>      if (!Attr->getValueAsBit("ASTNode"))
>        continue;
> -
> +
> +    if (AttrHasPragmaSpelling(Attr))
> +      PragmaAttrs.push_back(Attr);
> +
>      if (Attr->isSubClassOf(InhParamClass))
>        InhParamAttrs.push_back(Attr);
>      else if (Attr->isSubClassOf(InhClass))
> @@ -1649,6 +1668,7 @@ void EmitClangAttrList(RecordKeeper &Records, raw_ostream &OS) {
>        NonInhAttrs.push_back(Attr);
>    }
>
> +  EmitAttrList(OS, "PRAGMA_SPELLING_ATTR", PragmaAttrs);
>    EmitAttrList(OS, "INHERITABLE_PARAM_ATTR", InhParamAttrs);
>    EmitAttrList(OS, "INHERITABLE_ATTR", InhAttrs);
>    EmitAttrList(OS, "ATTR", NonInhAttrs);
> @@ -1657,6 +1677,8 @@ void EmitClangAttrList(RecordKeeper &Records, raw_ostream &OS) {
>    OS << "#undef INHERITABLE_ATTR\n";
>    OS << "#undef LAST_INHERITABLE_ATTR\n";
>    OS << "#undef LAST_INHERITABLE_PARAM_ATTR\n";
> +  OS << "#undef LAST_PRAGMA_ATTR\n";
> +  OS << "#undef PRAGMA_SPELLING_ATTR\n";
>    OS << "#undef ATTR\n";
>  }
>
>

~Aaron



More information about the cfe-commits mailing list