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

Alexey Bataev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Feb 21 12:22:28 PST 2020


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
+      Expr::EvalResult ResultL;
+      if (!OASE->getLength()->isValueDependent()) {
+        if (OASE->getLength()->EvaluateAsInt(ResultR,
                                              SemaRef.getASTContext())) {
----------------
Just a nit. Try to reduce the structural complexity here by using logical exressions, `if (!OASE->getLength()->isValueDependent() && OASE->getLength()->EvaluateAsInt(...) && !ResultR.Val.getInt().isOneValue())`. Also, you can 


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:15433-15436
+      if (OASE->getLowerBound() && !OASE->getLowerBound()->isValueDependent()) {
+        if (OASE->getLowerBound()->EvaluateAsInt(
+                                         ResultL, SemaRef.getASTContext())) {
           if (!ResultL.Val.getInt().isNullValue()) {
----------------
Same here, try to reduce complexity


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74970/new/

https://reviews.llvm.org/D74970





More information about the cfe-commits mailing list