Meinersbur marked 8 inline comments as done. Meinersbur added inline comments.
================ Comment at: clang/include/clang/AST/StmtOpenMP.h:463-478 + static const SpecificClause *getSingleClause(ArrayRef<OMPClause *> Clauses) { + auto ClausesOfKind = getClausesOfKind<SpecificClause>(Clauses); - if (Clauses.begin() != Clauses.end()) { - assert(std::next(Clauses.begin()) == Clauses.end() && + if (ClausesOfKind.begin() != ClausesOfKind.end()) { + assert(std::next(ClausesOfKind.begin()) == ClausesOfKind.end() && "There are at least 2 clauses of the specified kind"); + return *ClausesOfKind.begin(); ---------------- ABataev wrote: > Better to commit it it separately as NFC. D103665 ================ Comment at: clang/lib/Sema/SemaOpenMP.cpp:10139-10142 +/// Find and diagnose mutually exclusive clause kinds. +static bool checkMutuallyExclusiveClauses( + Sema &S, ArrayRef<OMPClause *> Clauses, + ArrayRef<OpenMPClauseKind> MutuallyExclusiveClauses) { ---------------- ABataev wrote: > Would be good if this change is committed separately as NFC. D103666 ================ Comment at: clang/lib/Sema/SemaOpenMP.cpp:12821 +/// Determine whether an expression is constant without emitting diagnostic. +static bool isConstantExpression(Sema &SemaRef, Expr *E) { + struct ConstTripcountDiagnoser : public Sema::VerifyICEDiagnoser { ---------------- ABataev wrote: > Can you use `VerifyPositiveIntegerConstantInClause` function instead? The reason for this new function is that `VerifyPositiveIntegerConstantInClause` also emits a note on why it is not a constant expression: ``` note: read of non-const variable '.capture_expr.' is not allowed in a constant expression ``` Printing internal variable names is more confusing than helpful to the user. ================ Comment at: clang/lib/Sema/SemaOpenMP.cpp:12964 + assert(Factor > 0 && "Expected positive unroll factor"); + auto makeFactorExpr = [this, Factor, IVTy, FactorLoc]() { + return IntegerLiteral::Create( ---------------- ABataev wrote: > Why do you need it? Just use original `PartialClause->getFactor()` The AST invariant that no ASTNode can be used multiple times (within the same DeclContext) must be preserved. This is a utility lambda (like the other `MakeXyz`) that create a new IntegerLiteral node for ever use. See discussion here: https://reviews.llvm.org/D94973?id=322600#inline-905341 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99459/new/ https://reviews.llvm.org/D99459 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits