r186262 - PR16214, PR14467: DebugInfo: use "RequireCompleteType" to decide when to emit the full definition of a type in -flimit-debug-info

[David Blaikie]

On Sat, Jul 13, 2013 at 6:32 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> On Sat, Jul 13, 2013 at 6:16 PM, David Blaikie <dblaikie at gmail.com> wrote:
>> On Sat, Jul 13, 2013 at 3:21 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>>>
>>> On Sat, Jul 13, 2013 at 2:08 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>> > Author: dblaikie
>>> > Date: Sat Jul 13 16:08:14 2013
>>> > New Revision: 186262
>>> >
>>> > URL: http://llvm.org/viewvc/llvm-project?rev=186262&view=rev
>>> > Log:
>>> > PR16214, PR14467: DebugInfo: use "RequireCompleteType" to decide when to emit the full definition of a type in -flimit-debug-info
>>> >
>>> > This simplifies the core benefit of -flimit-debug-info by taking a more
>>> > systematic approach to avoid emitting debug info definitions for types
>>> > that only require declarations. The previous ad-hoc approach (3 cases
>>> > removed in this patch) had many holes.
>>> >
>>> > The general approach (adding a bit to TagDecl and callback through
>>> > ASTConsumer) has been discussed with Richard Smith - though always open
>>> > to revision.
>>> >
>>> > Modified:
>>> >     cfe/trunk/include/clang/AST/ASTConsumer.h
>>> >     cfe/trunk/include/clang/AST/Decl.h
>>> >     cfe/trunk/include/clang/Sema/Sema.h
>>> >     cfe/trunk/lib/CodeGen/CGClass.cpp
>>> >     cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
>>> >     cfe/trunk/lib/CodeGen/CGExprCXX.cpp
>>> >     cfe/trunk/lib/CodeGen/CGExprScalar.cpp
>>> >     cfe/trunk/lib/CodeGen/CodeGenAction.cpp
>>> >     cfe/trunk/lib/CodeGen/ModuleBuilder.cpp
>>> >     cfe/trunk/lib/Sema/SemaType.cpp
>>> >     cfe/trunk/test/CodeGenCXX/debug-info-class-limited.cpp
>>> >
>>> > Modified: cfe/trunk/include/clang/AST/ASTConsumer.h
>>> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTConsumer.h?rev=186262&r1=186261&r2=186262&view=diff
>>> > ==============================================================================
>>> > --- cfe/trunk/include/clang/AST/ASTConsumer.h (original)
>>> > +++ cfe/trunk/include/clang/AST/ASTConsumer.h Sat Jul 13 16:08:14 2013
>>> > @@ -72,6 +72,10 @@ public:
>>> >    /// can be defined in declspecs).
>>> >    virtual void HandleTagDeclDefinition(TagDecl *D) {}
>>> >
>>> > +  /// \brief This callback is invoked the first time each TagDecl is required to
>>> > +  /// be complete.
>>> > +  virtual void HandleTagDeclRequiredDefinition(const TagDecl *D) {}
>>> > +
>>> >    /// \brief Invoked when a function is implicitly instantiated.
>>> >    /// Note that at this point point it does not have a body, its body is
>>> >    /// instantiated at the end of the translation unit and passed to
>>> >
>>> > Modified: cfe/trunk/include/clang/AST/Decl.h
>>> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=186262&r1=186261&r2=186262&view=diff
>>> > ==============================================================================
>>> > --- cfe/trunk/include/clang/AST/Decl.h (original)
>>> > +++ cfe/trunk/include/clang/AST/Decl.h Sat Jul 13 16:08:14 2013
>>> > @@ -2445,6 +2445,9 @@ protected:
>>> >    /// This option is only enabled when modules are enabled.
>>> >    bool MayHaveOutOfDateDef : 1;
>>> >
>>> > +  /// Has the full definition of this type been required by a use somewhere in
>>> > +  /// the TU.
>>> > +  bool IsCompleteDefinitionRequired : 1;
>>>
>>> It looks like you're missing serialization/deserialization support for
>>> this flag.
>>
>> Added in r186266 - not sure if it needs separate tests, but when I got
>> it wrong in a few ways the tests certainly failed, so I guess it's
>> somewhat covered.
>
> You could test this with a PCH test that emits and checks debug info.

Ah, thanks! - test case added in r186351.