[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.


================
Comment at: unittests/AST/OMPStructuredBlockTest.cpp:64
+++i;
+})";
+  ASSERT_TRUE(
----------------
Same comment as in the other patch -- I would prefer that the source is inlined into the ASSERT_TRUE, with implicit string concatenation and "\n"s, if raw strings don't work.


================
Comment at: unittests/AST/OMPStructuredBlockTest.cpp:75
+  ASSERT_TRUE(PrintedOMPStmtMatches(Source, OMPStandaloneDirectivekMatcher(),
+                                    "#pragma omp barrier\n"));
+}
----------------
Could you also add an assertion that `OMPStructuredBlockMatcher()` matches zero AST nodes?  Similarly in every test that has no structured blocks.


================
Comment at: unittests/AST/OMPStructuredBlockTest.cpp:121
+
+TEST(OMPStructuredBlock, TestDistributeParallelForSimdDirective0) {
+  const char *Source =
----------------
This test file repeats this pattern many times, only with different OpenMP directives:

TEST(OMPStructuredBlock, Test~~~Directive0)
TEST(OMPStructuredBlock, Test~~~Directive1)
TEST(OMPStructuredBlock, Test~~~DirectiveCollapse1)
TEST(OMPStructuredBlock, Test~~~DirectiveCollapse2)
TEST(OMPStructuredBlock, Test~~~DirectiveCollapse22)

WDYT about deduplicating it using parameterized tests?  For example, see AssignmentTest in `llvm/tools/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp`.


================
Comment at: unittests/AST/OMPStructuredBlockTest.cpp:656
+
+TEST(OMPStructuredBlock, DISABLED_TestParallelMaster0XFAIL) {
+  const char *Source =
----------------
Is it only a pretty-printer problem or something more severe?


================
Comment at: unittests/AST/OMPStructuredBlockTest.cpp:704
+
+StatementMatcher OMPSectionsDirectivekMatcher() {
+  return stmt(
----------------
Extra "k" before "Matcher".


================
Comment at: unittests/AST/OMPStructuredBlockTest.cpp:712
+
+StatementMatcher OMPSectionDirectivekMatcher() {
+  return stmt(isOMPStructuredBlock(),
----------------
Extra "k" before "Matcher".


================
Comment at: unittests/AST/OMPStructuredBlockTest.cpp:1481
+
+TEST(OMPStructuredBlock, TesteamsDistributeParallelForSimdDirective0) {
+  const char *Source =
----------------
"TestTeams"


================
Comment at: unittests/AST/PrintTest.h:1
+//===- unittests/AST/PrintTest.h ------------------------------------------===//
+//
----------------
"ASTPrint.h"?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59214/new/

https://reviews.llvm.org/D59214