ABataev added inline comments.
================
Comment at: clang/include/clang/AST/OpenMPClause.h:8859
+/// \code
+/// #pragma omp metadirective when(user={consition(N<100)}:parallel for)
+/// \endcode
----------------
condition?
================
Comment at: clang/include/clang/AST/OpenMPClause.h:8893-8894
+
+ /// Sets the location of '('.
+ void setLParenLoc(SourceLocation Loc) { LParenLoc = Loc; }
+
----------------
make it private
================
Comment at: clang/include/clang/AST/OpenMPClause.h:8900
+ /// Returns the directive variant kind
+ OpenMPDirectiveKind getDKind() { return DKind; }
+
----------------
const
================
Comment at: clang/include/clang/AST/OpenMPClause.h:8905
+ /// Returns the OMPTraitInfo
+ OMPTraitInfo &getTI() { return *TI; }
+
----------------
const
================
Comment at: clang/include/clang/AST/StmtOpenMP.h:5479
EmptyShell);
+
Stmt *getIfStmt() const { return IfStmt; }
----------------
Remove this change
================
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">;
----------------
We do not use explamations in messages, better to have something `only single
default ...`
================
Comment at: clang/include/clang/Sema/Sema.h:10688
Stmt *AStmt, SourceLocation StartLoc, SourceLocation EndLoc);
+ StmtResult ActOnOpenMPExecutableMetaDirective(
+ OpenMPDirectiveKind Kind, const DeclarationNameInfo &DirName,
----------------
Comment for this new function
================
Comment at: clang/include/clang/Sema/Sema.h:11138
+ OMPClause *ActOnOpenMPWhenClause(OMPTraitInfo &TI, OpenMPDirectiveKind DKind,
+ StmtResult Directive,
+ SourceLocation StartLoc,
----------------
formatting
================
Comment at: clang/lib/AST/OpenMPClause.cpp:1614-1617
+ if (Node->getTI().Sets.size() == 0) {
+ OS << "default(";
+ return;
+ }
----------------
Is this correct? Just `default(` is expected to be printed?
================
Comment at: clang/lib/AST/StmtPrinter.cpp:658
bool ForceNoStmt) {
+
OMPClausePrinter Printer(OS, Policy);
----------------
Do not insert empty line here
================
Comment at: clang/lib/AST/StmtPrinter.cpp:661
ArrayRef<OMPClause *> Clauses = S->clauses();
- for (auto *Clause : Clauses)
+ for (auto *Clause : Clauses){
if (Clause && !Clause->isImplicit()) {
----------------
formatting
================
Comment at: clang/lib/AST/StmtPrinter.cpp:665
Printer.Visit(Clause);
+ if (dyn_cast<OMPMetaDirective>(S)){
+
----------------
use `isa`
================
Comment at: clang/lib/AST/StmtPrinter.cpp:667
+
+ OMPWhenClause *c = dyn_cast<OMPWhenClause>(Clause);
+ if (c!=NULL){
----------------
Camel naming style expceted
================
Comment at: clang/lib/AST/StmtPrinter.cpp:668
+ OMPWhenClause *c = dyn_cast<OMPWhenClause>(Clause);
+ if (c!=NULL){
+ if (c->getDKind() != llvm::omp::OMPD_unknown){
----------------
formatting, use nullptr instead of NULL (or just `C`)
================
Comment at: clang/lib/AST/StmtPrinter.cpp:669
+ if (c!=NULL){
+ if (c->getDKind() != llvm::omp::OMPD_unknown){
+ Printer.VisitOMPWhenClause(c);
----------------
Formatting
================
Comment at: clang/lib/Parse/ParseOpenMP.cpp:2433
Parser::ParseOpenMPDeclarativeOrExecutableDirective(ParsedStmtContext StmtCtx)
{
+// need to check about the following
static bool ReadDirectiveWithinMetadirective = false;
----------------
What is this?
================
Comment at: clang/lib/Parse/ParseOpenMP.cpp:2474
tok::annot_pragma_openmp_end);
+
while (Tok.isNot(tok::annot_pragma_openmp_end)) {
----------------
Restore original code here
================
Comment at: clang/lib/Parse/ParseOpenMP.cpp:2498-2506
+ // Parse ':' // You have parsed the OpenMP Context in the meta
directive
if (Tok.is(tok::colon))
ConsumeAnyToken();
else {
Diag(Tok, diag::err_omp_expected_colon) << "when clause";
TPA.Commit();
return Directive;
----------------
Remove this extra stuff
================
Comment at: clang/lib/Parse/ParseOpenMP.cpp:2546
+ // Array A will be used for sorting
+ SmallVector<std::pair<unsigned, llvm::APInt>> A;
+
----------------
What `A` stands after, what does it mean?
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
