ABataev added inline comments.
================ Comment at: include/clang/Basic/DiagnosticSemaKinds.td:9136 +def err_omp_target_before_requires : Error < + "Target region encountered before requires directive with %0 clause">; +def note_omp_requires_encountered_target : Note < ---------------- Enclose `%0` in single quotes for better reading ================ Comment at: lib/Sema/SemaOpenMP.cpp:2440 + // Check if any of the requires clauses affect target regions. + if (!TargetLocations.empty() && + (isa<OMPUnifiedSharedMemoryClause>(CNew) || ---------------- Better to check for `TargetLocations.empty()` before the loop ================ Comment at: lib/Sema/SemaOpenMP.cpp:4207 + DSAStack->hasRequiresDeclWithClause<OMPDynamicAllocatorsClause>()) && + !CurContext->isDependentContext()) { + // Register target to DSA Stack. ---------------- Better to check for the dependent context at first, only after that check for the clause ================ Comment at: test/OpenMP/requires_target_messages.cpp:11 + +#pragma omp requires unified_shared_memory //expected-error {{Target region encountered before requires directive with unified_shared_memory clause}} ---------------- Add the checks for all the clauses that may cause this error Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60875/new/ https://reviews.llvm.org/D60875 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits