Add begin source location for the attributed statement created from PragmaLoopHint decorated loop

[ychen]

Summary

Right now it is a '<invalid sloc>' for cases like this.
CounterCoverageMappingBuilder relies on the information to decide the
region for a attributed loop.

Fix PR40971

[jdenny]

This looks like a continuation of the work started in D61643 (which makes the loc of #pragma available to pragma handlers) and D61509 (which changes the start loc for OpenMP pragmas from omp to #pragma).

clang/lib/Parse/ParsePragma.cpp

3102

When I remove this change, the new test still passes. Please make sure this case is covered.

clang/lib/Parse/ParseStmt.cpp

2176

I think the call to getExpansionLoc shouldn't be here. If the ast dumper or other clients want to convert to that form, they can do so.

[ychen]

• Address feedback

[ychen]

clang/lib/Parse/ParsePragma.cpp

3102

Added a test case for this.

clang/lib/Parse/ParseStmt.cpp

2176

Indeed. Removing this helps code coverage shows the coverage for the macro spelling location. Thanks.

[jdenny]

LGTM except for the nit I put in a comment.

However, I have little experience with these particular pragmas. Before pushing, please give others a few more days in case they want to review.

clang/test/AST/sourceranges.cpp

116

Is there any reason not to check for LoopHintAttr here and on the _Pragma case below? If so, I suggest a comment to explain why these cases are different.

[ychen]

• add comment

clang/test/AST/sourceranges.cpp

116

Comment added.

The existing checking of LoopHintAttr already covered the code changes in ParsePragma.cpp. In the other two cases LoopHintAttr have its location in <scratch space> which I felt unrelated to this change so I skipped them to avoid confusion.

[jdenny]

clang/test/AST/sourceranges.cpp

116

Thanks.

[ychen]

• update

[aaron.ballman]

LGTM aside from a minor nit.

clang/lib/Parse/ParseStmt.cpp

2198

You can use isInvalid() here rather than !isValid(). Also, I'd appreciate an assertion message like && "start of attribute range already set" or something.

[ychen]

• address comments

[erichkeane]

Note that this causes a regression (reported here) https://bugs.llvm.org/show_bug.cgi?id=46336

The assert added here breaks in the case where a pragma loop attribute is combined with a C++ attribute.

[ychen]

In D80944#2095564, @erichkeane wrote:

Note that this causes a regression (reported here) https://bugs.llvm.org/show_bug.cgi?id=46336

The assert added here breaks in the case where a pragma loop attribute is combined with a C++ attribute.

Resolved with
4676cf444ea2678660ee48279be99efde4bf60e9
8c6c606cdc72c3ddd55f382d91ef1afc3cb9f2a8

clang/lib/Parse/ParsePragma.cpp

while (Tok.is(tok::identifier)) {
     if (ParseLoopHintValue(PP, Tok, PragmaName, Option, /*ValueInParens=*/true,
                            *Info))
       return;
 
     // Generate the loop hint token.
     Token LoopHintTok;
     LoopHintTok.startToken();
     LoopHintTok.setKind(tok::annot_pragma_loop_hint);
-    LoopHintTok.setLocation(PragmaName.getLocation());
+    LoopHintTok.setLocation(Introducer.Loc);
     LoopHintTok.setAnnotationEndLoc(PragmaName.getLocation());
     LoopHintTok.setAnnotationValue(static_cast<void *>(Info));
     TokenList.push_back(LoopHintTok);
   }
 
   if (Tok.isNot(tok::eod)) {
     PP.Diag(Tok.getLocation(), diag::warn_pragma_extra_tokens_at_eol)
         << "clang loop";
if (Tok.isNot(tok::eod)) {
       return;
     }
   }
 
   // Generate the hint token.
   auto TokenArray = std::make_unique<Token[]>(1);
   TokenArray[0].startToken();
   TokenArray[0].setKind(tok::annot_pragma_loop_hint);
-  TokenArray[0].setLocation(PragmaName.getLocation());
+  TokenArray[0].setLocation(Introducer.Loc);
   TokenArray[0].setAnnotationEndLoc(PragmaName.getLocation());
   TokenArray[0].setAnnotationValue(static_cast<void *>(Info));
   PP.EnterTokenStream(std::move(TokenArray), 1,
                       /*DisableMacroExpansion=*/false, /*IsReinject=*/false);
 }
 
 /// Handle the Microsoft \#pragma intrinsic extension.
 ///

clang/lib/Parse/ParseStmt.cpp

 
 StmtResult Parser::ParsePragmaLoopHint(StmtVector &Stmts,
                                        ParsedStmtContext StmtCtx,
                                        SourceLocation *TrailingElseLoc,
                                        ParsedAttributesWithRange &Attrs) {
   // Create temporary attribute list.
   ParsedAttributesWithRange TempAttrs(AttrFactory);
 
+  SourceLocation StartLoc = Tok.getLocation();
+
   // Get loop hints and consume annotated token.
   while (Tok.is(tok::annot_pragma_loop_hint)) {
     LoopHint Hint;
     if (!HandlePragmaLoopHint(Hint))
       continue;
 
     ArgsUnion ArgHints[] = {Hint.PragmaNameLoc, Hint.OptionLoc, Hint.StateLoc,
                             ArgsUnion(Hint.ValueExpr)};
     TempAttrs.addNew(Hint.PragmaNameLoc->Ident, Hint.Range, nullptr,
                      Hint.PragmaNameLoc->Loc, ArgHints, 4,
                      ParsedAttr::AS_Pragma);
   }
 
   // Get the next statement.
   MaybeParseCXX11Attributes(Attrs);
 
   StmtResult S = ParseStatementOrDeclarationAfterAttributes(
       Stmts, StmtCtx, TrailingElseLoc, Attrs);
 
   Attrs.takeAllFrom(TempAttrs);
+
+  assert(Attrs.Range.getBegin().isInvalid() &&
+         "start of attribute range already set");
+  Attrs.Range.setBegin(StartLoc);
   return S;
 }
 
 Decl *Parser::ParseFunctionStatementBody(Decl *Decl, ParseScope &BodyScope) {
   assert(Tok.is(tok::l_brace));
   SourceLocation LBraceLoc = Tok.getLocation();
 
   PrettyDeclStackTraceEntry CrashInfo(Actions.Context, Decl, LBraceLoc,

clang/test/AST/sourceranges.cpp

void f() {
     int __attribute__((unused)) i3;
     // CHECK: DeclStmt {{.*}} <<built-in>:{{.*}}, {{.*}}:[[@LINE+1]]:40>
     __declspec(dllexport) extern int i4;
     // CHECK: DeclStmt {{.*}} <line:[[@LINE+1]]:5, col:40>
     extern int __declspec(dllexport) i5;
   }
 }
 
+// CHECK: NamespaceDecl {{.*}} attributed_stmt
+namespace attributed_stmt {
+  // In DO_PRAGMA and _Pragma cases, `LoopHintAttr` comes from <scratch space>
+  // file.
+
+  #define DO_PRAGMA(x) _Pragma (#x)
+
+  void f() {
+    // CHECK: AttributedStmt {{.*}} <line:[[@LINE-3]]:24, line:[[@LINE+2]]:33>
+    DO_PRAGMA (unroll(2))
+    for (int i = 0; i < 10; ++i);
+
+    // CHECK: AttributedStmt {{.*}} <line:[[@LINE+2]]:5, line:[[@LINE+3]]:33>
+    // CHECK: LoopHintAttr {{.*}} <line:[[@LINE+1]]:13, col:22>
+    #pragma unroll(2)
+    for (int i = 0; i < 10; ++i);
+
+    // CHECK: AttributedStmt {{.*}} <line:[[@LINE+2]]:5, line:[[@LINE+5]]:33>
+    // CHECK: LoopHintAttr {{.*}} <line:[[@LINE+1]]:19, col:41>
+    #pragma clang loop vectorize(enable)
+    // CHECK: LoopHintAttr {{.*}} <line:[[@LINE+1]]:19, col:42>
+    #pragma clang loop interleave(enable)
+    for (int i = 0; i < 10; ++i);
+
+    // CHECK: AttributedStmt {{.*}} <line:[[@LINE+1]]:5, line:[[@LINE+2]]:33>
+    _Pragma("unroll(2)")
+    for (int i = 0; i < 10; ++i);
+  }
+}
+
 #if __cplusplus >= 201703L
 // CHECK-1Z: FunctionDecl {{.*}} construct_with_init_list
 std::map<int, int> construct_with_init_list() {
   // CHECK-1Z-NEXT: CompoundStmt
   // CHECK-1Z-NEXT: ReturnStmt {{.*}} <line:[[@LINE+5]]:3, col:35
   // CHECK-1Z-NEXT: ExprWithCleanups {{.*}} <col:10, col:35
   // CHECK-1Z-NEXT: CXXBindTemporaryExpr {{.*}} <col:10, col:35
   // CHECK-1Z-NEXT: CXXTemporaryObjectExpr {{.*}} <col:10, col:35