r195495 - Debug Info: add a "Debug Info Version" module flag to output the current debug

[Eric Christopher]

On Fri, Nov 22, 2013 at 12:07 PM, Manman Ren <manman.ren at gmail.com> wrote:
>
>
>
> On Fri, Nov 22, 2013 at 11:59 AM, Eric Christopher <echristo at gmail.com>
> wrote:
>>
>> On Fri, Nov 22, 2013 at 11:42 AM, Manman Ren <manman.ren at gmail.com> wrote:
>> > Author: mren
>> > Date: Fri Nov 22 13:42:45 2013
>> > New Revision: 195495
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=195495&view=rev
>> > Log:
>> > Debug Info: add a "Debug Info Version" module flag to output the current
>> > debug
>> > info version number.
>> >
>> > Will error out when modules have different version numbers.
>> >
>>
>> I don't think the commit message or comment are correct. We're going
>> to strip debug info at module merge time if they have different
>> versions, not error out at merge time.
>
>
> The commit message and the comment is for what is implemented in the patch.
> The module flag will be used to strip debug info later on.
> And I plan to talk about stripping debug info when the patch that actually
> strips debug info is committed.
> The comment tries to explain why we choose the error mode and what will
> happen if the linker sees different versions.
>
> Should we talk about how the module flag will be used when adding the module
> flag in the source code?
> Maybe if we want to be verbose.
>

Basically it means that your comment and commit message are contrary
to the actual use that we have planned. If someone were to look at
clang and your comment without knowing what's going on then they'll be
confused when they see the actual implementation.

-eric

> Manman
>
>>
>>
>> -eric
>>
>> > Added:
>> >     cfe/trunk/test/CodeGen/debug-info-version.c
>> > Modified:
>> >     cfe/trunk/lib/CodeGen/CodeGenModule.cpp
>> >
>> > Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=195495&r1=195494&r2=195495&view=diff
>> >
>> > ==============================================================================
>> > --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
>> > +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Fri Nov 22 13:42:45 2013
>> > @@ -258,6 +258,11 @@ void CodeGenModule::Release() {
>> >      // We can change from Warning to Latest if such mode is supported.
>> >      getModule().addModuleFlag(llvm::Module::Warning, "Dwarf Version",
>> >                                CodeGenOpts.DwarfVersion);
>> > +  if (DebugInfo)
>> > +    // We support a single version in the linked module: error out when
>> > +    // modules do not have the same version.
>> > +    getModule().addModuleFlag(llvm::Module::Error, "Debug Info
>> > Version",
>> > +                              llvm::dwarf::DEBUG_INFO_VERSION);
>> >
>> >    SimplifyPersonality();
>> >
>> >
>> > Added: cfe/trunk/test/CodeGen/debug-info-version.c
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/debug-info-version.c?rev=195495&view=auto
>> >
>> > ==============================================================================
>> > --- cfe/trunk/test/CodeGen/debug-info-version.c (added)
>> > +++ cfe/trunk/test/CodeGen/debug-info-version.c Fri Nov 22 13:42:45 2013
>> > @@ -0,0 +1,8 @@
>> > +// RUN: %clang -g -S -emit-llvm -o - %s | FileCheck %s
>> > +// RUN: %clang -S -emit-llvm -o - %s | FileCheck %s
>> > --check-prefix=NO_DEBUG
>> > +int main (void) {
>> > +  return 0;
>> > +}
>> > +
>> > +// CHECK: metadata !{i32 1, metadata !"Debug Info Version", i32 1}
>> > +// NO_DEBUG-NOT: metadata !"Debug Info Version"
>> >
>> >
>> > _______________________________________________
>> > cfe-commits mailing list
>> > cfe-commits at cs.uiuc.edu
>> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>