AaronBallman wrote:

> > It's not quite clear to me what the long-term direction is. e.g., in Clang, 
> > we tend to use tablegen to generate documentation (attributes, command line 
> > options, etc) so that we can generate C++ code for the implementation which 
> > is tied to the documentation we generate. Are you expecting to follow a 
> > similar direction for CIR? I ask because I don't think we want code 
> > generators in tablegen and python if we can avoid it.
> 
> In CIR, our documentation and code is generated from a tool named 
> `mlir-tblgen`, provided by the MLIR framework (so it is not under our 
> control). This tool reads our TableGen definitions and emits C++ code and 
> documentation, similar to what `clang-tblgen` is doing.
> 
> However, `mlir-tblgen` does not generate immediately usable output. We need 
> some post-processing on its output content to incorporate it into our 
> documentation page. That's what the tiny Python script included in this patch 
> does.

Thank you for the background information!

> > Clang's documentation should consistently be in .rst files; there have been 
> > a few things moved to `.md` but those changes have been questioned enough 
> > that we should not use it as precedent for adding more .md files.
> 
> > I don't think we should disable this diagnostic; MLIR should not generate 
> > invalid markdown code blocks in the first place IMO.
> 
> The main problem here is that we have a conflict with what `mlir-tblgen` 
> follows:
> 
>     1. `mlir-tblgen` does not generate `.rst` files. It emits `.md` files 
> only.
>
>     2. `mlir-tblgen` generates Markdown code blocks of language `mlir`, which 
> is not recognized by Sphinx.
> 
> 
> With some more post-processing on the output material of `mlir-tblgen`, we 
> could replace all `mlir` code blocks with a plaintext code block, thus 
> resolving problem 2.
> 
> I don't see an easy way to bypass problem 1 if we intend to follow a strict 
> `.rst` policy now. Teaching `mlir-tblgen` to generate `.rst` files could 
> work, but it requires non-trivial amount of additional work on implementation 
> and maintenance.

Thank you for the background information! This is another case where it's a bit 
awkward that MLIR picked a different approach from the rest of the project 
(we've run into this previously with CIR around naming convention choices too). 
I'd prefer mlir-tblgen didn't have to know how to spit out .rst and .md files 
because that seems rather silly, but it's also not reasonable to expect the 
Clang community worsen our tech debt in this area either. On balance, I think 
I'd rather the tool be updated to handle either format on the assumption that 
1) MLIR doesn't want to change away from .md and Clang should not force them 
to, 2) Clang doesn't want more inconsistency with .md and .rst and MLIR should 
not force us to, 3) .rst and .md markdown formats are sufficiently similar this 
isn't a huge implementation or maintenance burden to support both. #3 is pure 
speculation on my part though, so maybe I'm off-base?

> Since CIR is opt-in behind a CMake flag CLANG_ENABLE_CIR, the CIRLangRef.rst 
> page is generated only when CIR is included in the build.

Hmmm, I can see why but this still caught be my surprise. To me, documentation 
is a separate thing from enabling support for CIR at all so I was not expecting 
`CLANG_ENABLE_CIR` to mean no CIR docs get built. So I think I'd be fine if the 
page was generated always regardless, if that happened to be easier to support.

> But I don't think this is a problem in the long-term since eventually we will 
> make CIR enabled by default.

+1

https://github.com/llvm/llvm-project/pull/190354
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to