Address review comments

Signed-off-by: yronglin <[email protected]>

Author: yronglin

clang/include/clang/Sema/Sema.h

@@ -5469,8 +5469,7 @@ class Sema final : public SemaBase {
   ExprResult ActOnPredefinedExpr(SourceLocation Loc, tok::TokenKind Kind);
   ExprResult ActOnIntegerConstant(SourceLocation Loc, uint64_t Val);
 
-  bool CheckLoopHintExpr(Expr *E, SourceLocation Loc,
-                         const IdentifierInfo *PragmaNameInfo);
+  bool CheckLoopHintExpr(Expr *E, SourceLocation Loc, bool AllowZero);
 
   ExprResult ActOnNumericConstant(const Token &Tok, Scope *UDLScope = nullptr);
   ExprResult ActOnCharacterConstant(const Token &Tok,

clang/lib/Parse/ParsePragma.cpp

@@ -1570,7 +1570,7 @@ bool Parser::HandlePragmaLoopHint(LoopHint &Hint) {
 
       if (Arg2Error || R.isInvalid() ||
           Actions.CheckLoopHintExpr(R.get(), Toks[0].getLocation(),
-                                    PragmaNameInfo))
+                                    /*AllowZero=*/false))
         return false;
 
       // Argument is a constant expression with an integer type.
@@ -1594,8 +1594,9 @@ bool Parser::HandlePragmaLoopHint(LoopHint &Hint) {
 
     ConsumeToken(); // Consume the constant expression eof terminator.
 
-    if (R.isInvalid() || Actions.CheckLoopHintExpr(
-                             R.get(), Toks[0].getLocation(), PragmaNameInfo))
+    if (R.isInvalid() ||
+        Actions.CheckLoopHintExpr(R.get(), Toks[0].getLocation(),
+                                  /*AllowZero=*/true))
       return false;
 
     // Argument is a constant expression with an integer type.

clang/lib/Sema/SemaExpr.cpp

@@ -3885,8 +3885,7 @@ static Expr *BuildFloatingLiteral(Sema &S, NumericLiteralParser &Literal,
   return FloatingLiteral::Create(S.Context, Val, isExact, Ty, Loc);
 }
 
-bool Sema::CheckLoopHintExpr(Expr *E, SourceLocation Loc,
-                             const IdentifierInfo *PragmaNameInfo) {
+bool Sema::CheckLoopHintExpr(Expr *E, SourceLocation Loc, bool AllowZero) {
   assert(E && "Invalid expression");
 
   if (E->isValueDependent())
@@ -3909,9 +3908,8 @@ bool Sema::CheckLoopHintExpr(Expr *E, SourceLocation Loc,
   // "The values of 0 and 1 block any unrolling of the loop."
   // The values doesn't have to be strictly positive in '#pragma GCC unroll' and
   // '#pragma unroll' cases.
-  bool ValueIsPositive = PragmaNameInfo->isStr("unroll")
-                             ? ValueAPS.isNonNegative()
-                             : ValueAPS.isStrictlyPositive();
+  bool ValueIsPositive =
+      AllowZero ? ValueAPS.isNonNegative() : ValueAPS.isStrictlyPositive();
   if (!ValueIsPositive || ValueAPS.getActiveBits() > 31) {
     Diag(E->getExprLoc(), diag::err_pragma_loop_invalid_argument_value)
         << toString(ValueAPS, 10) << ValueIsPositive;

clang/lib/Sema/SemaStmtAttr.cpp

@@ -150,8 +150,8 @@ static Attr *handleLoopHintAttr(Sema &S, Stmt *St, const ParsedAttr &A,
     if (Option == LoopHintAttr::VectorizeWidth) {
       assert((ValueExpr || (StateLoc && StateLoc->Ident)) &&
              "Attribute must have a valid value expression or argument.");
-      if (ValueExpr &&
-          S.CheckLoopHintExpr(ValueExpr, St->getBeginLoc(), OptionLoc->Ident))
+      if (ValueExpr && S.CheckLoopHintExpr(ValueExpr, St->getBeginLoc(),
+                                           /*AllowZero=*/false))
         return nullptr;
       if (StateLoc && StateLoc->Ident && StateLoc->Ident->isStr("scalable"))
         State = LoopHintAttr::ScalableWidth;
@@ -161,7 +161,8 @@ static Attr *handleLoopHintAttr(Sema &S, Stmt *St, const ParsedAttr &A,
                Option == LoopHintAttr::UnrollCount ||
                Option == LoopHintAttr::PipelineInitiationInterval) {
       assert(ValueExpr && "Attribute must have a valid value expression.");
-      if (S.CheckLoopHintExpr(ValueExpr, St->getBeginLoc(), OptionLoc->Ident))
+      if (S.CheckLoopHintExpr(ValueExpr, St->getBeginLoc(),
+                              /*AllowZero=*/false))
         return nullptr;
       State = LoopHintAttr::Numeric;
     } else if (Option == LoopHintAttr::Vectorize ||

clang/lib/Sema/SemaTemplateInstantiate.cpp

@@ -2151,7 +2151,7 @@ TemplateInstantiator::TransformLoopHintAttr(const LoopHintAttr *LH) {
 
   // Generate error if there is a problem with the value.
   if (getSema().CheckLoopHintExpr(TransformedExpr, LH->getLocation(),
-                                  LH->getAttrName()))
+                                  LH->getOption() == LoopHintAttr::UnrollCount))
     return LH;
 
   // Create new LoopHintValueAttr with integral expression in place of the