ABataev added inline comments.

================
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:
> > 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;
> > ```
> I understood what you want it to look like. However, that style makes it 
> impossible to add additional diagnostics to this function
Let's think about it later, if(!) some changes will be required


https://reviews.llvm.org/D30135



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to