[PATCH] D129864: [Flang] Generate documentation for compiler flags

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

Makes sense, thanks for working on this! Some minor comments below and inline.

>From your summary:

> This is done by using clang tablegen

What do you mean by "clang tablegen"? Is it Clang's clang_tablegen <https://github.com/llvm/llvm-project/blob/f80a4321ef1bafcd8041884bcb85d9ba24335adb/clang/cmake/modules/AddClang.cmake#L4-L32> CMake function?

> from options.td

Options.td ;-)

> shared with clang

Most people associate "clang" with the executable. You may want to use "Clang" to avoid confusion (IIUC, you are referring to the sub-project rather than the executable here).

> specific flang flags

Flang :) (same rationale as above)



================
Comment at: clang/utils/TableGen/ClangOptionDocEmitter.cpp:329
                 raw_ostream &OS) {
-  if (isExcluded(Option.Option, DocInfo))
+  if (isExcluded(Option.Option, DocInfo) || !isIncluded(Option.Option, DocInfo))
     return;
----------------
I think that the way this is written at the moment is a bit counterintuitive (i.e. "if excluded or not included"). This is really down to how these include/exclude flags are used in various other places :( (i.e. there's little that we can do about it).

I think that it would make more sense to follow the style from [[ https://github.com/llvm/llvm-project/blob/f80a4321ef1bafcd8041884bcb85d9ba24335adb/llvm/lib/Option/OptTable.cpp#L656-L659 | OptTable.cpp ]]. In particular (pseudo code):
```
if (excluded)
  return;
if (included_not_empty && !included)
  return;
```

This way it becomes clear that `IncludedFlags` (from "FlangOptionsDocs.td") is only taken into account when not empty. I would also add an assert in `isIncluded`:
```
assert(DocInfo->getValue("IncludedFlags") && Missing includeFlags);
```

WDYT? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129864