[PATCH] D74970: [OpenMP] Refactor the analysis in checkMapClauseBaseExpression using StmtVisitor class.

estewart08 added inline comments. Herald added subscribers: sstefan1, yaxunl. Herald added a project: All. Comment at: clang/lib/Sema/SemaOpenMP.cpp:15450 } + bool VisitCXXThisExpr(CXXThisExpr *CTE) { return true; } + bool VisitStmt(Stmt *) { cchen wrote: >

[PATCH] D74970: [OpenMP] Refactor the analysis in checkMapClauseBaseExpression using StmtVisitor class.

This revision was automatically updated to reflect the committed changes. Closed by commit rGd66d25f83824: [OpenMP] Refactor the analysis in checkMapClauseBaseExpression using… (authored by cchen, committed by ABataev). Changed prior to commit:

[PATCH] D74970: [OpenMP] Refactor the analysis in checkMapClauseBaseExpression using StmtVisitor class.

ABataev added a comment. Sure. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74970/new/ https://reviews.llvm.org/D74970 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D74970: [OpenMP] Refactor the analysis in checkMapClauseBaseExpression using StmtVisitor class.

cchen added a comment. @ABataev, can you land it for me when you have time? Thanks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74970/new/ https://reviews.llvm.org/D74970 ___ cfe-commits mailing list

[PATCH] D74970: [OpenMP] Refactor the analysis in checkMapClauseBaseExpression using StmtVisitor class.

ABataev accepted this revision. ABataev added a comment. This revision is now accepted and ready to land. Thanks, LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74970/new/ https://reviews.llvm.org/D74970

[PATCH] D74970: [OpenMP] Refactor the analysis in checkMapClauseBaseExpression using StmtVisitor class.

cchen updated this revision to Diff 245995. cchen added a comment. Update test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74970/new/ https://reviews.llvm.org/D74970 Files: clang/lib/Sema/SemaOpenMP.cpp clang/test/OpenMP/target_messages.cpp

[PATCH] D74970: [OpenMP] Refactor the analysis in checkMapClauseBaseExpression using StmtVisitor class.

ABataev added a comment. In D74970#1887337 , @cchen wrote: > Fix test and sorry for my carelessness No problem. But I don't see the changes in the test. Comment at: clang/lib/Sema/SemaOpenMP.cpp:15473 +

[PATCH] D74970: [OpenMP] Refactor the analysis in checkMapClauseBaseExpression using StmtVisitor class.

cchen updated this revision to Diff 245983. cchen added a comment. Fix test and sorry for my carelessness Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74970/new/ https://reviews.llvm.org/D74970 Files: clang/lib/Sema/SemaOpenMP.cpp

[PATCH] D74970: [OpenMP] Refactor the analysis in checkMapClauseBaseExpression using StmtVisitor class.

ABataev added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:15358-15360 + if (!AE->getIdx()->isValueDependent()) +if (AE->getIdx()->EvaluateAsInt(Result, SemaRef.getASTContext())) { if (!Result.Val.getInt().isNullValue()) {

[PATCH] D74970: [OpenMP] Refactor the analysis in checkMapClauseBaseExpression using StmtVisitor class.

cchen updated this revision to Diff 245978. cchen added a comment. Add test for mapping function Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74970/new/ https://reviews.llvm.org/D74970 Files: clang/lib/Sema/SemaOpenMP.cpp

[PATCH] D74970: [OpenMP] Refactor the analysis in checkMapClauseBaseExpression using StmtVisitor class.

cchen added a comment. In D74970#1887252 , @ABataev wrote: > In D74970#1887246 , @cchen wrote: > > > In D74970#1887193 , @ABataev wrote: > > > > > Did you try to run the

[PATCH] D74970: [OpenMP] Refactor the analysis in checkMapClauseBaseExpression using StmtVisitor class.

ABataev added a comment. In D74970#1887246 , @cchen wrote: > In D74970#1887193 , @ABataev wrote: > > > Did you try to run the tests? I would suggest adding a test, at least, > > where a function is mapped. We

[PATCH] D74970: [OpenMP] Refactor the analysis in checkMapClauseBaseExpression using StmtVisitor class.

cchen marked an inline comment as done. cchen added a comment. In D74970#1887193 , @ABataev wrote: > Did you try to run the tests? I would suggest adding a test, at least, where > a function is mapped. We should see the error message here. Seems to me,

[PATCH] D74970: [OpenMP] Refactor the analysis in checkMapClauseBaseExpression using StmtVisitor class.

ABataev added a comment. Did you try to run the tests? I would suggest adding a test, at least, where a function is mapped. We should see the error message here. Seems to me, we don't have this one. Comment at: clang/lib/Sema/SemaOpenMP.cpp:15422-15423 +

[PATCH] D74970: [OpenMP] Refactor the analysis in checkMapClauseBaseExpression using StmtVisitor class.

cchen updated this revision to Diff 245955. cchen added a comment. Fix by feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74970/new/ https://reviews.llvm.org/D74970 Files: clang/lib/Sema/SemaOpenMP.cpp Index:

[PATCH] D74970: [OpenMP] Refactor the analysis in checkMapClauseBaseExpression using StmtVisitor class.

cchen marked 11 inline comments as done. cchen added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:15450 } + bool VisitCXXThisExpr(CXXThisExpr *CTE) { return true; } + bool VisitStmt(Stmt *) { ABataev wrote: > Do you really need this function?

[PATCH] D74970: [OpenMP] Refactor the analysis in checkMapClauseBaseExpression using StmtVisitor class.

ABataev added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:15319 } + return Visit(E); +} `return RelevantExpr || Visit(E);`? Comment at: clang/lib/Sema/SemaOpenMP.cpp:15346 } + return Visit(E); +}

[PATCH] D74970: [OpenMP] Refactor the analysis in checkMapClauseBaseExpression using StmtVisitor class.

cchen updated this revision to Diff 245912. cchen added a comment. Fix based on feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74970/new/ https://reviews.llvm.org/D74970 Files: clang/lib/Sema/SemaOpenMP.cpp Index: