ABataev added a comment. In D144634#4308872 <https://reviews.llvm.org/D144634#4308872>, @koops wrote:
> 1. Taken care of some of the comments by Alexey. > 2. Added extra tests of loop_bind_messages.cpp > > However, generic_loop_messages.cpp & generic_loop_ast_print.cpp are present > and provide a good coverage. I rather doubt that these tests provide good coverage, since you're changing the directive kind here on the fly. This is a very new functionality, which was not tested before. Add the tests for nesting of the regions, i.e. loop bind directive, enclosed in different regions, and diagnostics. ================ Comment at: clang/lib/Sema/SemaOpenMP.cpp:339 + OpenMPDirectiveKind Map1D = OMPD_unknown; + ---------------- What Ma1D stands for, what is the meaning of this abbrev? Add a comment. ================ Comment at: clang/test/OpenMP/loop_bind_codegen.cpp:2 +// expected-no-diagnostics +// RUN: %clang_cc1 -DCK1 -verify -fopenmp -x c++ %s -emit-llvm -o - | FileCheck %s +// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s ---------------- Add tests with save/load precompiled code. ================ Comment at: clang/test/OpenMP/loop_bind_messages.cpp:3 +#define HEADER +// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -verify %s + ---------------- Add tests with save/load precompiled code. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144634/new/ https://reviews.llvm.org/D144634 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits