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

Joel E. Denny via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 6 15:15:50 PDT 2020


jdenny added inline comments.


================
Comment at: llvm/utils/TableGen/DirectiveEmitter.cpp:296
+
+  OS << "\n"; // Empty line at end of file
 }
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82982





More information about the llvm-commits mailing list