ABataev added inline comments.
================
Comment at: clang/lib/Parse/ParseOpenMP.cpp:2134-2151
+ Sema::DeclareTargetContextInfo *DTCI =
+ new Sema::DeclareTargetContextInfo(DKind, DTLoc);
+ if (HasClauses)
+ ParseOMPDeclareTargetClauses(*DTCI);
// Skip the last annot_pragma_openmp_end.
ConsumeAnyToken();
----------------
jdoerfert wrote:
> ABataev wrote:
> > jdoerfert wrote:
> > > ABataev wrote:
> > > > Add a RAII to control these new/delete or just create local var
> > > Local variable and RAII do not always work, there is a delete in the
> > > `end_declare_target` below as well.
> > >
> > >
> > But here you have new/delete pair within a single block. Either you can put
> > the ar into stack/RAII or some extra checks are missing. Maybe must be
> > something like this:
> > ```
> > if (DKind == OMPD_declare_target)
> > delete DTCI;
> > ```
> > ?
> If this is a directive without implicit mappings, the DTCI will be deleted
> right away. It is just used to collect the clauses (which include explicit
> mappings). If this is a directive with implicit mappings, which we cannot
> tell from the DKind alone, the DTCI will be stored in Sema until we reach the
> corresponding end directive. For the former case we could do stack/RAII, but
> not for the latter case, the scope is left and DTCI survives. I'll look into
> this again and try to avoid new/delete, feel free to look at the rest of the
> patch.
Everything else looks good, just need to clarify the question with this
new/delete. I missed `if (HasImplicitMappings)` check, so maybe this is correct
but better to double-check.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D101030/new/
https://reviews.llvm.org/D101030
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
