[PATCH] D83261: [OPENMP]Redesign of OMPExecutableDirective representation.

2020-08-18 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. In D83261#2224619 , @cchen wrote: > In PR45212 comment 5 (https://bugs.llvm.org/show_bug.cgi?id=45212#c5), there > is a reduced test case that failed after adding this patch. The assertion is > from OMPChildren::getInnermostCaptu

[PATCH] D83261: [OPENMP]Redesign of OMPExecutableDirective representation.

2020-08-18 Thread Chi Chun Chen via Phabricator via cfe-commits
cchen added a comment. In PR45212 comment 5 (https://bugs.llvm.org/show_bug.cgi?id=45212#c5), there is a reduced test case that failed after adding this patch. The assertion is from OMPChildren::getInnermostCapturedStmt. Test case: #include #include #include class Myclass {

[PATCH] D83261: [OPENMP]Redesign of OMPExecutableDirective representation.

2020-08-05 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:3329 } - OMPDeclareMapperDecl *NewDMD = SemaRef.ActOnOpenMPDeclareMapperDirectiveStart( - /*S=*/nullptr, Owner, D->getDeclName(), SubstMapperTy, D->getLocation(),

[PATCH] D83261: [OPENMP]Redesign of OMPExecutableDirective representation.

2020-08-05 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur added a comment. I think the ActOnOpenMPDeclareMapperDirective changes should be separated into its own patch. Otherwise, LGTM. Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:3329 } - OMPDeclareMapperDecl *NewDMD = SemaRef.ActOnOpenMPDeclareMapperDi

[PATCH] D83261: [OPENMP]Redesign of OMPExecutableDirective representation.

2020-08-03 Thread Bruno Ricci via Phabricator via cfe-commits
riccibruno added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:345 Diag(Loc, diag::err_omp_declare_mapper_wrong_var) -<< DMD->getVarName().getAsString(); +<< getOpenMPDeclareMapperVarName(); Diag(D->getLocation(), diag::note_entity_declared_at

[PATCH] D83261: [OPENMP]Redesign of OMPExecutableDirective representation.

2020-07-22 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. In D83261#2166766 , @Meinersbur wrote: > In D83261#2162561 , @ABataev wrote: > > > 1. OMPChildren class uses standard TrailingObjects harness instead of > > manual calculation. > > > Note t

[PATCH] D83261: [OPENMP]Redesign of OMPExecutableDirective representation.

2020-07-22 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur added a comment. In D83261#2162561 , @ABataev wrote: > 1. OMPChildren class uses standard TrailingObjects harness instead of manual > calculation. Note that that having a separate object defeats the purpose of `TrailingObjects` of having jus

[PATCH] D83261: [OPENMP]Redesign of OMPExecutableDirective representation.

2020-07-21 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. In D83261#2162904 , @Meinersbur wrote: > In D83261#2162561 , @ABataev wrote: > > > 1. OMPChildren class uses standard TrailingObjects harness instead of > > manual calculation. > > > I was

[PATCH] D83261: [OPENMP]Redesign of OMPExecutableDirective representation.

2020-07-20 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur added a comment. In D83261#2162561 , @ABataev wrote: > 1. OMPChildren class uses standard TrailingObjects harness instead of manual > calculation. I was going to argue that OMPExecutableDirective could derive from TrailingObjects as well, bu

[PATCH] D83261: [OPENMP]Redesign of OMPExecutableDirective representation.

2020-07-20 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. In D83261#2161217 , @Meinersbur wrote: > AFAICS this extract out the handling of subnodes of OMPExecutableDirectives > into the OMPChildren class which is made optional since `OMPChildren > *OMPExecutableDirectives::Data` can be

[PATCH] D83261: [OPENMP]Redesign of OMPExecutableDirective representation.

2020-07-19 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur added a comment. AFAICS this extract out the handling of subnodes of OMPExecutableDirectives into the OMPChildren class which is made optional since `OMPChildren *OMPExecutableDirectives::Data` can be nullptr. However, since it also stores clauses, it seems that about every executabl