[PATCH] D146814: [Flang] Add debug flag to enable current debug information pass

[Sacha Ballantyne via Phabricator via cfe-commits]

SBallantyne added a comment.

In D146814#4223967 <https://reviews.llvm.org/D146814#4223967>, @awarzynski wrote:

> What's the overall design goal here? 100% consistency with Clang? Could this be documented?

The goal of this patch is just to enable the current debug pass with an appropriate flag, and ensure that its fairly easy to expand this should more passes/work be done for flang. I've chosen to just copy the majority of the clang debug system on the assumption flang will follow the same path, but i don't think that is explicitly planned or the only way forwards.



================
Comment at: clang/include/clang/Driver/ToolChain.h:23
 #include "llvm/ADT/StringRef.h"
-#include "llvm/ADT/Triple.h"
 #include "llvm/Frontend/Debug/Options.h"
----------------
awarzynski wrote:
> Unrelated change?
Accidental include from D142347, I've removed it there so it shouldn't have to be removed here.


================
Comment at: flang/lib/Frontend/CompilerInvocation.cpp:127
+    unsigned val =
+        llvm::StringSwitch<unsigned>(arg->getValue())
+            .Case("line-tables-only", llvm::codegenoptions::DebugLineTablesOnly)
----------------
awarzynski wrote:
> 1. Why `unsigned` instead of [[ https://github.com/llvm/llvm-project/blob/cf60d3f1a688671c8eb7859bf0572c403c3c0cca/clang/include/clang/Basic/DebugInfoOptions.h#L20-L55 | DebugInfoKind ]]
> 2. Why not use `std::optional`, e.g. `llvm::StringSwitch<std::optional<DebugInfoKind>>arg->getValue())`? This way you could avoid magic numbers like `~0U`.
Sure i'm happy to implement that here. This code originally comes from [[ https://github.com/llvm/llvm-project/blob/main/clang/lib/Frontend/CompilerInvocation.cpp#L1667 | the clang frontend ]]


================
Comment at: flang/lib/Frontend/CompilerInvocation.cpp:144
+        val != llvm::codegenoptions::NoDebugInfo) {
+      // TODO: This is not a great warning message, could be improved
+      const auto debugErr = diags.getCustomDiagID(
----------------
awarzynski wrote:
> Please improve it :)
> 
> In particular, you are testing for `DebugLineTablesOnly` and `NoDebugInfo`, yet the diagnostic refers to `-g1`.
I previously had it emit these debug names, but i think its more confusing for the user as they will be passing `-g2 / -g3` in order to get this error, and the internal name is not as helpful. The TODO was from a previous patch and i just forgot to remove it, i am happy with the current state of warning. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146814