ABataev added a comment. Herald added a subscriber: jdoerfert. Also, need some kind of the stubbed codegen for the mapper
================ Comment at: include/clang/AST/OpenMPClause.h:4236 + static OMPMapClause * + Create(const ASTContext &C, SourceLocation StartLoc, SourceLocation LParenLoc, + SourceLocation EndLoc, ArrayRef<Expr *> Vars, ---------------- The function has more than 10 parameters, it is not too good. Would be good to pack some of them into some kind of structure. ================ Comment at: include/clang/Sema/Sema.h:9347 ArrayRef<SourceLocation> MapTypeModifiersLoc, + CXXScopeSpec &MapperIdScopeSpec, DeclarationNameInfo &MapperId, OpenMPMapClauseKind MapType, bool IsMapTypeImplicit, ---------------- Also would be good to pack some of the params into structure ================ Comment at: include/clang/Sema/Sema.h:9431 + OMPClause *ActOnOpenMPMapClause( + ArrayRef<OpenMPMapModifierKind> MapTypeModifiers, + ArrayRef<SourceLocation> MapTypeModifiersLoc, ---------------- Same, too many params ================ Comment at: lib/Sema/SemaLookup.cpp:1074 + if (NameKind == LookupOMPMapperName) { + // Skip out-of-scope declarations. ---------------- Why do we need special processing of the mapper here? ================ Comment at: lib/Sema/SemaLookup.cpp:1793 continue; + else if (NameKind == LookupOMPMapperName) { + // Skip out-of-scope declarations. ---------------- Again, what's so special in mapper lookup? ================ Comment at: lib/Sema/SemaOpenMP.cpp:13186-13192 + if (ER.isInvalid()) { + continue; + } else if (ER.isUsable()) { + MVLI.UDMapperList.push_back(ER.get()); + } else { + MVLI.UDMapperList.push_back(nullptr); + } ---------------- I think this can be simplified: ``` if (ER.isInvalid()) continue; MVLI.UDMapperList.push_back(ER.get()); ``` ================ Comment at: lib/Sema/SemaOpenMP.cpp:13232-13238 + if (ER.isInvalid()) { + continue; + } else if (ER.isUsable()) { + MVLI.UDMapperList.push_back(ER.get()); + } else { + MVLI.UDMapperList.push_back(nullptr); + } ---------------- Also can be simplified. ================ Comment at: lib/Sema/SemaOpenMP.cpp:13363-13369 + if (ER.isInvalid()) { + continue; + } else if (ER.isUsable()) { + MVLI.UDMapperList.push_back(ER.get()); + } else { + MVLI.UDMapperList.push_back(nullptr); + } ---------------- Again, simplify ================ Comment at: lib/Sema/TreeTransform.h:8845 + } else + UnresolvedMappers.push_back(nullptr); + } ---------------- Enclose into braces ================ Comment at: lib/Serialization/ASTReader.cpp:12312 + UDMappers.reserve(NumVars); + for (unsigned i = 0; i < NumVars; ++i) + UDMappers.push_back(Record.readExpr()); ---------------- `i`->`I` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58074/new/ https://reviews.llvm.org/D58074 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits