================
Comment at: include/clang/AST/StmtOpenMP.h:80
@@ +79,3 @@
+ /// \brief Fetches list of variables associated with this clause.
+ llvm::MutableArrayRef<Expr *> getVars() {
+ return llvm::MutableArrayRef<Expr *>(
----------------
Doug Gregor wrote:
> getVars() is slightly misleading. getVarRefs(), perhaps?
Ok
================
Comment at: include/clang/AST/StmtOpenMP.h:169
@@ +168,3 @@
+ ///
+ explicit OMPDefaultClause()
+ : OMPClause(OMPC_default, SourceLocation(), SourceLocation()),
----------------
Doug Gregor wrote:
> This doesn't need to be 'explicit'.
Ok
================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6418
@@ -6407,1 +6417,3 @@
+def err_omp_clause_ref_type_arg : Error<
+ "arguments of OpenMP clause '%0' cannot be of reference type">;
def err_omp_threadprivate_incomplete_type : Error<
----------------
Doug Gregor wrote:
> How about adding the actual type (via %1) at the end of this? It can be
> important when dealing with template instantiations.
Ok
================
Comment at: lib/Sema/SemaOpenMP.cpp:202
@@ +201,3 @@
+namespace {
+class StructuredStmtChecker : public StmtVisitor<StructuredStmtChecker, bool> {
+public:
----------------
Doug Gregor wrote:
> I gather that this is trying to check whether the given statement is a
> structured block, but it's not clear exactly what rules are being followed.
> (The OpenMP Specification is ridiculously vague in its definition). Note that
> the checking for "goto" rules are elsewhere (as they should be), and we
> aren't trying to check the rules for "throw" (which we can't figure out
> statically anyway), or "setjmp" (which could be here).
Doug, it tries to exclude stand-alone goto, break, continue, throw, return
statements. All other statements are allowed.
================
Comment at: lib/Sema/SemaOpenMP.cpp:210
@@ +209,3 @@
+ //return false;
+ return true;
+ }
----------------
Doug Gregor wrote:
> Why is this always returning true?
I'll fix it
================
Comment at: lib/Sema/SemaOpenMP.cpp:255
@@ +254,3 @@
+ if (!Checker.Visit(AStmt)) {
+ Diag(StartLoc, diag::err_omp_directive_nonblock)
+ << getOpenMPDirectiveName(Kind) << AStmt;
----------------
Doug Gregor wrote:
> This isn't a very helpful diagnostic, because it doesn't show which part of
> the statement makes it not a structured block. Also, there don't seem to be
> any tests for this diagnostic.
>
> I suggest removing the structured-block checking from this patch, because
> it's not nearly as baked as the rest of the patch and I don't want it holding
> up the rest.
Ok, I'll remove it for now.
================
Comment at: lib/Sema/SemaOpenMP.cpp:380
@@ +379,3 @@
+ // structure element) cannot appear in a private clause.
+ DeclRefExpr *DE = dyn_cast_or_null<DeclRefExpr>(*I);
+ if (!DE || !isa<VarDecl>(DE->getDecl())) {
----------------
Doug Gregor wrote:
> It doesn't have to be handled as part of this patch, but within templates
> you'll also need to handle DependentScopeDeclRefExprs, as well have
> VD->getType() being type-dependent.
Ok.
================
Comment at: lib/Sema/SemaOpenMP.cpp:422
@@ +421,3 @@
+ if (!CD ||
+ CheckConstructorAccess(ELoc, CD,
+ InitializedEntity::InitializeTemporary(Type),
----------------
Doug Gregor wrote:
> Should also check that CD is not deleted and somewhere check for
> unavailable/deprecated (e.g., via DiagnoseUseOfDecl). I suspect you'll end up
> building a call to the default constructor to stash in the AST, but that
> doesn't need to be part of this patch.
>
>
Ok.
================
Comment at: lib/Sema/SemaOpenMP.cpp:436
@@ +435,3 @@
+ CXXDestructorDecl *DD = RD->getDestructor();
+ if (DD && !DD->isTrivial()) {
+ if (CheckDestructorAccess(ELoc, DD, PD) == AR_inaccessible) {
----------------
Doug Gregor wrote:
> Why the trivial check?
Sorry, copied from another place.
================
Comment at: lib/Serialization/ASTWriterStmt.cpp:1713
@@ +1712,3 @@
+void ASTStmtWriter::VisitOMPParallelDirective(OMPParallelDirective *D) {
+ VisitOMPExecutableDirective(D);
+ Code = serialization::STMT_OMP_PARALLEL_DIRECTIVE;
----------------
Doug Gregor wrote:
> There are no tests for this code?
I'll add the test
http://llvm-reviews.chandlerc.com/D572
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
