[PATCH] D81736: [openmp] Base of tablegen generated OpenMP common declaration

[Valentin Clement via Phabricator via llvm-commits]

clementval marked 10 inline comments as done.
clementval added a comment.

@jdenny I will have a look at your suggestion to dump the name instead of the id. If it looks straight forward I will do it. Other comments were addressed. Thanks.



================
Comment at: clang/test/AST/ast-dump-openmp-target-parallel-for.c:105
 // CHECK-NEXT: |       | | `-FieldDecl {{.*}} <line:5:3> col:3 implicit 'int'
-// CHECK-NEXT: |       | |   `-OMPCaptureKindAttr {{.*}} <<invalid sloc>> Implicit 9
+// CHECK-NEXT: |       | |   `-OMPCaptureKindAttr {{.*}} <<invalid sloc>> Implicit 23
 // CHECK-NEXT: |       | `-CapturedDecl {{.*}} <<invalid sloc>> <invalid sloc> nothrow
----------------
jdenny wrote:
> Why 23 not `{{.*}}` like the others?
Good catch!


================
Comment at: llvm/include/llvm/Frontend/Directive/DirectiveBase.td:22
+  //
+  // By default, uses the name of the dialect as the only namespace. To avoid
+  // placing in any namespace, use "". To specify nested namespaces, use "::"
----------------
jdenny wrote:
> "dialect" -> "directive language" as above?
Should be "directive language". copy-paste error.


================
Comment at: llvm/test/TableGen/directive.td:11
+  let clausePrefix = "TDLC_";
+  let makeEnumAvailableInNamespace = 1;
+}
----------------
jdenny wrote:
> Does it make sense to go ahead and test the case where this is 0?  What about testing enableBitmaskEnumInNamespace?
I added a test for that. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81736