[OpenMP] clang doesnt diagnose if there is a lexical block around a for stmt for OpenMP loops. It is technically not allowed in the OpenMP standard

[• davidsh]

Summary

#pragma omp for
{
for(...)
}

This is technically not allowed by the standard, gcc doesnt allow such code.

[ABataev]

lib/Sema/SemaOpenMP.cpp

5137–5138

You also should skip all AttributedStmts.

[• davidsh]

Adding a parameter to IgnoreContainers instead of copying the logic.

[kkwli0]

Should we issue a warning message in this case?

[ABataev]

In D24615#561532, @kkwli0 wrote:

Should we issue a warning message in this case?

Agree

include/clang/AST/Stmt.h

public:
   /// viewAST - Visualize an AST rooted at this Stmt* using GraphViz.  Only
   ///   works on systems with GraphViz (Mac OS X) or dot+gv installed.
   void viewAST() const;
 
   /// Skip past any implicit AST nodes which might surround this
   /// statement, such as ExprWithCleanups or ImplicitCastExpr nodes.
   Stmt *IgnoreImplicit();
 
-  /// \brief Skip no-op (attributed, compound) container stmts and skip captured
-  /// stmt at the top, if \a IgnoreCaptured is true.
-  Stmt *IgnoreContainers(bool IgnoreCaptured = false);
+  /// \brief Skip no-op (attributed, compound if \a IgnoreCompound is true)
+  /// container stmts and skip captured stmt at the top, if \a IgnoreCaptured
+  /// is true.
+  Stmt *IgnoreContainers(bool IgnoreCaptured = false, 
+                         bool IgnoreCompound = true);
 
   const Stmt *stripLabelLikeStatements() const;
   Stmt *stripLabelLikeStatements() {
     return const_cast<Stmt*>(
       const_cast<const Stmt*>(this)->stripLabelLikeStatements());
   }
 
   /// Child Iterators: All subclasses must implement 'children'

lib/AST/Stmt.cpp

if (auto *bte = dyn_cast<CXXBindTemporaryExpr>(s))
     s = bte->getSubExpr();
 
   while (auto *ice = dyn_cast<ImplicitCastExpr>(s))
     s = ice->getSubExpr();
 
   return s;
 }
 
-/// \brief Skip no-op (attributed, compound) container stmts and skip captured
-/// stmt at the top, if \a IgnoreCaptured is true.
-Stmt *Stmt::IgnoreContainers(bool IgnoreCaptured) {
+/// \brief Skip no-op (attributed, compound if \a IgnoreCompound is true)
+/// container stmts and skip captured stmt at the top, if \a IgnoreCaptured
+/// is true.
+Stmt *Stmt::IgnoreContainers(bool IgnoreCaptured, bool IgnoreCompound) {
   Stmt *S = this;
   if (IgnoreCaptured)
     if (auto CapS = dyn_cast_or_null<CapturedStmt>(S))
       S = CapS->getCapturedStmt();
   while (true) {
     if (auto AS = dyn_cast_or_null<AttributedStmt>(S))
       S = AS->getSubStmt();
-    else if (auto CS = dyn_cast_or_null<CompoundStmt>(S)) {
+    else if (IgnoreCompound) {
+      if (auto CS = dyn_cast_or_null<CompoundStmt>(S)) {
         if (CS->size() != 1)
           break;
         S = CS->body_back();
       } else
          break;
+    } else
+      break;
   }
   return S;
 }
 
 /// \brief Strip off all label-like statements.
 ///
 /// This will strip off label statements, case statements, attributed
 /// statements and default statements recursively.

lib/Sema/SemaOpenMP.cpp

if (OrderedLoopCountExpr->EvaluateAsInt(Result, SemaRef.getASTContext())) {
       NestedLoopCount = Result.getLimitedValue();
     }
   }
   // This is helper routine for loop directives (e.g., 'for', 'simd',
   // 'for simd', etc.).
   llvm::MapVector<Expr *, DeclRefExpr *> Captures;
   SmallVector<LoopIterationSpace, 4> IterSpaces;
   IterSpaces.resize(NestedLoopCount);
-  Stmt *CurStmt = AStmt->IgnoreContainers(/* IgnoreCaptured */ true);
+  Stmt *CurStmt = AStmt->IgnoreContainers(/* IgnoreCaptured */ true, 
+                                          /* IgnoreCompound */ false);
   for (unsigned Cnt = 0; Cnt < NestedLoopCount; ++Cnt) {
     if (CheckOpenMPIterationSpace(DKind, CurStmt, SemaRef, DSA, Cnt,
                                   NestedLoopCount, CollapseLoopCountExpr,
                                   OrderedLoopCountExpr, VarsWithImplicitDSA,
                                   IterSpaces[Cnt], Captures))
       return 0;
     // Move on to the next nested for loop, or to the loop body.
     // OpenMP [2.8.1, simd construct, Restrictions]
     // All loops associated with the construct must be perfectly nested; that

test/OpenMP/for_loop_messages.cpp

 #pragma omp parallel
 // expected-error@+2 {{statement after '#pragma omp for' must be a for loop}}
 #pragma omp for
   for (auto &item : a) {
     item = item + 1;
   }
 
 #pragma omp parallel
+// expected-error@+2 {{statement after '#pragma omp for' must be a for loop}}
+#pragma omp for
+  {
+  for (int i = 0; i < 16; ++i)
+    ;
+  }
+
+#pragma omp parallel
 // expected-note@+3 {{loop step is expected to be positive due to this condition}}
 // expected-error@+2 {{increment expression must cause 'i' to increase on each iteration of OpenMP for loop}}
 #pragma omp for
   for (unsigned i = 9; i < 10; i--) {
     c[i] = a[i] + b[i];
   }
 
   int(*lb)[4] = nullptr;