awarzynski added a comment.
Thanks for the updates! A few more pointers, but nothing major.
Btw, are there any tests that check for debug info in the compiled files? For
example:
! RUN: flang -g1 -S %s | FileCheck -check-prefixes=DEBUG-INFO-PRESENT
! RUN: flang -g0 -S %s | FileCheck -check-prefixes=DEBUG-INFO-MISSING
end program
================
Comment at: clang/lib/Driver/ToolChains/Flang.cpp:32
+static void
+addFortranDebugOptions(ArgStringList &CmdArgs,
+ llvm::codegenoptions::DebugInfoKind DebugInfoKind) {
----------------
There isn't anything Fortran specific here, is there? And this method basically
implements
https://github.com/llvm/llvm-project/blob/cf60d3f1a688671c8eb7859bf0572c403c3c0cca/clang/lib/Driver/ToolChains/Clang.cpp#L975-L1000,
right? Why not extract it into e.g. `renderDebugEnablingArgs` and move to
CommonArgs.cpp?
================
Comment at: flang/lib/Frontend/CompilerInvocation.cpp:868
parsePreprocessorArgs(res.getPreprocessorOpts(), args);
- parseCodeGenArgs(res.getCodeGenOpts(), args, diags);
+ success &= parseCodeGenArgs(res.getCodeGenOpts(), args, diags);
success &= parseSemaArgs(res, args, diags);
----------------
WDYT? I think that it makes sense to keep these separate.
================
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(
----------------
SBallantyne wrote:
> 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.
Thanks for the explanation! My main reservation is that it's not obvious how
`-g2` and `-g3` map onto `llvm::codegenoptions`. This warnings refers to `-g1`,
but there's no mention of `-g1` in this function.
TBH, I would replace "Debug options greater than -g1 not yet implemented" with
something a bit more generic and future-proof, e.g. `"Unsupported debug option:
%0" << arg.getValue()`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146814/new/
https://reviews.llvm.org/D146814
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
