[PATCH] [OPENMP] Loop canonical form analysis (Sema)

[Alexey Bataev]

> ================
> Comment at: lib/Sema/SemaOpenMP.cpp:1451-1457
> @@ +1450,9 @@
> +  // Perform conformance checks of the loop body.
> +  ForBreakStmtChecker BreakCheck;
> +  if (CurStmt && BreakCheck.Visit(CurStmt)) {
> +    for (auto Break : BreakCheck.getBreakStmts())
> +      Diag(Break->getLocStart(), diag::err_omp_loop_cannot_break)
> +          << getOpenMPDirectiveName(DKind);
> +    return true;
> +  }
> +
> ----------------
> Maybe check this when creating the `BreakStmt` itself? (Clear the `BreakScope` flag in your OpenMP loop statement's `Scope`.)
>
Richard,
But then we will see some bad kind of diagnostic, like "'break' 
statement not in loop or switch statement", though actually there is a 
loop statement. That's quite confusing.

Best regards,
Alexey Bataev
=============
Software Engineer
Intel Compiler Team
Intel Corp.

23.05.2014 4:32, Richard Smith пишет:
> ================
> Comment at: lib/Sema/SemaOpenMP.cpp:1222-1281
> @@ +1221,62 @@
> +
> +/// \brief Return true if the given type is pointer or other random access
> +/// iterator (currently only std-compatible iterators are allowed here).
> +bool Sema::IsRandomAccessIteratorType(QualType Ty, SourceLocation Loc) {
> +  if (Ty->isPointerType())
> +    return true;
> +  if (!getLangOpts().CPlusPlus || !Ty->isOverloadableType())
> +    return false;
> +  // Check that var type is a random access iterator, i.e. we can apply
> +  // 'std::distance' to the init and test arguments of the for-loop.
> +  CXXScopeSpec StdScopeSpec;
> +  StdScopeSpec.Extend(Context, getOrCreateStdNamespace(), SourceLocation(),
> +                      SourceLocation());
> +
> +  TemplateDecl *TIT = nullptr;
> +  {
> +    DeclarationNameInfo DNI(&Context.Idents.get("iterator_traits"),
> +                            SourceLocation());
> +    LookupResult LR(*this, DNI, LookupNestedNameSpecifierName);
> +    if (!LookupParsedName(LR, DSAStack->getCurScope(), &StdScopeSpec) ||
> +        !LR.isSingleResult() || !(TIT = LR.getAsSingle<TemplateDecl>()))
> +      return false;
> +  }
> +
> +  CXXRecordDecl *IT = nullptr;
> +  {
> +    TemplateArgumentListInfo Args;
> +    TemplateArgument Arg(Ty);
> +    TemplateArgumentLoc ArgLoc(Arg, Context.CreateTypeSourceInfo(Ty));
> +    Args.addArgument(ArgLoc);
> +    QualType T = CheckTemplateIdType(TemplateName(TIT), SourceLocation(), Args);
> +    if (T.isNull() || RequireCompleteType(Loc, T, 0) ||
> +        !(IT = T->getAsCXXRecordDecl()))
> +      return false;
> +  }
> +
> +  TypeDecl *RAI = nullptr;
> +  {
> +    DeclarationNameInfo DNI(&Context.Idents.get("random_access_iterator_tag"),
> +                            SourceLocation());
> +    LookupResult LR(*this, DNI, LookupOrdinaryName);
> +    CXXRecordDecl *RDType = Ty->getAsCXXRecordDecl();
> +    if (!LookupParsedName(LR, DSAStack->getCurScope(), &StdScopeSpec) ||
> +        !LR.isSingleResult() || !(RAI = LR.getAsSingle<TypeDecl>()) || !RDType)
> +      return false;
> +  }
> +
> +  TypeDecl *IC = nullptr;
> +  {
> +    DeclarationNameInfo DNI(&Context.Idents.get("iterator_category"),
> +                            SourceLocation());
> +    LookupResult LR(*this, DNI, LookupOrdinaryName);
> +    if (!LookupQualifiedName(LR, IT) || !LR.isSingleResult() ||
> +        !(IC = LR.getAsSingle<TypeDecl>()) ||
> +        !Context.hasSameType(Context.getTypeDeclType(RAI),
> +                             Context.getTypeDeclType(IC)))
> +      return false;
> +  }
> +
> +  return true;
> +}
> +
> ----------------
> Wow. I really don't think we should be computing this here. It also seems wrong, since you presumably want to know whether you have contiguously-stored elements, not whether you have random access iterators, right?
>
> ================
> Comment at: lib/Sema/SemaOpenMP.cpp:1451-1457
> @@ +1450,9 @@
> +  // Perform conformance checks of the loop body.
> +  ForBreakStmtChecker BreakCheck;
> +  if (CurStmt && BreakCheck.Visit(CurStmt)) {
> +    for (auto Break : BreakCheck.getBreakStmts())
> +      Diag(Break->getLocStart(), diag::err_omp_loop_cannot_break)
> +          << getOpenMPDirectiveName(DKind);
> +    return true;
> +  }
> +
> ----------------
> Maybe check this when creating the `BreakStmt` itself? (Clear the `BreakScope` flag in your OpenMP loop statement's `Scope`.)
>
> http://reviews.llvm.org/D3778
>
>