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
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
