ABataev added inline comments.
================
Comment at: clang/lib/Sema/Sema.cpp:2108
void Sema::PushCapturedRegionScope(Scope *S, CapturedDecl *CD, RecordDecl *RD,
CapturedRegionKind K) {
+ CapturedRegionScopeInfo *CSI = new CapturedRegionScopeInfo(
----------------
jdenny wrote:
> ABataev wrote:
> > ABataev wrote:
> > > jdenny wrote:
> > > > ABataev wrote:
> > > > > I would suggest adding a new parameter for CapturedLevel and the same
> > > > > parameter to `ActOnCapturedRegionStart` function. And pass the value
> > > > > for `CapturedLevel` explicitly at `ActOnCapturedRegionStart` function
> > > > > call in SemaOpenMP.cpp rather than rely on this solution.
> > > > > Also, for each separate OpenMP directive, this counter must start
> > > > > from `0`. Currently, I think, it may lead to crash if we have several
> > > > > directives. Something like:
> > > > > ```
> > > > > #pragma omp parallel
> > > > > #pragma omp target teams
> > > > > ...
> > > > > ```
> > > > > I think it will work incorrectly for this example.
> > > > I think the check that `OpenMPLevel` is the same prevents the problem
> > > > you mention.
> > > Ah, yes, missed this. Anyway, better to specify this explicitly as
> > > function parameters, I think, rather than trying to evaluate it here this
> > > way. We eventually may change the handling of captured regions in OpenMP,
> > > better to have everything in SemaOpenMP.cpp
> > BTW, here you can use the parameter with the default value `0`.
> OK.
>
> `CaptureLevel` or `CapturedLevel`? Do you care?
Up to you
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65835/new/
https://reviews.llvm.org/D65835
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
