[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

This revision was automatically updated to reflect the committed changes. Closed by commit rG8d8f9c244074: [clang] Add -fdebug-default-version for specifying the default DWARF version (authored by dblaikie). Changed prior to commit: https://reviews.llvm.org/D69822?vs=228250&id=228287#toc Repos

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

cmtice updated this revision to Diff 228250. cmtice added a comment. Fix small typo in comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69822/new/ https://reviews.llvm.org/D69822 Files: clang/include/clang/Driver/Options.td clang/lib/Driver/ToolChains/Clang.cpp clang/lib/D

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

probinson added a comment. Sigh, one last typo. I'm happy otherwise. Comment at: clang/test/Driver/debug-default-version.c:11 +// environment, we should use codeview. You can enable dwarf, which implicitly +// disables codeview, of you can explicitly ask for both if you don't

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

cmtice updated this revision to Diff 228144. cmtice added a comment. Remove quotes from -NOT tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69822/new/ https://reviews.llvm.org/D69822 Files: clang/include/clang/Driver/Options.td clang/lib/Driver/ToolChains/Clang.cpp clang/li

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

MaskRay accepted this revision. MaskRay added a comment. LG after `// NOCODEVIEW-NOT: "-gcodeview"` and `// NODWARF4-NOT: "-dwarf-version=4"` are fixed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69822/new/ https://reviews.llvm.org/D69822 _

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

cmtice updated this revision to Diff 228130. cmtice added a comment. Fix NODEBUGINFO-NOT test. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69822/new/ https://reviews.llvm.org/D69822 Files: clang/include/clang/Driver/Options.td clang/lib/Driver/ToolChains/Clang.cpp clang/lib/Dri

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

dblaikie added inline comments. Comment at: clang/test/Driver/debug-default-version.c:37 + +// NODEBUGINFO-NOT: "-debug-info-kind=" + Same issue as with the dwarf-version below. It's probably easier to remove the "" (FileCheck arguments aren't quoted - these qu

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

cmtice updated this revision to Diff 228115. cmtice added a comment. Remove period from help text. Fix tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69822/new/ https://reviews.llvm.org/D69822 Files: clang/include/clang/Driver/Options.td clang/lib/Driver/ToolChains/Clang.cpp

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

MaskRay added inline comments. Comment at: clang/test/Driver/debug-default-version.c:66 +// NOCODEVIEW-NOT: "-gcodeview" +// NODWARF-NOT: "-dwarf-version=" +// NODWARF-NOT: "-dwarf-version=" This is not correct. I suppose this to be `"-dwarf-version=`. The patte

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

probinson added a comment. Just a couple of typos, and I'm happy. I agree with the other reviewers on the last needed test tweaks. Comment at: clang/include/clang/Driver/Options.td:1955 Flags<[CC1Option]>; +def fdebug_default_version: Joined<["-"], "fdebug-default-version=

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

MaskRay added inline comments. Comment at: clang/test/Driver/debug-default-version.c:9-17 +// The -isysroot is used as a hack to avoid LIT messing with the SDKROOT +// environment variable which indirecty overrides the version in the target +// triple used here. +// RUN: %clang -

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Looks good to me - but maybe give it a day to see if anyone else has further thoughts/that you've addressed their feedback too. Comment at: clang/test/Driver/debug-defau

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

cmtice updated this revision to Diff 227977. cmtice added a comment. Rename flag to -fdebug-default-version; add help text, mentioning DWARF. Update tests to use -### and add assembler tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69822/new/ https://reviews.llvm.org/D69822 File

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

cmtice marked an inline comment as done. cmtice added a comment. Yes, I will change the flag name back to debug-default-version, and add help text mentioning dwarf. I'm in the process of trying to re-do the test cases as required (I'm a bit new to this so it's taking me a bit to figure this out

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

dblaikie added inline comments. Comment at: clang/include/clang/Driver/Options.td:1955 Flags<[CC1Option]>; +def fdwarf_default_version: Joined<["-"], "fdwarf-default-version=">, Group; def fdebug_prefix_map_EQ probinson wrote: > Probably should have HelpText

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

probinson added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6171 + + if (DwarfVersion == 0) +DwarfVersion = ParseDwarfDefaultVersion(getToolChain(), Args); MaskRay wrote: > lang=cpp > if (DwarfVersion == 0) { > DwarfVersion = P

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

probinson added inline comments. Comment at: clang/include/clang/Driver/Options.td:1955 Flags<[CC1Option]>; +def fdebug_default_version: Joined<["-"], "fdebug-default-version=">, Group; def fdebug_prefix_map_EQ dblaikie wrote: > probinson wrote: > > If this

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

cmtice updated this revision to Diff 227954. cmtice added a comment. Make second call to ParseDwarfDefaultVersion unconditional (to match the first and avoid errors with -Werror). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69822/new/ https://reviews.llvm.org/D69822 Files: clang/i

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

probinson added a comment. The test is in the right place, now it needs to behave more like other driver tests. Sorry if it feels like I'm whaling on you, but the driver is a bit of a peculiar beast with an atypical testing mode. Taming it is harder than it looks. Comment

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

cmtice added a comment. For the record, I double-checked and if we use the flag and don't check for it (i.e. if we move the parsing inside the EmitDwarf is True block) then -Werror does indeed complain about an unused command line argument. CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

MaskRay added inline comments. Comment at: clang/test/Driver/dwarf-default-version.c:1 +// RUN: %clang -target x86_64-linux-gnu -fdwarf-default-version=4 -gdwarf-2 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=DWARF2 +// RUN: %clang -target x86_64-linux-gnu -gdwarf-3 -fdwa

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3237 +// If the user specified a default DWARF version, that takes precedence +// over the platform default. lang=cpp if (DefaultDWARFVersion) { // If the use

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

cmtice added a comment. dblaikie: The code, as written, parses the dwarf default version flag, whether or not DWARFVersion is 0, i.e. whether or not a -gdwarf-N flag was passed. It only USES the value if there's no overriding value. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69822

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

dblaikie added inline comments. Comment at: clang/include/clang/Driver/Options.td:1955 Flags<[CC1Option]>; +def fdebug_default_version: Joined<["-"], "fdebug-default-version=">, Group; def fdebug_prefix_map_EQ probinson wrote: > If this is specifically the d

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

cmtice added a comment. Ok, I think the upload was correct this time. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69822/new/ https://reviews.llvm.org/D69822 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

cmtice updated this revision to Diff 227942. cmtice added a comment. re-try upload again. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69822/new/ https://reviews.llvm.org/D69822 Files: clang/include/clang/Driver/Options.td clang/lib/Driver/ToolChains/Clang.cpp clang/lib/Driver/T

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

cmtice added a comment. Hmmm...I'm having upload issues. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3250 // -gline-directives-only supported only for the DWARF debug info. if (DWARFVersion == 0 && DebugInfoKind == codegenoptions::DebugDirectivesOnly) DebugInf

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

cmtice updated this revision to Diff 227940. cmtice added a comment. Try uploading correct diff file? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69822/new/ https://reviews.llvm.org/D69822 Files: clang/include/clang/Driver/Options.td clang/lib/Driver/ToolChains/Clang.cpp clang/

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

MaskRay added a comment. In D69822#1734255 , @cmtice wrote: > Made requested changes: > > - renamed option to be dwarf-specific > - fixed spelling & blank line issues > - only set version if emit-dwarf is true > - move test to Driver directory > > I *thi

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

cmtice updated this revision to Diff 227910. cmtice marked 9 inline comments as done. cmtice added a comment. Made requested changes: - renamed option to be dwarf-specific - fixed spelling & blank line issues - only set version if emit-dwarf is true - move test to Driver directory I *think* I go

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

probinson added inline comments. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:1151 + || Value > 5 + || Value < 1) +TC.getDriver().Diag(diag::err_drv_invalid_int_value) dblaikie wrote: > probinson wrote: > > Clang doesn't support DWARF v1.

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

dblaikie added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3250 // -gline-directives-only supported only for the DWARF debug info. if (DWARFVersion == 0 && DebugInfoKind == codegenoptions::DebugDirectivesOnly) DebugInfoKind = codegenoptions::NoD

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

probinson added a comment. In D69822#1733269 , @dblaikie wrote: > In D69822#1733262 , @probinson wrote: > > > The maze of twisty -g passages gets a new secret door. Oh well. > > > If you find this to be a significa

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

probinson added inline comments. Comment at: clang/include/clang/Driver/Options.td:1955 Flags<[CC1Option]>; +def fdebug_default_version: Joined<["-"], "fdebug-default-version=">, Group; def fdebug_prefix_map_EQ If this is specifically the default DWARF versi

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

dblaikie added a comment. In D69822#1733262 , @probinson wrote: > In D69822#1733243 , @dblaikie wrote: > > > In D69822#1733226 , @probinson > > wrote: > > > > > > Want to de

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

probinson added a comment. In D69822#1733243 , @dblaikie wrote: > In D69822#1733226 , @probinson wrote: > > > > Want to decouple setting the DWARF version from enabling/disabling > > > generation of debug info. > >

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

dblaikie added a comment. In D69822#1733258 , @aprantl wrote: > Since this sounds like it is hidden inside of some other tooling — is passing > the frontend option `-dwarf-version=5` not an option? "hidden inside of some other tooling" - in the same sen

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

aprantl added a comment. Since this sounds like it is hidden inside of some other tooling — is passing the frontend option `-dwarf-version=5` not an option? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69822/new/ https://reviews.llvm.org/D69822 _

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

probinson added inline comments. Comment at: clang/include/clang/Driver/Options.td:2002 def gno_codeview_ghash : Flag<["-"], "gno-codeview-ghash">, Flags<[CoreOption]>; + def ginline_line_tables : Flag<["-"], "ginline-line-tables">, Flags<[CoreOption]>; Unexp

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

dblaikie added a comment. In D69822#1733226 , @probinson wrote: > > Want to decouple setting the DWARF version from enabling/disabling > > generation of debug info. > > Um, why? If you've got a big build system with various users opting in/out of DWARF

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

probinson added a comment. > Want to decouple setting the DWARF version from enabling/disabling generation > of debug info. Um, why? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69822/new/ https://reviews.llvm.org/D69822 ___ cfe-commits

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

dblaikie added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3247-3248 } + else if (DefaultDWARFVersion != 0) +DWARFVersion = DefaultDWARFVersion; cmtice wrote: > dblaikie wrote: > > Hmm, actually - why is this case necessary/what d

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

cmtice updated this revision to Diff 22. cmtice added a comment. Made requested formatting changes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69822/new/ https://reviews.llvm.org/D69822 Files: clang/include/clang/Driver/Options.td clang/lib/Driver/ToolChains/Clang.cpp clan

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

MaskRay added inline comments. Comment at: clang/test/CodeGen/debug-default-version.c:2 +// RUN: %clang -target x86_64-linux-gnu -fdebug-default-version=4 -gdwarf-2 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER2 +// RUN: %clang -target x86_64-linux-gnu -gdwarf-3 -fdebu

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

MaskRay added inline comments. Comment at: clang/test/CodeGen/debug-default-version.c:1 +// RUN: %clang -target x86_64-linux-gnu -fdebug-default-version=4 -gdwarf-2 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER2 +// RUN: %clang -target x86_64-linux-gnu -gdwarf-3 -fdebu

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

dblaikie added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3246-3247 DWARFVersion = ExplicitVersion; } + else if (DefaultDWARFVersion != 0) +DWARFVersion = DefaultDWARFVersion; dblaikie wrote: > Looks like this should be on

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

cmtice marked 3 inline comments as done. cmtice added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3246-3247 DWARFVersion = ExplicitVersion; } + else if (DefaultDWARFVersion != 0) +DWARFVersion = DefaultDWARFVersion; dblaiki

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

dblaikie added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3246-3247 DWARFVersion = ExplicitVersion; } + else if (DefaultDWARFVersion != 0) +DWARFVersion = DefaultDWARFVersion; Looks like this should be on a single line to

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

cmtice created this revision. cmtice added a reviewer: dblaikie. Herald added subscribers: cfe-commits, fedor.sergeev, aprantl. Herald added a project: clang. Want to decouple setting the DWARF version from enabling/disabling generation of debug info. This adds a new flag '-fdebug-default-versi