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);
+ }
----------------
`return RelevantExpr || Visit(E);`?
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:15358-15359
+ Expr::EvalResult Result;
+ if (AE->isValueDependent())
+ return false;
+ if (AE->getIdx()->EvaluateAsInt(Result, SemaRef.getASTContext())) {
----------------
1. Need to check `AE->getIdx()`, not `AE`.
2. Why return `false` here? I would say, just skip the check.
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:15374
- QualType CurType =
- OMPArraySectionExpr::getBaseOriginalType(E).getCanonicalType();
+ return E && Visit(E);
+ }
----------------
`return RelevantExpr || Visit(E);`?
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:15423-15424
+ Expr::EvalResult ResultL;
+ if (OASE->isValueDependent())
+ return false;
+ if (OASE->getLength()->EvaluateAsInt(ResultR,
----------------
1. Need to check `OASE->getLength()`, not `OASE`.
2. Why return `false` here? I would say, just skip the check.
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:15434
}
-
- // Record the component - we don't have any declaration associated.
- CurComponents.emplace_back(CurE, nullptr);
- } else {
- if (!NoDiagnose) {
- // If nothing else worked, this is not a valid map clause expression.
- SemaRef.Diag(
- ELoc, diag::err_omp_expected_named_var_member_or_array_expression)
- << ERange;
+ if (OASE->getLowerBound() && OASE->getLowerBound()->EvaluateAsInt(
+ ResultL, SemaRef.getASTContext())) {
----------------
Need a check for value-dependent `OASE->getLowerBound()`
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:15448
+ Components.emplace_back(OASE, nullptr);
+ return RelevantExpr || Visit(E->IgnoreParenImpCasts());
}
----------------
Just `Visit(E)`;
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:15450
}
+ bool VisitCXXThisExpr(CXXThisExpr *CTE) { return true; }
+ bool VisitStmt(Stmt *) {
----------------
Do you really need this function?
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:15467-15470
+// Return the expression of the base of the mappable expression or null if it
+// cannot be determined and do all the necessary checks to see if the
expression
+// is valid as a standalone mappable expression. In the process, record all the
+// components of the expression.
----------------
Use `\\\` style of comment here
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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
