gribozavr added inline comments.
================
Comment at: include/clang/AST/StmtOpenMP.h:266
+
+ /// Returns the AST statement representing OpenMP structured-block of this
+ /// OpenMP executable directive,
----------------
"the AST node"
================
Comment at: include/clang/AST/StmtOpenMP.h:2158
+ /// OpenMP flush construct is a stand-alone directive.
+ llvm::Optional<Stmt *> getStructuredBlockImpl() const { return llvm::None; };
----------------
This comment (and other similar comments on getStructuredBlockImpl()) are
implementation comments and should be written in the function body.
================
Comment at: include/clang/Basic/OpenMPKinds.def:18
+#ifndef OPENMP_EXECUTABLE_DIRECTIVE
+# define OPENMP_EXECUTABLE_DIRECTIVE(Name, Class)
+#endif
----------------
Why not add a default definition:
```
# define OPENMP_EXECUTABLE_DIRECTIVE(Name, Class) OPENMP_DIRECTIVE(Name)
```
(Also for `OPENMP_EXECUTABLE_DIRECTIVE_EXT` below.)
================
Comment at: test/AST/ast-dump-openmp-atomic.c:1
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fopenmp -ast-dump %s |
FileCheck -strict-whitespace -implicit-check-not=openmp_structured_block %s
+
----------------
I'm not a fan of ast-dump tests. They are fragile, difficult to update,
difficult to read (when they are 700 lines long), and it is unclear what the
important parts are.
WDYT about converting them to unit tests? See
`clang/unittests/AST/StmtPrinterTest.cpp` for an example.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59214/new/
https://reviews.llvm.org/D59214
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
