[cfe-dev] Removing '-g' as an option to cc1 and cc1as

Fri Oct 2 16:13:54 PDT 2015

David, thanks for the feedback.

The bug fix is integrally tied to the new code, though it was really only
by accident - The entire goal was to avoid adding another special-case of
"if platform X then assume darwin2" which as can be seen from the deleted
code, required twice mentioning every special case.

One of my comments in the patch lists the test files whose changes are
actually meaningful, as opposed to test that were forced against their will
to go along for the ride. The test with explicitly changed expectation is
mentioned - it's "cl-options.c" but I'll explain here too:

A test specified "/z7" and "-gdwarf" in that order.  "/z7" means
line-tables-only.  But "-gdwarf" implies normal ("limited") debug info.
The "/z7" option was properly translated into "-gline-tables-only" but did
not check whether it occurred earlier or later than any other 'g' option.
(It is not a g-group flag).
So in this one case, and only this case, you have both a line-tables-only
flag and a more-than-lines-tables-only flag getting through to
CompilerInvocation. Then when you look at what CompilerInvocation did, it
asked "Args.hasArg(OPT_gline_tables_only)" as the first check, and if so,
did not care about presence of other 'g' flags. So the leftmost arg "won".
It clearly assumed that at most one of any of the conflicting g flags would
have been picked by the Driver.

The new code's 3-valued argument can't really get this wrong in the same
way. Usually I don't like to change the expectation of one test in a big
suite of tests like this, but to avoid changing the test's expectation
would require temporarily injecting a bug-for-bug compatible behavior
saying that "/z7" is always stronger than '-g'.  But really that's a bit
silly, don't you think?

On Fri, Oct 2, 2015 at 6:17 PM, David Blaikie <dblaikie at gmail.com> wrote:

>
>
> On Thu, Oct 1, 2015 at 8:18 AM, Douglas Katzman via cfe-dev <
> cfe-dev at lists.llvm.org> wrote:
>
>> Hi,
>>
>> I have a patch up on Phabricator which regularizes some 'g' arguments in
>> the tool frontends by trusting the clang Driver to handle
>> toolchain-specific choices about the option family, such as when the
>> toolchain defaults to Dwarf 2 vs Dwarf 4 and/or when -fstandalone-debug
>> should be assumed.
>>
>> The internal option name for setting DebugInfoKind is
>> "-di-kind={full|limited|line-tables}" which directly sets the codegen opts
>> to the enum value whose name is as given.
>> Similarly there's a dwarf-version option.  So the '-g' group options will
>> be stripped out by the driver and not passed along to cc1 or cc1as.
>> Specifically, the '-gdwarf', '-gline-tables-only', and '-g' options will
>> be rejected if seen in {cc1,cc1as} invocations that were hand-crafted in a
>> lot of regression tests.
>>
>
> Yes, cc1/cc1as commmand lines are intentionally hand crafted in tests to
> limit the scope of a test. Driver tests should test only the driver (just
> check how the normal command line args are mapped to cc1/cc1as) and other
> tests should not test the driver.
>
> But the cc1/cc1as interface is not stable, and up to us to change at will.
> So it's intentional that these tests are like this, but not an indication
> that they need to stay that way - any changes there should be totally fine,
> so long as they produce the same effect overall.
>
> (this is similar to the boundary between Clang and LLVM being LLVM IR - we
> can and do change the textual IR and just update tests on both sides of
> that boundary whenever we do so)
>
>
>> I don't think this should be controversial, as the cc1 invocation is not
>> considered a public API, but maybe some folks would prefer that '-g' still
>> have some meaning.
>>
>
> I don't think it's important for cc1 to have a -g option.
>
>
>> So I'd like to ask whether that's the case, as well as invite a little
>> bit of "bikeshed" discussion as to whether the option name I've introduced
>> makes sense.
>>
>> The names "limited" and "full" have slightly strange connotations even
>> though they have the right semantics.
>> "limited" is the normal level of debugging, whereas "full" means
>> "-fstandalone" because the debugger is unable to cope with "limited".
>> So it would also be totally reasonable to name these choices
>> {standalone,normal,line-tables} which unfortunately don't coincide with
>> names of DebugInfoKind.
>>
>
> I'm happy with a smaller taxonomy and for cc1 to match the source code
> (rather than the command line) more closely... probably. (arguably we could
> change the implementation to more closely match the command line, if it's a
> good taxonomy - I dunno)
>
> The "limited" terminology came from a narrower version of -fstandalone
> from a long time ago (-flimit-debug-info was the default and you could
> opt-out with -fno-limit-debug-info). I improved and then expanded on that
> until it broke a bunch of stuff for some debuggers. So we wanted to
> describe better the principles by which users could understand when they
> might need to use this feature. ("standalone" being "if I'm building this
> object file with debug info and I don't want the compiler to assume other
> parts of the program will be built with debug info": ie: let the debug info
> for this object stand on its own/standalone)
>
> (in case any of that history is helpful to you)
>
>
>> I chose the names that matched, but I find them unappealing.
>>
>> So for what it's worth, could some folks maybe express an opinion, and/or
>> take a crack at reviewing this?
>>  http://reviews.llvm.org/D13221
>>
>
> Taking a quick glance - is it possible to separate moving the info from
> frontend to driver from the bug fixes that enables? Or are they
> intrinsically tied?
>
> It'd just be a bit easier to review if they were separate - otherwise you
> might need to point us to where the bug fix test improvements/changes have
> been made, amongst all the other changes.
>
>
>> Also I'll note one pro and one con for keeping '-g' as an option to {cc1,
>> cc1as}.
>> Pro: some of the tests that were affected in the above change could
>> remain untouched.
>> Con: it's better to visibly fail than to silently accept an option whose
>> meaning differs, which is to say, if people expected that the
>> CompilerInvocation would automatically use Dwarf 2 on Darwin when you give
>> '-g' in the cc1 invocation, that is most definitely no longer true.
>>
>> Thanks and regards,
>> Doug
>>
>> _______________________________________________
>> cfe-dev mailing list
>> cfe-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20151002/561ec090/attachment.html>