================
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;
+}
+
----------------
Alexander Musman wrote:
> [email protected] wrote:
> > Richard Smith wrote:
> > > 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?
> > No, section 2.6 of the OpenMP spec specifically requires a random-access
> > iterator type for the iteration variable; specifically:
> >
> > var One of the following:
> > A variable of a signed or unsigned integer type.
> > For C++, a variable of a random access iterator type.
> > For C, a variable of a pointer type.
> >
> Basically it would be OK to have as a loop counter any type, which value at
> any iteration can be calculated in constant time using "begin" and the
> current iteration's number (of some integer type), if we knew how to
> calculate it. As noted by Hal, it is explicitly allowed by OpenMP 4 for
> random access iterators - for example, it specifies that the type in which
> the loop count is: "For C++, if var is of a random access iterator type, then
> the type is the type that would be used by std::distance applied to variables
> of the type of var.". This is why we need a check that there is std::distance.
>
OK. This still seems strange to me, but I agree that the OpenMP spec appears to
want this.
The next question is, what does it mean by "a random access iterator type"? In
the C++ standard, that does not mean
"`std::iterator_traits<T>::iterator_category` is derived from
`std::random_access_iterator_tag`". It means that the type conforms to the
requirements of a random access iterator (see 24.2.7 [random.access.iterators]
p1). And that's not actually a computable property.
Is a diagnostic really required here, or should we just *assume* that the
iteration variable is a random access iterator, use the operations which the
random access iterator requirements require to exist, and produce the relevant
diagnostics if they didn't work? (That's how range-based for loops work, for
instance.)
================
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;
+ }
+
----------------
Alexander Musman wrote:
> Richard Smith wrote:
> > Maybe check this when creating the `BreakStmt` itself? (Clear the
> > `BreakScope` flag in your OpenMP loop statement's `Scope`.)
> How do you think, it is worth adding a bit in Scope, to modify error message
> (this seems to me a rare enough case)?
> There is already an "OpenMPDirectiveScope" bit, but not all openmp directives
> prohibit break stmts.
Adding a bit to `Scope` to indicate that it is an OpenMP scope that prohibits
`break` statements seems fine to me. (Be careful to bump `Flags` from `unsigned
short` to `unsigned int` at the same time, and reorder it before `Depth` to
avoid unnecessary padding.)
http://reviews.llvm.org/D3778
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
