abidmalikwaterloo marked 4 inline comments as done. abidmalikwaterloo added inline comments.
================ Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:10852 +def err_omp_misplaced_default_clause : Error< + "misplaced default clause! Only one default clause is allowed in" + "metadirective in the end">; ---------------- jdoerfert wrote: > ABataev wrote: > > We do not use explamations in messages, better to have something `only > > single default ...` > "allowed in" what? Also, there is no test for this, or is there? It should be "Only one default clause is allowed. ================ Comment at: clang/lib/AST/OpenMPClause.cpp:1679 + OS << ": "; +} + ---------------- jdoerfert wrote: > I'm assuming we already have a printer for trait selectors, no? Doesn't > `OMPTraitInfo::print` do this already and actually handle scores? Looked into the function. `OMPTraitInfo::print` can be used. The function needs to be extended as well to take the other traits as well. ================ Comment at: clang/lib/Sema/SemaOpenMP.cpp:7439 + } + } + ---------------- jdoerfert wrote: > Why does this perform partial trait matching? We should have code for this. > Also, the logic for device_arch and vendor (which is most what there is), is > not what we want. Reuse the existing matching logic instead. Ok. What do you mean by `existing matching logic`? ================ Comment at: clang/lib/Sema/SemaStmt.cpp:4795 + + CD->setBody(Res->getCapturedStmt()); RD->completeDefinition(); ---------------- jdoerfert wrote: > Unrelated. Please go over the patch and avoid these in the future. We have so > many comments that point out these things that now the comments make it hard > to read/review. An accidental tap. Removed Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122255/new/ https://reviews.llvm.org/D122255 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
