[Github] Enable warnings as errors on flang sphinx build

[boomanaiden154]

Now that the number of warnings in the flang sphinx build has come down significantly, we can turn on warnings as errors in the sphinx build, which is the default configuration in CMake.

[llvmbot]

@llvm/pr-subscribers-github-workflow

Author: Aiden Grossman (boomanaiden154)

Changes

Now that the number of warnings in the flang sphinx build has come down significantly, we can turn on warnings as errors in the sphinx build, which is the default configuration in CMake.

---

Full diff: https://github.com/llvm/llvm-project/pull/72723.diff

1 Files Affected:

• (modified) .github/workflows/docs.yml (+1-3)

diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml
index 6329d777a182031..1eaa76cf838c049 100644
--- a/.github/workflows/docs.yml
+++ b/.github/workflows/docs.yml
@@ -150,9 +150,7 @@ jobs:
           TZ=UTC ninja -C polly-build docs-polly-html docs-polly-man
       - name: Build Flang docs
         if: steps.docs-changed-subprojects.outputs.flang_any_changed == 'true'
-        # TODO(boomanaiden154): Remove the SPHINX_WARNINGS_AS_ERRORS from the
-        # CMake invocation once the warnings in the flang docs build are fixed.
         run: |
-          cmake -B flang-build -GNinja -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_PROJECTS="clang;mlir;flang" -DLLVM_ENABLE_SPHINX=ON -DSPHINX_WARNINGS_AS_ERRORS=OFF ./llvm
+          cmake -B flang-build -GNinja -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_PROJECTS="clang;mlir;flang" -DLLVM_ENABLE_SPHINX=ON ./llvm
           TZ=UTC ninja -C flang-build docs-flang-html docs-flang-man
 

[boomanaiden154]

We'll probably need to wait a little bit before merging this, but there's only one final warning that I can see before this is ready to merge:

/tmp/llvm-project/build/tools/flang/docs/Source/FlangCommandLineReference.rst:176: WARNING: unknown document: 'DiagnosticsReference'

Once that gets fixed, we should be able to get this in.

[kiranchandramohan]

We'll probably need to wait a little bit before merging this, but there's only one final warning that I can see before this is ready to merge:

/tmp/llvm-project/build/tools/flang/docs/Source/FlangCommandLineReference.rst:176: WARNING: unknown document: 'DiagnosticsReference'

Once that gets fixed, we should be able to get this in.

This warning is coming from the fact there is a reference to this document in Options.td (

llvm-project/clang/include/clang/Driver/Options.td

Line 133 in 37fa327

See the :doc:`full list of warning and remark flags <DiagnosticsReference>`.}]>;

). Options.td is shared between flang and clang. While generating documentation for Flang this reference is present and that causes the warning. We do not generate DiagnosticsReference at the moment in Flang and do not have a plan for this at the moment. The code for Diagnostics sits inside Clang Basic directories, while in Flang it is in the Semantics code.

I will need help to fix this issue.

[tru]

Can this be merged or dropped if no longer needed?

[boomanaiden154]

The error mentioned above is still present. I haven't looked too much into fixing it. It looks like there have been a couple other regression in regards to warnings in the mean time as well looking at some recent runs.

[tru]

Would be good if we could fix so that errors are caught in the PR process if at all possible.

[kiranchandramohan]

We'll probably need to wait a little bit before merging this, but there's only one final warning that I can see before this is ready to merge:

/tmp/llvm-project/build/tools/flang/docs/Source/FlangCommandLineReference.rst:176: WARNING: unknown document: 'DiagnosticsReference'

Once that gets fixed, we should be able to get this in.

This warning (and all others) are now fixed. But I see the following mlir warning that is probably caused because flang needs mlir tools.

/home/buildbot/as-worker-4/publish-sphinx-docs/llvm-project/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp:2956:38: warning: unused variable ‘operand’ [-Wunused-variable]

[boomanaiden154]

This warning (and all others) are now fixed.

There were still two errors popping up with the CI configuration, but it looks like those are due to building the docs-flang-man target, which doesn't actually produce anything. Given that's not configured properly, it should probably be removed?

But I see the following mlir warning that is probably caused because flang needs mlir tools.

/home/buildbot/as-worker-4/publish-sphinx-docs/llvm-project/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp:2956:38: warning: unused variable ‘operand’ [-Wunused-variable]

I can't reproduce that warning at all with a recent clang version. The CI uses gcc (I believe), so it might show up there. It's a compiler warning though, so doesn't impact the viability of this patch.

I'm going to land this soonish as everything should be good to go now. Thanks for your work on fixing this up!

[kiranchandramohan]

I can't reproduce that warning at all with a recent clang version. The CI uses gcc (I believe), so it might show up there. It's a compiler warning though, so doesn't impact the viability of this patch.

This warning is also fixed by the following.
#81899

Thanks for your patience.