ABataev added inline comments.
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:15201
+namespace {
+class LocatorChecker final : public StmtVisitor<LocatorChecker, bool> {
+ OMPClauseMappableExprCommon::MappableExprComponentList Components;
----------------
Seems to me, you're skipping many expressions, which could be supported. Did
you try to test it with casting expressions and others?
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:15203-15205
+ DeclRefExpr *DRE;
+ CXXThisExpr *CTE;
+ size_t DerefCnt;
----------------
Use default initializers for fields.
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:15213
+ bool VisitDeclRefExpr(DeclRefExpr *E) {
+ if (const auto *VD = dyn_cast<VarDecl>(E->getDecl())) {
+ const clang::Type *PT = E->getType().getTypePtr();
----------------
Better to use early exit where possible to reduce structural complexity of the
functions.
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:15215-15216
+ const clang::Type *PT = E->getType().getTypePtr();
+ const std::string TypeStr = PT->getCanonicalTypeInternal().getAsString();
+ size_t PtrCnt = std::count(TypeStr.begin(), TypeStr.end(), '*');
+ // Normally we want to add Components if DerefCnt == PtrCnt, for example:
----------------
This really looks ugly. Need to find a better solution.
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:15233
+ // PtrCnt for B: 0
+ if (DerefCnt <= PtrCnt) {
+ pushComponentIfNoBaseDecl(E, E->getDecl());
----------------
Why do you need to count the number of dereferences? And have this comparison
here?
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:15368
if (auto *CurE = dyn_cast<DeclRefExpr>(E)) {
+ if (!(isa<VarDecl>(CurE->getDecl())))
----------------
If you have the visitor class, you can replace all this code with an analysis
in the visitor.
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:15369
if (auto *CurE = dyn_cast<DeclRefExpr>(E)) {
- if (!isa<VarDecl>(CurE->getDecl()))
+ if (!(isa<VarDecl>(CurE->getDecl())))
return nullptr;
----------------
Restore original code
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:15554
+ LocatorChecker Checker;
+ if (Checker.Visit(OrigExpr)) {
+ llvm::copy(Checker.getComponents(),
----------------
General question about several cases. How we're going to support them?
1. (a ? b : c).
2. __builtin_choose_expr(a, b, c).
3. a = b.
4. a?:b
5. __builtin_convertvector(x, ty)
6. (int&)a
7. v.xy, where v is an extended vector
8. a.b, where b is a bitfield
9. __builtin_bit_cast(v, ty)
10. const_cast<ty &>(a)
11. dynamic_cast<ty &>(a)
12. reinterpret_cast
13. static_cast
14. typeid() (also is an lvalue).
15. __uuidof(*comPtr)
16. lambda calls
17. User defined literals
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:15555
+ if (Checker.Visit(OrigExpr)) {
+ llvm::copy(Checker.getComponents(),
+ std::back_inserter(CurComponents));
----------------
Better to pass `CurComponents` as an argument at the construction of the
checker to avoid extra filling/copying.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72811/new/
https://reviews.llvm.org/D72811
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
