[PATCH] D82982: [openmp] Move isAllowedClauseForDirective to tablegen + add clause version to OMP.td

[Valentin Clement via Phabricator via llvm-commits]

clementval marked 2 inline comments as done.
clementval added inline comments.


================
Comment at: llvm/utils/TableGen/DirectiveEmitter.cpp:296
+
+  OS << "\n"; // Empty line at end of file
 }
----------------
jdenny wrote:
> clementval wrote:
> > jdenny wrote:
> > > clementval wrote:
> > > > jdenny wrote:
> > > > > Why is an empty line needed?
> > > > Just to be consistent with clang-format in the generated file.
> > > It's surprising that clang-format would require an empty line at the end of the file.  Any idea why?
> > Sorry, it is not clang-format but Phabricator which signal a missing empty line at end of file. This is not mandatory and can be removed. The file is generated. I just wanted to be consistent with the style. 
> Are you referring to, for example, the version of `directive2.td` on the left side of this diff?  Here, phabricator points out that the final line in the file is not newline-terminated.  I don't recall seeing phabricator suggest an additional empty line.
> 
> Likewise, in `EmitDirectivesImpl`, the final `GenerateIsAllowedClause` call already newline-terminates the final line of the generated file, so I don't see a need for an additional empty line.
Your are correct. I removed this since it was a confusion on my side. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82982