fraggamuffin marked 14 inline comments as done. fraggamuffin added a comment.
In http://reviews.llvm.org/D15321#304557, @ABataev wrote: > Michael, please provide full diff log, as described in > http://llvm.org/docs/Phabricator.html (with full context) Ok, no problem. Sorry I forgot. Thanks for the quick review. I only have the other 2 parts to address which is the attribute suggestion and the Helper class which you said should be removed. Thanks again. ================ Comment at: include/clang/AST/DeclBase.h:164 @@ -163,3 +163,3 @@ /// has been declared outside any function. - IDNS_LocalExtern = 0x0800 + IDNS_LocalExtern = 0x0800, }; ---------------- ABataev wrote: > I don't think this is required, revert it back please This was there before. I am assuming you just want me to revert back the comma. Done. ================ Comment at: lib/Parse/ParseOpenMP.cpp:209 @@ -142,2 +208,3 @@ case OMPD_taskloop_simd: + default: Diag(Tok, diag::err_omp_unexpected_directive) ---------------- ABataev wrote: > Do not add default:, coding standard recommends to not use it This is needed to generate these error messages in declare_target_messages.cpp error: 'error' diagnostics expected but not seen: File C:\llvm151206\llvm\tools\clang\test\OpenMP\declare_target_messages.cpp Li ne 3: unexpected OpenMP directive '#pragma omp end declare target' File C:\llvm151206\llvm\tools\clang\test\OpenMP\declare_target_messages.cpp Li ne 63: unexpected OpenMP directive '#pragma omp end declare target' File C:\llvm151206\llvm\tools\clang\test\OpenMP\declare_target_messages.cpp Li ne 72: unexpected OpenMP directive '#pragma omp end declare target' 3 errors generated. ================ Comment at: lib/Parse/ParseOpenMP.cpp:387-392 @@ -318,2 +386,8 @@ break; + default: + Diag(Tok, diag::err_omp_unexpected_directive) + << getOpenMPDirectiveName(DKind); + while (!SkipUntil(tok::annot_pragma_openmp_end)) + ; + break; } ---------------- ABataev wrote: > Do not add it This is needed to prevent infinite loop when there is an unexpected end of the pragma omp target. ================ Comment at: lib/Sema/SemaOpenMP.cpp:1139-1140 @@ -1105,3 +1138,4 @@ QualType ExprType = VD->getType().getNonReferenceType(); - ExprResult DE = buildDeclRefExpr(*this, VD, ExprType, Id.getLoc()); + //ExprResult DE = buildDeclRefExpr(*this, VD, ExprType, Id.getLoc()); + ExprResult DE = BuildDeclRefExpr(VD, ExprType, VK_LValue, Id.getLoc()); return DE; ---------------- ABataev wrote: > I don't understand why you changed this. The buildDeclRefExpr interface does not enable us to generate these messages. Someone changed it to buildDeclRefExpr(...) which has a different interface In the github repository it also uses ExprResult DE = BuildDeclRefExpr(VD, ExprType, VK_LValue, Id.getLoc()); With the buildDeclRefExpr interface, it misses generation of these messages error: 'warning' diagnostics expected but not seen: File C:\llvm151206\llvm\tools\clang\test\OpenMP\declare_target_messages.cpp Li ne 5: declaration is not declared in any declare target region error: 'note' diagnostics expected but not seen: File C:\llvm151206\llvm\tools\clang\test\OpenMP\declare_target_messages.cpp Li ne 21: used here 2 errors generated. Repository: rL LLVM http://reviews.llvm.org/D15321 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
