ABataev added inline comments.
================ Comment at: lib/Sema/SemaOpenMP.cpp:1959 +static bool CheckCancelRegion(Sema &SemaRef, OpenMPDirectiveKind CurrentRegion, + OpenMPDirectiveKind CancelRegion, ---------------- Hahnfeld wrote: > ABataev wrote: > > Should be `checkCancelRegion` > It's also `CheckNestingOfRegions`, but can do Old formatting, this should be fixed ================ Comment at: lib/Sema/SemaOpenMP.cpp:1968-1973 + if (CancelRegion != OMPD_parallel && CancelRegion != OMPD_for && + CancelRegion != OMPD_sections && CancelRegion != OMPD_taskgroup) { + SemaRef.Diag(StartLoc, diag::err_omp_wrong_cancel_region) + << getOpenMPDirectiveName(CancelRegion); + return true; + } ---------------- Hahnfeld wrote: > ABataev wrote: > > It is better to convert this to return `false` and make error message and > > `return true` statement unconditional > I wanted to keep the style in `CheckNestingOfRegions`: That way it's easier > to add more checks later on if needed like additional restrictions on > `CancelRegion` I just meant that it's better to make it look like this: ``` if (CancelRegion == OMPD_parallel || CancelRegion == OMPD_for || CancelRegion == OMPD_sections || CancelRegion == OMPD_taskgroup) return false; SemaRef.Diag(StartLoc, diag::err_omp_wrong_cancel_region) << getOpenMPDirectiveName(CancelRegion); return true; ``` https://reviews.llvm.org/D30135 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits