[PATCH] D14358: DWARF's forward decl of a template should have template parameters.

[Paul Robinson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL31: [DWARF] Allow forward declarations of a class 
template instantiation (authored by probinson).

Changed prior to commit:
  https://reviews.llvm.org/D14358?vs=116862=117030#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D14358

Files:
  cfe/trunk/include/clang/Driver/CC1Options.td
  cfe/trunk/include/clang/Frontend/CodeGenOptions.def
  cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
  cfe/trunk/lib/Driver/ToolChains/Clang.cpp
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/test/CodeGenCXX/debug-info-fwd-template-param.cpp


Index: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
===
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
@@ -833,6 +833,10 @@
   llvm::DICompositeType *RetTy = DBuilder.createReplaceableCompositeType(
   getTagForRecord(RD), RDName, Ctx, DefUnit, Line, 0, Size, Align,
   llvm::DINode::FlagFwdDecl, FullName);
+  if (CGM.getCodeGenOpts().DebugFwdTemplateParams)
+if (auto *TSpecial = dyn_cast(RD))
+  DBuilder.replaceArrays(RetTy, llvm::DINodeArray(),
+ CollectCXXTemplateParams(TSpecial, DefUnit));
   ReplaceMap.emplace_back(
   std::piecewise_construct, std::make_tuple(Ty),
   std::make_tuple(static_cast(RetTy)));
Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -2969,6 +2969,11 @@
 CmdArgs.push_back("-generate-type-units");
   }
 
+  // Decide how to render forward declarations of template instantiations.
+  // SCE wants full descriptions, others just get them in the name.
+  if (DebuggerTuning == llvm::DebuggerKind::SCE)
+CmdArgs.push_back("-debug-forward-template-params");
+
   RenderDebugInfoCompressionArgs(Args, CmdArgs, D);
 }
 
Index: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
===
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp
@@ -528,6 +528,7 @@
   Opts.SplitDwarfInlining = !Args.hasArg(OPT_fno_split_dwarf_inlining);
   Opts.DebugTypeExtRefs = Args.hasArg(OPT_dwarf_ext_refs);
   Opts.DebugExplicitImport = Triple.isPS4CPU();
+  Opts.DebugFwdTemplateParams = Args.hasArg(OPT_debug_forward_template_params);
 
   for (const auto  : Args.getAllArgValues(OPT_fdebug_prefix_map_EQ))
 Opts.DebugPrefixMap.insert(StringRef(Arg).split('='));
Index: cfe/trunk/include/clang/Driver/CC1Options.td
===
--- cfe/trunk/include/clang/Driver/CC1Options.td
+++ cfe/trunk/include/clang/Driver/CC1Options.td
@@ -200,6 +200,9 @@
 def dwarf_ext_refs : Flag<["-"], "dwarf-ext-refs">,
   HelpText<"Generate debug info with external references to clang modules"
" or precompiled headers">;
+def debug_forward_template_params : Flag<["-"], 
"debug-forward-template-params">,
+  HelpText<"Emit complete descriptions of template parameters in forward"
+   " declarations">;
 def fforbid_guard_variables : Flag<["-"], "fforbid-guard-variables">,
   HelpText<"Emit an error if a C++ static local initializer would need a guard 
variable">;
 def no_implicit_float : Flag<["-"], "no-implicit-float">,
Index: cfe/trunk/include/clang/Frontend/CodeGenOptions.def
===
--- cfe/trunk/include/clang/Frontend/CodeGenOptions.def
+++ cfe/trunk/include/clang/Frontend/CodeGenOptions.def
@@ -219,6 +219,10 @@
 CODEGENOPT(SplitDwarfInlining, 1, 1) ///< Whether to include inlining info in 
the
  ///< skeleton CU to allow for 
symbolication
 ///< of inline stack frames without .dwo 
files.
+CODEGENOPT(DebugFwdTemplateParams, 1, 0) ///< Whether to emit complete
+ ///< template parameter descriptions 
in
+ ///< forward declarations (versus just
+ ///< including them in the name).
 
 CODEGENOPT(EmitLLVMUseLists, 1, 0) ///< Control whether to serialize use-lists.
 
Index: cfe/trunk/test/CodeGenCXX/debug-info-fwd-template-param.cpp
===
--- cfe/trunk/test/CodeGenCXX/debug-info-fwd-template-param.cpp
+++ cfe/trunk/test/CodeGenCXX/debug-info-fwd-template-param.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 %s -triple=%itanium_abi_triple -debug-info-kind=limited 
-debug-forward-template-params -emit-llvm -o - | FileCheck --check-prefix=CHILD 
%s
+// RUN: %clang_cc1 %s -triple=%itanium_abi_triple -debug-info-kind=limited 
-emit-llvm -o - | FileCheck --check-prefix=NONE %s
+// A DWARF forward declaration of a template instantiation 

[PATCH] D14358: DWARF's forward decl of a template should have template parameters.

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

I think I will go ahead and commit this; it doesn't change the status quo for 
CodeView and if something is warranted we can do that in a follow-up.


https://reviews.llvm.org/D14358



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D14358: DWARF's forward decl of a template should have template parameters.

[David Blaikie via cfe-commits]

On Wed, Sep 27, 2017 at 1:58 PM Paul Robinson via Phabricator <
revi...@reviews.llvm.org> wrote:

> probinson added a reviewer: rnk.
> probinson added a comment.
>
> +rnk for the CodeView question.
>
>
>
> 
> Comment at: include/clang/Frontend/CodeGenOptions.def:222
>  ///< of inline stack frames without
> .dwo files.
> +CODEGENOPT(DebugFwdTemplateParams, 1, 0) ///< Whether to emit complete
> + ///< template parameter
> descriptions in
> 
> dblaikie wrote:
> > Maybe 'Decl' rather than 'Fwd'.
> Well, in a sense they are all declarations, and 'Fwd' is a clearer
> statement of the distinction this flag is trying to make.  Unless you feel
> strongly I'd prefer to leave it as is.
>

Fair enough.


> 
> Comment at: lib/CodeGen/CGDebugInfo.cpp:836
>llvm::DINode::FlagFwdDecl, FullName);
> +  if (CGM.getCodeGenOpts().DebugFwdTemplateParams)
> +if (auto *TSpecial = dyn_cast(RD))
> 
> It just occurred to me... should CodeView care about this?
>

Not sure

Reid?


>
>
> 
> Comment at: test/CodeGenCXX/debug-info-fwd-template-param.cpp:7
> +template class A;
> +A *p;
> +
> 
> dblaikie wrote:
> > Any particular reason for const int, rather than int?
> It was the illustrative example of the difference between the demangler
> ("int const") and clang ("const int") that the debugger guys tripped over,
> and so was in the source I started with when creating this test.  I think
> you are correct, it is not important to have it.
>

*nod* I understand that it's part of the original issue, but doesn't seem
needed/relevant here.  Thanks! :)


>
>
>
> https://reviews.llvm.org/D14358
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D14358: DWARF's forward decl of a template should have template parameters.

[Paul Robinson via Phabricator via cfe-commits]

probinson added a reviewer: rnk.
probinson added a comment.

+rnk for the CodeView question.




Comment at: include/clang/Frontend/CodeGenOptions.def:222
 ///< of inline stack frames without .dwo 
files.
+CODEGENOPT(DebugFwdTemplateParams, 1, 0) ///< Whether to emit complete
+ ///< template parameter descriptions 
in

dblaikie wrote:
> Maybe 'Decl' rather than 'Fwd'.
Well, in a sense they are all declarations, and 'Fwd' is a clearer statement of 
the distinction this flag is trying to make.  Unless you feel strongly I'd 
prefer to leave it as is.



Comment at: lib/CodeGen/CGDebugInfo.cpp:836
   llvm::DINode::FlagFwdDecl, FullName);
+  if (CGM.getCodeGenOpts().DebugFwdTemplateParams)
+if (auto *TSpecial = dyn_cast(RD))

It just occurred to me... should CodeView care about this?



Comment at: test/CodeGenCXX/debug-info-fwd-template-param.cpp:7
+template class A;
+A *p;
+

dblaikie wrote:
> Any particular reason for const int, rather than int?
It was the illustrative example of the difference between the demangler ("int 
const") and clang ("const int") that the debugger guys tripped over, and so was 
in the source I started with when creating this test.  I think you are correct, 
it is not important to have it.



https://reviews.llvm.org/D14358



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D14358: DWARF's forward decl of a template should have template parameters.

[David Blaikie via Phabricator via cfe-commits]

dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

Looks OK to me - couple of minor questions.




Comment at: include/clang/Frontend/CodeGenOptions.def:222
 ///< of inline stack frames without .dwo 
files.
+CODEGENOPT(DebugFwdTemplateParams, 1, 0) ///< Whether to emit complete
+ ///< template parameter descriptions 
in

Maybe 'Decl' rather than 'Fwd'.



Comment at: test/CodeGenCXX/debug-info-fwd-template-param.cpp:7
+template class A;
+A *p;
+

Any particular reason for const int, rather than int?


https://reviews.llvm.org/D14358



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

RE: [PATCH] D14358: DWARF's forward decl of a template should have template parameters.

[Robinson, Paul via cfe-commits]

(though perhaps we had some position that debugger tuning wouldn't ever be the 
only way to access functionality, only change defaults)

That's correct.  Tuning must unpack to other settings that can be set 
separately.  That was very strong feedback from when we introduced the tuning 
concept.
--paulr

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D14358: DWARF's forward decl of a template should have template parameters.

[David Blaikie via cfe-commits]

On Wed, Sep 27, 2017 at 11:50 AM Paul Robinson via Phabricator <
revi...@reviews.llvm.org> wrote:

> probinson added a comment.
>
> In https://reviews.llvm.org/D14358#882445, @dblaikie wrote:
>
> > > I would prefer to eliminate the `` from the instance name as
> well, because our debugger reconstructs a name more to its liking from the
> parameter children.  However, IIUC the name with params is used for
> deduplication in LTO, so that is probably not such a good idea. :-)
> >
> > Though you have this out of tree? How do you cope with LTO there?
>
>
> We discovered that we had to keep them in the name for LTO.
>
> > I've not fully refreshed myself on the previous conversations - but it
> looks like my thought was that this state proposed here is
> weird/inconsistent: The parameters are already in the name, so adding them
> in the DIEs seems redundant. If the parameters weren't in the name then
> this change might make more sense.
>
> Our debugger throws away the params in the name, and relies on the
> children.  The names as rendered in DWARF by Clang are not textually
> consistent with names as rendered by the demangler.  Our debugger uses the
> children to construct names that are consistent with how the demangler
> works.  Then it can match up type names returned by the demangler to type
> names it has constructed.  Assuming I am not misunderstanding our debugger
> guys again, but that's my recollection.
>
> I believe we have talked previously about using a different scheme for
> deduplication that doesn't depend (or not so much) on the names.  If we had
> that, we could eliminate params from the name, and save probably a
> noticeable chunk of space in the string section.  I haven't tried to
> measure that, though.  But we have to have the children in place before we
> can experiment with other deduplication schemes.
>

Fair enough - yeah, I would agree with Adrian that this probably isn't a
driver flag (at least not yet), though. Either only driven by the debugger
tuning (though perhaps we had some position that debugger tuning wouldn't
ever be the only way to access functionality, only change defaults) or
additionally a cc1 flag. Haven't thought about the name.


>
> There is also the pedantic point that DWARF doesn't say these child
> entries are optional, or omitted for forward declarations.  I know gcc
> doesn't (or didn't, anyway; what I have locally is gcc 5.4) but gcc is not
> the arbiter of what constitutes conforming DWARF.
>
>
> https://reviews.llvm.org/D14358
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D14358: DWARF's forward decl of a template should have template parameters.

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In https://reviews.llvm.org/D14358#882445, @dblaikie wrote:

> > I would prefer to eliminate the `` from the instance name as well, 
> > because our debugger reconstructs a name more to its liking from the 
> > parameter children.  However, IIUC the name with params is used for 
> > deduplication in LTO, so that is probably not such a good idea. :-)
>
> Though you have this out of tree? How do you cope with LTO there?


We discovered that we had to keep them in the name for LTO.

> I've not fully refreshed myself on the previous conversations - but it looks 
> like my thought was that this state proposed here is weird/inconsistent: The 
> parameters are already in the name, so adding them in the DIEs seems 
> redundant. If the parameters weren't in the name then this change might make 
> more sense.

Our debugger throws away the params in the name, and relies on the children.  
The names as rendered in DWARF by Clang are not textually consistent with names 
as rendered by the demangler.  Our debugger uses the children to construct 
names that are consistent with how the demangler works.  Then it can match up 
type names returned by the demangler to type names it has constructed.  
Assuming I am not misunderstanding our debugger guys again, but that's my 
recollection.

I believe we have talked previously about using a different scheme for 
deduplication that doesn't depend (or not so much) on the names.  If we had 
that, we could eliminate params from the name, and save probably a noticeable 
chunk of space in the string section.  I haven't tried to measure that, though. 
 But we have to have the children in place before we can experiment with other 
deduplication schemes.

There is also the pedantic point that DWARF doesn't say these child entries are 
optional, or omitted for forward declarations.  I know gcc doesn't (or didn't, 
anyway; what I have locally is gcc 5.4) but gcc is not the arbiter of what 
constitutes conforming DWARF.


https://reviews.llvm.org/D14358



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D14358: DWARF's forward decl of a template should have template parameters.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

> I would prefer to eliminate the `` from the instance name as well, 
> because our debugger reconstructs a name more to its liking from the 
> parameter children.  However, IIUC the name with params is used for 
> deduplication in LTO, so that is probably not such a good idea. :-)

Though you have this out of tree? How do you cope with LTO there?

I've not fully refreshed myself on the previous conversations - but it looks 
like my thought was that this state proposed here is weird/inconsistent: The 
parameters are already in the name, so adding them in the DIEs seems redundant. 
If the parameters weren't in the name then this change might make more sense.




Comment at: test/CodeGenCXX/debug-info-fwd-template-param.cpp:6-17
+template class A {
+public:
+  A(T val);
+private:
+  T x;
+};
+

Probably simpler:

  template class A;
  A *p;

?


https://reviews.llvm.org/D14358



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D14358: DWARF's forward decl of a template should have template parameters.

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In https://reviews.llvm.org/D14358#881666, @aprantl wrote:

> Does this have to be exposed through the driver or could this be a cc1 option 
> only?


My thinking was to make it easier for LLDB to play with it and decide whether 
DWARF conformance on this point is a good thing for them also.  But I admit 
it's not something an end user would really care about or want to change.  I 
can make it a cc1 option.




Comment at: include/clang/Basic/LangOptions.def:144
 BENIGN_LANGOPT(EmitAllDecls  , 1, 0, "emitting all declarations")
+BENIGN_LANGOPT(EmitFwdTemplateChildren, 1, 0, "emit template parameter 
children in forward declarations")
 LANGOPT(MathErrno , 1, 1, "errno in math functions")

aprantl wrote:
> Why is this a LangOpt instead of a CodeGenOpt?
> Should it reference `debug` somewhere in the name?
Because I thought EmitAllDecls was for debug info, and this felt related.  But 
I see EmitAllDecls is not used in CGDebugInfo and generally we do put 
debug-related options in CodeGenOpt so I will move that.  (Someday we should 
collect all that stuff into a DebugOpt class.)
And I will rename it to DebugFwdTemplateChildren.


https://reviews.llvm.org/D14358



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D14358: DWARF's forward decl of a template should have template parameters.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

Does this have to be exposed through the driver or could this be a cc1 option 
only?




Comment at: include/clang/Basic/LangOptions.def:144
 BENIGN_LANGOPT(EmitAllDecls  , 1, 0, "emitting all declarations")
+BENIGN_LANGOPT(EmitFwdTemplateChildren, 1, 0, "emit template parameter 
children in forward declarations")
 LANGOPT(MathErrno , 1, 1, "errno in math functions")

Why is this a LangOpt instead of a CodeGenOpt?
Should it reference `debug` somewhere in the name?


https://reviews.llvm.org/D14358



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D14358: DWARF's forward decl of a template should have template parameters.

[Paul Robinson via Phabricator via cfe-commits]

probinson updated this revision to Diff 116711.
probinson added a comment.

Add a command-line flag to control emitting the template parameter children. 
Default to on for SCE debugger tuning.
I am not super excited about my choice of option name or the help text; 
alternate suggestions welcome.

I would prefer to eliminate the `` from the instance name as well, 
because our debugger reconstructs a name more to its liking from the parameter 
children.  However, IIUC the name with params is used for deduplication in LTO, 
so that is probably not such a good idea. :-)


https://reviews.llvm.org/D14358

Files:
  include/clang/Basic/LangOptions.def
  include/clang/Driver/Options.td
  lib/CodeGen/CGDebugInfo.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGenCXX/debug-info-fwd-template-param.cpp
  test/Driver/clang_f_opts.c

Index: test/Driver/clang_f_opts.c
===
--- test/Driver/clang_f_opts.c
+++ test/Driver/clang_f_opts.c
@@ -495,6 +495,11 @@
 // CHECK-PROFILE-DEBUG: -fdebug-info-for-profiling
 // CHECK-NO-PROFILE-DEBUG-NOT: -fdebug-info-for-profiling
 
+// RUN: %clang -### -S -fdebug-forward-template-params %s 2>&1 | FileCheck -check-prefix=CHECK-FWD-TMPL %s
+// RUN: %clang -### -S -fno-debug-forward-template-params %s 2>&1 | FileCheck -check-prefix=CHECK-NO-FWD-TMPL %s
+// CHECK-FWD-TMPL: -fdebug-forward-template-params
+// CHECK-NO-FWD-TMPL-NOT: -fdebug-forward-template-params
+
 // RUN: %clang -### -S -fallow-editor-placeholders %s 2>&1 | FileCheck -check-prefix=CHECK-ALLOW-PLACEHOLDERS %s
 // RUN: %clang -### -S -fno-allow-editor-placeholders %s 2>&1 | FileCheck -check-prefix=CHECK-NO-ALLOW-PLACEHOLDERS %s
 // CHECK-ALLOW-PLACEHOLDERS: -fallow-editor-placeholders
Index: test/CodeGenCXX/debug-info-fwd-template-param.cpp
===
--- test/CodeGenCXX/debug-info-fwd-template-param.cpp
+++ test/CodeGenCXX/debug-info-fwd-template-param.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 %s -triple=%itanium_abi_triple -debug-info-kind=limited -fdebug-forward-template-params -emit-llvm -o - | FileCheck --check-prefix=CHILD %s
+// RUN: %clang_cc1 %s -triple=%itanium_abi_triple -debug-info-kind=limited -fno-debug-forward-template-params -emit-llvm -o - | FileCheck --check-prefix=NONE %s
+// A forward declaration of a template instantiation should have template
+// parameter children (if we ask for them).
+
+template class A {
+public:
+  A(T val);
+private:
+  T x;
+};
+
+struct B {
+  A *p;
+};
+
+B b;
+
+// CHILD:  !DICompositeType(tag: DW_TAG_class_type, name: "A"
+// CHILD-SAME: flags: DIFlagFwdDecl
+// CHILD-SAME: templateParams: [[PARAM_LIST:![0-9]*]]
+// CHILD:  [[PARAM_LIST]] = !{[[PARAM:![0-9]*]]}
+// CHILD:  [[PARAM]] = !DITemplateTypeParameter(name: "T",
+// CHILD-SAME: type: [[CTYPE:![0-9]*]]
+// CHILD:  [[CTYPE]] = !DIDerivedType(tag: DW_TAG_const_type
+// CHILD-SAME: baseType: [[BTYPE:![0-9]*]]
+// CHILD:  [[BTYPE]] = !DIBasicType(name: "int"
+
+// NONE:   !DICompositeType(tag: DW_TAG_class_type, name: "A"
+// NONE-SAME:  flags: DIFlagFwdDecl
+// NONE-NOT:   templateParams:
+// NONE-SAME:  )
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2201,6 +2201,7 @@
   Opts.EncodeExtendedBlockSig =
 Args.hasArg(OPT_fencode_extended_block_signature);
   Opts.EmitAllDecls = Args.hasArg(OPT_femit_all_decls);
+  Opts.EmitFwdTemplateChildren = Args.hasArg(OPT_fdebug_forward_template_params);
   Opts.PackStruct = getLastArgIntValue(Args, OPT_fpack_struct_EQ, 0, Diags);
   Opts.MaxTypeAlign = getLastArgIntValue(Args, OPT_fmax_type_align_EQ, 0, Diags);
   Opts.AlignDouble = Args.hasArg(OPT_malign_double);
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -2969,6 +2969,13 @@
 CmdArgs.push_back("-generate-type-units");
   }
 
+  // Decide how to render forward declarations of template instantiations.
+  // SCE defaults to on, others default to off.
+  if (Args.hasFlag(options::OPT_fdebug_forward_template_params,
+   options::OPT_fno_debug_forward_template_params,
+   DebuggerTuning == llvm::DebuggerKind::SCE))
+CmdArgs.push_back("-fdebug-forward-template-params");
+
   RenderDebugInfoCompressionArgs(Args, CmdArgs, D);
 }
 
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -833,6 +833,10 @@
   llvm::DICompositeType *RetTy = DBuilder.createReplaceableCompositeType(
   getTagForRecord(RD), RDName, Ctx, DefUnit, Line, 0, Size, Align,
   llvm::DINode::FlagFwdDecl, FullName);
+  if 

Re: [PATCH] D14358: DWARF's forward decl of a template should have template parameters.

[David Blaikie via cfe-commits]

I'm roughly where I was before, I think:

"In any case, it seems like it might make sense for you to upstream your
template naming change and put it under the PS4 debugger tuning option, and
put this change there too, once the motivation for it is in-tree. At that
point, while I'd be curious about the size tradeoff, it'd be essentially
academic."

Basically without the naming change in-tree, there seems to be a lack of
motivation for this change. I'd prefer/recommend/suggest keeping this
change wherever that change is. (so if the naming change is upstreamed
(conditional/behind the PS4 debugger tuning flag) then I agree this
could/should go in similarly)

- Dave

On Tue, Jul 11, 2017 at 10:34 AM Paul Robinson via Phabricator <
revi...@reviews.llvm.org> wrote:

> probinson updated this revision to Diff 106063.
> probinson added a comment.
>
> Refresh to current TOT, and ping.  Funny what you can find in a year-old
> to-do list
>
>
> https://reviews.llvm.org/D14358
>
> Files:
>   lib/CodeGen/CGDebugInfo.cpp
>   test/CodeGenCXX/debug-info-template-fwd-param.cpp
>   test/Modules/ExtDebugInfo.cpp
>
>
> Index: test/Modules/ExtDebugInfo.cpp
> ===
> --- test/Modules/ExtDebugInfo.cpp
> +++ test/Modules/ExtDebugInfo.cpp
> @@ -93,9 +93,13 @@
>  // CHECK-SAME: flags: DIFlagFwdDecl,
>  // CHECK-SAME: identifier:
> "_ZTSN8DebugCXX8TemplateIiNS_6traitsIi")
>
> -// This type isn't, however, even with standalone non-module debug info
> this
> -// type is a forward declaration.
> -// CHECK-NOT: !DICompositeType(tag: DW_TAG_structure_type, name:
> "traits",
> +// This type isn't, however, it is a template parameter type and so gets a
> +// forward declaration.
> +// CHECK: !DICompositeType(tag: DW_TAG_structure_type,
> +// CHECK-SAME: name: "traits",
> +// CHECK-SAME: scope: ![[NS]],
> +// CHECK-SAME: flags: DIFlagFwdDecl,
> +// CHECK-SAME: identifier: "_ZTSN8DebugCXX6traitsIiEE")
>
>  // This one isn't.
>  // CHECK: !DICompositeType(tag: DW_TAG_class_type,
> Index: test/CodeGenCXX/debug-info-template-fwd-param.cpp
> ===
> --- test/CodeGenCXX/debug-info-template-fwd-param.cpp
> +++ test/CodeGenCXX/debug-info-template-fwd-param.cpp
> @@ -0,0 +1,25 @@
> +// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin
> -debug-info-kind=limited -emit-llvm -o - | FileCheck %s
> +// A forward declaration of a template should still have template
> parameters.
> +
> +template class A {
> +public:
> +  A(T val);
> +private:
> +  T x;
> +};
> +
> +struct B {
> +  A *p;
> +};
> +
> +B b;
> +
> +// CHECK:  !DICompositeType(tag: DW_TAG_class_type, name: "A int>"
> +// CHECK-SAME: flags: DIFlagFwdDecl
> +// CHECK-SAME: templateParams: [[PARAM_LIST:![0-9]*]]
> +// CHECK:  [[PARAM_LIST]] = !{[[PARAM:![0-9]*]]}
> +// CHECK:  [[PARAM]] = !DITemplateTypeParameter(name: "T",
> +// CHECK-SAME: type: [[CTYPE:![0-9]*]]
> +// CHECK:  [[CTYPE]] = !DIDerivedType(tag: DW_TAG_const_type
> +// CHECK-SAME: baseType: [[BTYPE:![0-9]*]]
> +// CHECK:  [[BTYPE]] = !DIBasicType(name: "int"
> Index: lib/CodeGen/CGDebugInfo.cpp
> ===
> --- lib/CodeGen/CGDebugInfo.cpp
> +++ lib/CodeGen/CGDebugInfo.cpp
> @@ -805,6 +805,10 @@
>llvm::DICompositeType *RetTy = DBuilder.createReplaceableCompositeType(
>getTagForRecord(RD), RDName, Ctx, DefUnit, Line, 0, Size, Align,
>llvm::DINode::FlagFwdDecl, FullName);
> +  if (const ClassTemplateSpecializationDecl *TSpecial =
> +  dyn_cast(RD))
> +DBuilder.replaceArrays(RetTy, llvm::DINodeArray(),
> +   CollectCXXTemplateParams(TSpecial, DefUnit));
>ReplaceMap.emplace_back(
>std::piecewise_construct, std::make_tuple(Ty),
>std::make_tuple(static_cast(RetTy)));
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D14358: DWARF's forward decl of a template should have template parameters.

[Paul Robinson via Phabricator via cfe-commits]

probinson updated this revision to Diff 106063.
probinson added a comment.

Refresh to current TOT, and ping.  Funny what you can find in a year-old to-do 
list


https://reviews.llvm.org/D14358

Files:
  lib/CodeGen/CGDebugInfo.cpp
  test/CodeGenCXX/debug-info-template-fwd-param.cpp
  test/Modules/ExtDebugInfo.cpp


Index: test/Modules/ExtDebugInfo.cpp
===
--- test/Modules/ExtDebugInfo.cpp
+++ test/Modules/ExtDebugInfo.cpp
@@ -93,9 +93,13 @@
 // CHECK-SAME: flags: DIFlagFwdDecl,
 // CHECK-SAME: identifier: 
"_ZTSN8DebugCXX8TemplateIiNS_6traitsIi")
 
-// This type isn't, however, even with standalone non-module debug info this
-// type is a forward declaration.
-// CHECK-NOT: !DICompositeType(tag: DW_TAG_structure_type, name: "traits",
+// This type isn't, however, it is a template parameter type and so gets a
+// forward declaration.
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type,
+// CHECK-SAME: name: "traits",
+// CHECK-SAME: scope: ![[NS]],
+// CHECK-SAME: flags: DIFlagFwdDecl,
+// CHECK-SAME: identifier: "_ZTSN8DebugCXX6traitsIiEE")
 
 // This one isn't.
 // CHECK: !DICompositeType(tag: DW_TAG_class_type,
Index: test/CodeGenCXX/debug-info-template-fwd-param.cpp
===
--- test/CodeGenCXX/debug-info-template-fwd-param.cpp
+++ test/CodeGenCXX/debug-info-template-fwd-param.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin -debug-info-kind=limited 
-emit-llvm -o - | FileCheck %s
+// A forward declaration of a template should still have template parameters.
+
+template class A {
+public:
+  A(T val);
+private:
+  T x;
+};
+
+struct B {
+  A *p;
+};
+
+B b;
+
+// CHECK:  !DICompositeType(tag: DW_TAG_class_type, name: "A"
+// CHECK-SAME: flags: DIFlagFwdDecl
+// CHECK-SAME: templateParams: [[PARAM_LIST:![0-9]*]]
+// CHECK:  [[PARAM_LIST]] = !{[[PARAM:![0-9]*]]}
+// CHECK:  [[PARAM]] = !DITemplateTypeParameter(name: "T",
+// CHECK-SAME: type: [[CTYPE:![0-9]*]]
+// CHECK:  [[CTYPE]] = !DIDerivedType(tag: DW_TAG_const_type
+// CHECK-SAME: baseType: [[BTYPE:![0-9]*]]
+// CHECK:  [[BTYPE]] = !DIBasicType(name: "int"
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -805,6 +805,10 @@
   llvm::DICompositeType *RetTy = DBuilder.createReplaceableCompositeType(
   getTagForRecord(RD), RDName, Ctx, DefUnit, Line, 0, Size, Align,
   llvm::DINode::FlagFwdDecl, FullName);
+  if (const ClassTemplateSpecializationDecl *TSpecial =
+  dyn_cast(RD))
+DBuilder.replaceArrays(RetTy, llvm::DINodeArray(),
+   CollectCXXTemplateParams(TSpecial, DefUnit));
   ReplaceMap.emplace_back(
   std::piecewise_construct, std::make_tuple(Ty),
   std::make_tuple(static_cast(RetTy)));


Index: test/Modules/ExtDebugInfo.cpp
===
--- test/Modules/ExtDebugInfo.cpp
+++ test/Modules/ExtDebugInfo.cpp
@@ -93,9 +93,13 @@
 // CHECK-SAME: flags: DIFlagFwdDecl,
 // CHECK-SAME: identifier: "_ZTSN8DebugCXX8TemplateIiNS_6traitsIi")
 
-// This type isn't, however, even with standalone non-module debug info this
-// type is a forward declaration.
-// CHECK-NOT: !DICompositeType(tag: DW_TAG_structure_type, name: "traits",
+// This type isn't, however, it is a template parameter type and so gets a
+// forward declaration.
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type,
+// CHECK-SAME: name: "traits",
+// CHECK-SAME: scope: ![[NS]],
+// CHECK-SAME: flags: DIFlagFwdDecl,
+// CHECK-SAME: identifier: "_ZTSN8DebugCXX6traitsIiEE")
 
 // This one isn't.
 // CHECK: !DICompositeType(tag: DW_TAG_class_type,
Index: test/CodeGenCXX/debug-info-template-fwd-param.cpp
===
--- test/CodeGenCXX/debug-info-template-fwd-param.cpp
+++ test/CodeGenCXX/debug-info-template-fwd-param.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin -debug-info-kind=limited -emit-llvm -o - | FileCheck %s
+// A forward declaration of a template should still have template parameters.
+
+template class A {
+public:
+  A(T val);
+private:
+  T x;
+};
+
+struct B {
+  A *p;
+};
+
+B b;
+
+// CHECK:  !DICompositeType(tag: DW_TAG_class_type, name: "A"
+// CHECK-SAME: flags: DIFlagFwdDecl
+// CHECK-SAME: templateParams: [[PARAM_LIST:![0-9]*]]
+// CHECK:  [[PARAM_LIST]] = !{[[PARAM:![0-9]*]]}
+// CHECK:  [[PARAM]] = !DITemplateTypeParameter(name: "T",
+// CHECK-SAME: type: [[CTYPE:![0-9]*]]
+// CHECK:  [[CTYPE]] = !DIDerivedType(tag: DW_TAG_const_type
+// CHECK-SAME: baseType: [[BTYPE:![0-9]*]]
+// CHECK:  

RE: [PATCH] D14358: DWARF's forward decl of a template should have template parameters.

[Robinson, Paul via cfe-commits]

Actually no, we prefer to have the original typedef names in the instantiation 
name, for source fidelity.  "Name as it is in the source" or something 
reasonably close.  Unwrapping typedefs is going too far.

Re. "looseness" of the DWARF spec, it is not so loose as you like to think.  
Attributes tend to be fairly optional or can be used "in novel ways" but the 
DIEs and their relationships are not like that.  "Where this specification 
provides a means for describing the source language, implementors are expected 
to adhere to that specification."
--paulr

From: David Blaikie [mailto:dblai...@gmail.com]
Sent: Wednesday, December 09, 2015 10:49 AM
To: Robinson, Paul
Cc: Marshall, Peter; llvm-dev; cfe-commits (cfe-commits@lists.llvm.org)
Subject: Re: [PATCH] D14358: DWARF's forward decl of a template should have 
template parameters.



On Wed, Dec 9, 2015 at 10:40 AM, Robinson, Paul 
<paul_robin...@playstation.sony.com<mailto:paul_robin...@playstation.sony.com>> 
wrote:
That doesn't seem to be the DWARF I'm seeing from Clang (& it'd be surprising 
if we used the typedef (or otherwise non-canonical) name in the class name):

Finally getting back to this…..  Ha.  We don't unwrap the typedefs ("name as it 
is in the source"), while the upstream compiler does.

Yeah, I imagine you'd want to fix that as I expect it would cause you other 
problems, no? (or is there some reason you have this change to the compiler? I 
imagine it'd be hard to have that divergence by accident?)

Providing the template-parameter DIEs is still the correct thing to do per the 
DWARF
spec.

I still don't agree that the DWARF we produce here is incorrect (the DWARF spec 
is pretty loose on "correctness" of DWARF). If there's some practical 
problem/use case it'd be useful to understand it so we make sure we're fixing 
it the right way.

- Dave

--paulr

From: David Blaikie [mailto:dblai...@gmail.com<mailto:dblai...@gmail.com>]
Sent: Friday, November 13, 2015 11:21 AM
To: Marshall, Peter; llvm-dev
Cc: Robinson, Paul

Subject: Re: [PATCH] D14358: DWARF's forward decl of a template should have 
template parameters.



On Fri, Nov 13, 2015 at 6:16 AM, 
<peter_marsh...@sn.scee.net<mailto:peter_marsh...@sn.scee.net>> wrote:
Hi Paul,

Sorry for the delay, I've been out of the office.

I think this example shows that name matching does not always work:

template class A {
public:
A(T val);
private:
T x;
};

struct B {
typedef float MONKEY;

A *p;
};

B b;

struct C {
typedef int MONKEY;

A *p;
};

C c;

This gives this DWARF:

+-003f DW_TAG_structure_type "B"
   -DW_AT_name  DW_FORM_strp  "B"
  +-0047 DW_TAG_member "p"
 -DW_AT_name  DW_FORM_strp  "p"
+-DW_AT_type  DW_FORM_ref4  0x0054
  +-0054 DW_TAG_pointer_type
+-DW_AT_type  DW_FORM_ref4  0x0059
  +-0059 DW_TAG_class_type "A"
 -DW_AT_name  DW_FORM_strp  "A"
 -DW_AT_declaration  DW_FORM_flag_present

+-0073 DW_TAG_structure_type "C"
   -DW_AT_name  DW_FORM_strp  "C"
  +-007b DW_TAG_member "p"
 -DW_AT_name  DW_FORM_strp  "p"
+-DW_AT_type  DW_FORM_ref4  0x0088
  +-0088 DW_TAG_pointer_type
+-DW_AT_type  DW_FORM_ref4  0x008d
  +-008d DW_TAG_class_type "A"
 -DW_AT_name  DW_FORM_strp  "A"
 -DW_AT_declaration  DW_FORM_flag_present

That doesn't seem to be the DWARF I'm seeing from Clang (& it'd be surprising 
if we used the typedef (or otherwise non-canonical) name in the class name):

(I've trimmed a few irrelevant attributes)
0x001e:   DW_TAG_variable [2]
DW_AT_name [DW_FORM_strp]   ( .debug_str[0x004c] = "b")
DW_AT_type [DW_FORM_ref4]   (cu + 0x0033 => {0x0033})

0x0033:   DW_TAG_structure_type [3] *
DW_AT_name [DW_FORM_strp]   ( .debug_str[0x0059] = "B")

0x003b: DW_TAG_member [4]
  DW_AT_name [DW_FORM_strp] ( .debug_str[0x004e] = "p")
  DW_AT_type [DW_FORM_ref4] (cu + 0x0048 => {0x0048})

0x0047: NULL

0x0048:   DW_TAG_pointer_type [5]
DW_AT_type [DW_FORM_ref4]   (cu + 0x004d => {0x004d})

0x004d:   DW_TAG_class_type [6]
DW_AT_name [DW_FORM_strp]   ( .debug_str[0x0050] = 
"A")
DW_AT_declaration [DW_FORM_flag_present](true)

0x0052:   DW_TAG_variable [2]
DW_AT_name [DW_FORM_strp]   ( .debug_str[0x005b] = "c")
DW_AT_type [DW_FORM_ref4]   (cu + 0x0067 => {0x0067})

0x0067:   DW_TAG_structure_type [3] *
DW_AT_name [DW_FORM_strp]   (

Re: [PATCH] D14358: DWARF's forward decl of a template should have template parameters.

[David Blaikie via cfe-commits]

On Wed, Dec 9, 2015 at 11:11 AM, Robinson, Paul <
paul_robin...@playstation.sony.com> wrote:

> Actually no, we prefer to have the original typedef names in the
> instantiation name, for source fidelity.
>

Then perhaps you should keep this change in your tree too - since that's
where the need is?


>   "Name as it is in the source" or something reasonably close.  Unwrapping
> typedefs is going too far.
>

Yet this isn't the choice upstream in Clang or GCC. I don't know about
other DWARF generators, but it seems your interpretation isn't the way some
other people/implementers are reading the DWARF spec.

[This seems like it would present a multitude of challenges to any DWARF
debugger dealing with this kind of debug info - it'd have to know far more
about the rules of the C++ language (which you've previously argued in
favor of avoiding) to perform a variety of operations if the types don't
match up fairly trivially.]

In any case, arguably 5.5.8 (Class Template Instantiations) 1 only applies
to definitions of a type, not declarations. ("Each formal parameterized
type declaration appearing in the template definition is represented by a
debugging information entry with the tag DW_TAG_template_type_parameter")
which, I agree, seems like a bug in the spec to not /allow/ them on
declarations, but I'd equally argue requiring them would seem too narrow to
me.


> Re. "looseness" of the DWARF spec, it is not so loose as you like to
> think.  Attributes tend to be fairly optional or can be used "in novel
> ways" but the DIEs and their relationships are not like that.  "Where this
> specification provides a means for describing the source language,
> implementors are expected to adhere to that specification."
>

Why would that clause apply to attributes any less than it applies to DIEs?
It seems like a fairly broad statement.

I forget whether we already discussed it - but do you have any size data
(preferably/possibly from a fission build or otherwise measurement of "just
the debug info" not the whole binary) on, for example, a clang selfhost?

- Dave


> --paulr
>
>
>
> *From:* David Blaikie [mailto:dblai...@gmail.com]
> *Sent:* Wednesday, December 09, 2015 10:49 AM
> *To:* Robinson, Paul
> *Cc:* Marshall, Peter; llvm-dev; cfe-commits (cfe-commits@lists.llvm.org)
>
> *Subject:* Re: [PATCH] D14358: DWARF's forward decl of a template should
> have template parameters.
>
>
>
>
>
>
>
> On Wed, Dec 9, 2015 at 10:40 AM, Robinson, Paul <
> paul_robin...@playstation.sony.com> wrote:
>
> That doesn't seem to be the DWARF I'm seeing from Clang (& it'd be
> surprising if we used the typedef (or otherwise non-canonical) name in the
> class name):
>
>
>
> Finally getting back to this…..  Ha.  We don't unwrap the typedefs ("name
> as it is in the source"), while the upstream compiler does.
>
>
>
> Yeah, I imagine you'd want to fix that as I expect it would cause you
> other problems, no? (or is there some reason you have this change to the
> compiler? I imagine it'd be hard to have that divergence by accident?)
>
>
>
> Providing the template-parameter DIEs is still the correct thing to do per
> the DWARF
>
> spec.
>
>
>
> I still don't agree that the DWARF we produce here is incorrect (the DWARF
> spec is pretty loose on "correctness" of DWARF). If there's some practical
> problem/use case it'd be useful to understand it so we make sure we're
> fixing it the right way.
>
> - Dave
>
>
>
> --paulr
>
>
>
> *From:* David Blaikie [mailto:dblai...@gmail.com]
> *Sent:* Friday, November 13, 2015 11:21 AM
> *To:* Marshall, Peter; llvm-dev
> *Cc:* Robinson, Paul
>
>
> *Subject:* Re: [PATCH] D14358: DWARF's forward decl of a template should
> have template parameters.
>
>
>
>
>
>
>
> On Fri, Nov 13, 2015 at 6:16 AM, <peter_marsh...@sn.scee.net> wrote:
>
> Hi Paul,
>
> Sorry for the delay, I've been out of the office.
>
> I think this example shows that name matching does not always work:
>
> template class A {
> public:
> A(T val);
> private:
> T x;
> };
>
> struct B {
> typedef float MONKEY;
>
> A *p;
> };
>
> B b;
>
> struct C {
> typedef int MONKEY;
>
> A *p;
> };
>
> C c;
>
> This gives this DWARF:
>
> +-003f DW_TAG_structure_type "B"
>-DW_AT_name  DW_FORM_strp  "B"
>   +-0047 DW_TAG_member "p"
>  -DW_AT_name  DW_FORM_strp  "p"
> +-DW_AT_type  DW_FORM_ref4  0x0054
>   +-0054 DW_TAG_pointer_type
> +-DW_AT_type  DW_FORM_ref4  0x0

RE: [PATCH] D14358: DWARF's forward decl of a template should have template parameters.

[Robinson, Paul via cfe-commits]

That doesn't seem to be the DWARF I'm seeing from Clang (& it'd be surprising 
if we used the typedef (or otherwise non-canonical) name in the class name):

Finally getting back to this…..  Ha.  We don't unwrap the typedefs ("name as it 
is in the source"), while the upstream compiler does.
Providing the template-parameter DIEs is still the correct thing to do per the 
DWARF spec.
--paulr

From: David Blaikie [mailto:dblai...@gmail.com]
Sent: Friday, November 13, 2015 11:21 AM
To: Marshall, Peter; llvm-dev
Cc: Robinson, Paul
Subject: Re: [PATCH] D14358: DWARF's forward decl of a template should have 
template parameters.



On Fri, Nov 13, 2015 at 6:16 AM, 
<peter_marsh...@sn.scee.net<mailto:peter_marsh...@sn.scee.net>> wrote:
Hi Paul,

Sorry for the delay, I've been out of the office.

I think this example shows that name matching does not always work:

template class A {
public:
A(T val);
private:
T x;
};

struct B {
typedef float MONKEY;

A *p;
};

B b;

struct C {
typedef int MONKEY;

A *p;
};

C c;

This gives this DWARF:

+-003f DW_TAG_structure_type "B"
   -DW_AT_name  DW_FORM_strp  "B"
  +-0047 DW_TAG_member "p"
 -DW_AT_name  DW_FORM_strp  "p"
+-DW_AT_type  DW_FORM_ref4  0x0054
  +-0054 DW_TAG_pointer_type
+-DW_AT_type  DW_FORM_ref4  0x0059
  +-0059 DW_TAG_class_type "A"
 -DW_AT_name  DW_FORM_strp  "A"
 -DW_AT_declaration  DW_FORM_flag_present

+-0073 DW_TAG_structure_type "C"
   -DW_AT_name  DW_FORM_strp  "C"
  +-007b DW_TAG_member "p"
 -DW_AT_name  DW_FORM_strp  "p"
+-DW_AT_type  DW_FORM_ref4  0x0088
  +-0088 DW_TAG_pointer_type
+-DW_AT_type  DW_FORM_ref4  0x008d
  +-008d DW_TAG_class_type "A"
 -DW_AT_name  DW_FORM_strp  "A"
 -DW_AT_declaration  DW_FORM_flag_present

That doesn't seem to be the DWARF I'm seeing from Clang (& it'd be surprising 
if we used the typedef (or otherwise non-canonical) name in the class name):

(I've trimmed a few irrelevant attributes)
0x001e:   DW_TAG_variable [2]
DW_AT_name [DW_FORM_strp]   ( .debug_str[0x004c] = "b")
DW_AT_type [DW_FORM_ref4]   (cu + 0x0033 => {0x0033})

0x0033:   DW_TAG_structure_type [3] *
DW_AT_name [DW_FORM_strp]   ( .debug_str[0x0059] = "B")

0x003b: DW_TAG_member [4]
  DW_AT_name [DW_FORM_strp] ( .debug_str[0x004e] = "p")
  DW_AT_type [DW_FORM_ref4] (cu + 0x0048 => {0x0048})

0x0047: NULL

0x0048:   DW_TAG_pointer_type [5]
DW_AT_type [DW_FORM_ref4]   (cu + 0x004d => {0x004d})

0x004d:   DW_TAG_class_type [6]
DW_AT_name [DW_FORM_strp]   ( .debug_str[0x0050] = 
"A")
DW_AT_declaration [DW_FORM_flag_present](true)

0x0052:   DW_TAG_variable [2]
DW_AT_name [DW_FORM_strp]   ( .debug_str[0x005b] = "c")
DW_AT_type [DW_FORM_ref4]   (cu + 0x0067 => {0x0067})

0x0067:   DW_TAG_structure_type [3] *
DW_AT_name [DW_FORM_strp]   ( .debug_str[0x0064] = "C")

0x006f: DW_TAG_member [4]
  DW_AT_name [DW_FORM_strp] ( .debug_str[0x004e] = "p")
  DW_AT_type [DW_FORM_ref4] (cu + 0x007c => {0x007c})

0x007b: NULL

0x007c:   DW_TAG_pointer_type [5]
DW_AT_type [DW_FORM_ref4]   (cu + 0x0081 => {0x0081})

0x0081:   DW_TAG_class_type [6]
DW_AT_name [DW_FORM_strp]   ( .debug_str[0x005d] = 
"A")
DW_AT_declaration [DW_FORM_flag_present](true)



As there are no template parameters for the forward declaration of either 
A
they are indistinguishable.

The reason we currently have no need for the parameters in a template name is 
because we
reconstruct template names from their parameter tags. This allow the pretty 
printing to match
the templates from the DWARF to match our demangled symbols from the ELF symbol 
table.

-Pete




From:"Robinson, Paul" 
<paul_robin...@playstation.sony.com<mailto:paul_robin...@playstation.sony.com>>
To:David Blaikie <dblai...@gmail.com<mailto:dblai...@gmail.com>>, 
"Marshall, Peter" 
<peter_marsh...@sn.scee.net<mailto:peter_marsh...@sn.scee.net>>
Cc:
"reviews+d14358+public+d3104135076f0...@reviews.llvm.org<mailto:reviews%2bd14358%2bpublic%2bd3104135076f0...@reviews.llvm.org>"

<reviews+d14358+public+d3104135076f0...@reviews.llvm.org<mailto:reviews%2bd14358

Re: [PATCH] D14358: DWARF's forward decl of a template should have template parameters.

[David Blaikie via cfe-commits]

On Wed, Dec 9, 2015 at 10:40 AM, Robinson, Paul <
paul_robin...@playstation.sony.com> wrote:

> That doesn't seem to be the DWARF I'm seeing from Clang (& it'd be
> surprising if we used the typedef (or otherwise non-canonical) name in the
> class name):
>
>
>
> Finally getting back to this…..  Ha.  We don't unwrap the typedefs ("name
> as it is in the source"), while the upstream compiler does.
>

Yeah, I imagine you'd want to fix that as I expect it would cause you other
problems, no? (or is there some reason you have this change to the
compiler? I imagine it'd be hard to have that divergence by accident?)


> Providing the template-parameter DIEs is still the correct thing to do per
> the DWARF
>
spec.
>

I still don't agree that the DWARF we produce here is incorrect (the DWARF
spec is pretty loose on "correctness" of DWARF). If there's some practical
problem/use case it'd be useful to understand it so we make sure we're
fixing it the right way.

- Dave


> --paulr
>
>
>
> *From:* David Blaikie [mailto:dblai...@gmail.com]
> *Sent:* Friday, November 13, 2015 11:21 AM
> *To:* Marshall, Peter; llvm-dev
> *Cc:* Robinson, Paul
>
> *Subject:* Re: [PATCH] D14358: DWARF's forward decl of a template should
> have template parameters.
>
>
>
>
>
>
>
> On Fri, Nov 13, 2015 at 6:16 AM, <peter_marsh...@sn.scee.net> wrote:
>
> Hi Paul,
>
> Sorry for the delay, I've been out of the office.
>
> I think this example shows that name matching does not always work:
>
> template class A {
> public:
> A(T val);
> private:
> T x;
> };
>
> struct B {
> typedef float MONKEY;
>
> A *p;
> };
>
> B b;
>
> struct C {
> typedef int MONKEY;
>
> A *p;
> };
>
> C c;
>
> This gives this DWARF:
>
> +-003f DW_TAG_structure_type "B"
>-DW_AT_name  DW_FORM_strp  "B"
>   +-0047 DW_TAG_member "p"
>  -DW_AT_name  DW_FORM_strp  "p"
> +-DW_AT_type  DW_FORM_ref4  0x0054
>   +-0054 DW_TAG_pointer_type
> +-DW_AT_type  DW_FORM_ref4  0x0059
>   +-0059 DW_TAG_class_type "A"
>  -DW_AT_name  DW_FORM_strp  "A"
>  -DW_AT_declaration  DW_FORM_flag_present
>
> +-0073 DW_TAG_structure_type "C"
>-DW_AT_name  DW_FORM_strp  "C"
>   +-007b DW_TAG_member "p"
>  -DW_AT_name  DW_FORM_strp  "p"
> +-DW_AT_type  DW_FORM_ref4  0x0088
>   +-0088 DW_TAG_pointer_type
> +-DW_AT_type  DW_FORM_ref4  0x008d
>   +-008d DW_TAG_class_type "A"
>  -DW_AT_name  DW_FORM_strp  "A"
>  -DW_AT_declaration  DW_FORM_flag_present
>
>
>
> That doesn't seem to be the DWARF I'm seeing from Clang (& it'd be
> surprising if we used the typedef (or otherwise non-canonical) name in the
> class name):
>
> (I've trimmed a few irrelevant attributes)
>
> 0x001e:   DW_TAG_variable [2]
>
> DW_AT_name [DW_FORM_strp]   ( .debug_str[0x004c] =
> "b")
>
> DW_AT_type [DW_FORM_ref4]   (cu + 0x0033 =>
> {0x0033})
>
>
>
> 0x0033:   DW_TAG_structure_type [3] *
>
> DW_AT_name [DW_FORM_strp]   ( .debug_str[0x0059] =
> "B")
>
>
>
> 0x003b: DW_TAG_member [4]
>
>   DW_AT_name [DW_FORM_strp] ( .debug_str[0x004e] =
> "p")
>
>   DW_AT_type [DW_FORM_ref4] (cu + 0x0048 =>
> {0x0048})
>
>
>
> 0x0047: NULL
>
>
>
> 0x0048:   DW_TAG_pointer_type [5]
>
> DW_AT_type [DW_FORM_ref4]   (cu + 0x004d =>
> {0x004d})
>
>
>
> 0x004d:   DW_TAG_class_type [6]
>
> DW_AT_name [DW_FORM_strp]   ( .debug_str[0x0050] =
> "A")
>
> DW_AT_declaration [DW_FORM_flag_present](true)
>
>
>
> 0x0052:   DW_TAG_variable [2]
>
> DW_AT_name [DW_FORM_strp]   ( .debug_str[0x005b] =
> "c")
>
> DW_AT_type [DW_FORM_ref4]   (cu + 0x0067 =>
> {0x0067})
>
>
>
> 0x0067:   DW_TAG_structure_type [3] *
>
> DW_AT_name [DW_FORM_strp]   ( .debug_str[0x0064] =
> "C")
>
>
>
> 0x006f: DW_TAG_member [4]
>
>   DW_AT_name [DW_FORM_strp] ( .debug_str[0x004e] =
> "p")
>
>   DW_AT_type [DW_FORM_ref4] (cu +

RE: [PATCH] D14358: DWARF's forward decl of a template should have template parameters.

[Robinson, Paul via cfe-commits]

| Types are a bit more vague (as to whether omitting unreferenced types is 
supported by the standard) DWARF 4 just says "Structure, union, and class types 
are represented by debugging information entries ...".

There's some expansion of the "permissive" discussion in the works for DWARF 5. 
 In essence, DWARF doesn't tell you _what_ to describe, but if you describe 
something, you do it _how_ the spec says.  So, omitting unused types and 
function declarations, or lexical blocks with no containing declarations, or 
even things like inlined subroutines, etc. etc. is all kosher, while things 
like the template parameter DIEs and the artificial import of anonymous 
namespaces are actually required.

| Any size numbers for this change?
I got in the neighborhood of 1% (just under, IIRC) of the sum of .debug_* 
sections for a self-build of Clang.

| In any case, it seems like it might make sense for you to upstream your 
template naming change and put it under the PS4 debugger tuning option, and put 
this change there too, once the motivation for it is in-tree. At that point, 
while I'd be curious about the size tradeoff, it'd be essentially academic

Exposing tuning up through Clang is actually very nearly at the top of my list 
now.
--paulr

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D14358: DWARF's forward decl of a template should have template parameters.

[David Blaikie via cfe-commits]

On Wed, Dec 9, 2015 at 11:59 AM, Robinson, Paul <
paul_robin...@playstation.sony.com> wrote:

> Maybe we are being too pedantic about the names.  I'll have to go back and
> look in detail at why we decided to do that.
>
>
>
> In any case, arguably 5.5.8 (Class Template Instantiations) 1 only applies
> to definitions of a type, not declarations. ("Each formal parameterized
> type declaration appearing in the template definition is represented by a
> debugging information entry with the tag DW_TAG_template_type_parameter")
>
> Not so fast… It's a template definition of a type declaration.  DWARF 5 is
> less ambiguous about this IMO, although you are actually very good at
> finding the ambiguities!  The relevant text in DWARF 5 current draft is: "A
> debugging information entry that represents a template instantiation will
> contain child entries describing the actual template parameters."  Are you
> willing to argue that this type declaration is not an instantiation?  (If
> not, what is it?)
>

Nope, it is an instantiation, for sure.

Beyond that, I'd still argue that existing implementation experience points
to this being pretty reasonable - and without the motivating use case
(which only exists in your fork) it seems hard to justify adding the extra
data.


>  Why would that clause apply to attributes any less than it applies to
> DIEs?
>
> It does apply equally.  However, nearly all attribute descriptions are
> specified as "may have" and therefore can be omitted freely without being
> non-conforming.
>

Fair point (& the same goes for the example I was going to give of omitting
DW_TAG_friends - they're "may" too)

Types are a bit more vague (as to whether omitting unreferenced types is
supported by the standard) DWARF 4 just says "Structure, union, and class
types are represented by debugging information entries ...".

Any size numbers for this change?

In any case, it seems like it might make sense for you to upstream your
template naming change and put it under the PS4 debugger tuning option, and
put this change there too, once the motivation for it is in-tree. At that
point, while I'd be curious about the size tradeoff, it'd be essentially
academic.

- David


> --paulr
>
>
>
> *From:* David Blaikie [mailto:dblai...@gmail.com]
> *Sent:* Wednesday, December 09, 2015 11:28 AM
>
> *To:* Robinson, Paul
> *Cc:* Marshall, Peter; llvm-dev; cfe-commits (cfe-commits@lists.llvm.org)
> *Subject:* Re: [PATCH] D14358: DWARF's forward decl of a template should
> have template parameters.
>
>
>
>
>
>
>
> On Wed, Dec 9, 2015 at 11:11 AM, Robinson, Paul <
> paul_robin...@playstation.sony.com> wrote:
>
> Actually no, we prefer to have the original typedef names in the
> instantiation name, for source fidelity.
>
>
>
> Then perhaps you should keep this change in your tree too - since that's
> where the need is?
>
>
>
>   "Name as it is in the source" or something reasonably close.  Unwrapping
> typedefs is going too far.
>
>
>
> Yet this isn't the choice upstream in Clang or GCC. I don't know about
> other DWARF generators, but it seems your interpretation isn't the way some
> other people/implementers are reading the DWARF spec.
>
> [This seems like it would present a multitude of challenges to any DWARF
> debugger dealing with this kind of debug info - it'd have to know far more
> about the rules of the C++ language (which you've previously argued in
> favor of avoiding) to perform a variety of operations if the types don't
> match up fairly trivially.]
>
> In any case, arguably 5.5.8 (Class Template Instantiations) 1 only applies
> to definitions of a type, not declarations. ("Each formal parameterized
> type declaration appearing in the template definition is represented by a
> debugging information entry with the tag DW_TAG_template_type_parameter")
> which, I agree, seems like a bug in the spec to not /allow/ them on
> declarations, but I'd equally argue requiring them would seem too narrow to
> me.
>
>
>
> Re. "looseness" of the DWARF spec, it is not so loose as you like to
> think.  Attributes tend to be fairly optional or can be used "in novel
> ways" but the DIEs and their relationships are not like that.  "Where this
> specification provides a means for describing the source language,
> implementors are expected to adhere to that specification."
>
>
>
> Why would that clause apply to attributes any less than it applies to
> DIEs? It seems like a fairly broad statement.
>
> I forget whether we already discussed it - but do you have any size data
> (preferably/possibly from a fission build or otherwise measurement of 

Re: [PATCH] D14358: DWARF's forward decl of a template should have template parameters.

[David Blaikie via cfe-commits]

On Wed, Dec 9, 2015 at 12:46 PM, Robinson, Paul <
paul_robin...@playstation.sony.com> wrote:

> | Types are a bit more vague (as to whether omitting unreferenced types is
> supported by the standard) DWARF 4 just says "Structure, union, and class
> types are represented by debugging information entries ...".
>
> There's some expansion of the "permissive" discussion in the works for
> DWARF 5.  In essence, DWARF doesn't tell you _what_ to describe, but if you
> describe something, you do it _how_ the spec says.  So, omitting unused
> types and function declarations, or lexical blocks with no containing
> declarations, or even things like inlined subroutines, etc. etc. is all
> kosher, while things like the template parameter DIEs and the artificial
> import of anonymous namespaces are actually required.
>

Honestly I'm more with you on the template parameter DIEs than I am on the
anonymous namespace... especially from a source fidelity perspective.


>
>
> | Any size numbers for this change?
>
> I got in the neighborhood of 1% (just under, IIRC) of the sum of .debug_*
> sections for a self-build of Clang.
>

Ah, cool - good to know. Thanks!


>
>
> | In any case, it seems like it might make sense for you to upstream your
> template naming change and put it under the PS4 debugger tuning option, and
> put this change there too, once the motivation for it is in-tree. At that
> point, while I'd be curious about the size tradeoff, it'd be essentially
> academic
>
>
>
> Exposing tuning up through Clang is actually very nearly at the top of my
> list now.
>

Great :)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

RE: [PATCH] D14358: DWARF's forward decl of a template should have template parameters.

[Robinson, Paul via cfe-commits]

Maybe we are being too pedantic about the names.  I'll have to go back and look 
in detail at why we decided to do that.

In any case, arguably 5.5.8 (Class Template Instantiations) 1 only applies to 
definitions of a type, not declarations. ("Each formal parameterized type 
declaration appearing in the template definition is represented by a debugging 
information entry with the tag DW_TAG_template_type_parameter")
Not so fast… It's a template definition of a type declaration.  DWARF 5 is less 
ambiguous about this IMO, although you are actually very good at finding the 
ambiguities!  The relevant text in DWARF 5 current draft is: "A debugging 
information entry that represents a template instantiation will contain child 
entries describing the actual template parameters."  Are you willing to argue 
that this type declaration is not an instantiation?  (If not, what is it?)

Why would that clause apply to attributes any less than it applies to DIEs?
It does apply equally.  However, nearly all attribute descriptions are 
specified as "may have" and therefore can be omitted freely without being 
non-conforming.
--paulr

From: David Blaikie [mailto:dblai...@gmail.com]
Sent: Wednesday, December 09, 2015 11:28 AM
To: Robinson, Paul
Cc: Marshall, Peter; llvm-dev; cfe-commits (cfe-commits@lists.llvm.org)
Subject: Re: [PATCH] D14358: DWARF's forward decl of a template should have 
template parameters.



On Wed, Dec 9, 2015 at 11:11 AM, Robinson, Paul 
<paul_robin...@playstation.sony.com<mailto:paul_robin...@playstation.sony.com>> 
wrote:
Actually no, we prefer to have the original typedef names in the instantiation 
name, for source fidelity.

Then perhaps you should keep this change in your tree too - since that's where 
the need is?

  "Name as it is in the source" or something reasonably close.  Unwrapping 
typedefs is going too far.

Yet this isn't the choice upstream in Clang or GCC. I don't know about other 
DWARF generators, but it seems your interpretation isn't the way some other 
people/implementers are reading the DWARF spec.

[This seems like it would present a multitude of challenges to any DWARF 
debugger dealing with this kind of debug info - it'd have to know far more 
about the rules of the C++ language (which you've previously argued in favor of 
avoiding) to perform a variety of operations if the types don't match up fairly 
trivially.]

In any case, arguably 5.5.8 (Class Template Instantiations) 1 only applies to 
definitions of a type, not declarations. ("Each formal parameterized type 
declaration appearing in the template definition is represented by a debugging 
information entry with the tag DW_TAG_template_type_parameter") which, I agree, 
seems like a bug in the spec to not /allow/ them on declarations, but I'd 
equally argue requiring them would seem too narrow to me.

Re. "looseness" of the DWARF spec, it is not so loose as you like to think.  
Attributes tend to be fairly optional or can be used "in novel ways" but the 
DIEs and their relationships are not like that.  "Where this specification 
provides a means for describing the source language, implementors are expected 
to adhere to that specification."

Why would that clause apply to attributes any less than it applies to DIEs? It 
seems like a fairly broad statement.

I forget whether we already discussed it - but do you have any size data 
(preferably/possibly from a fission build or otherwise measurement of "just the 
debug info" not the whole binary) on, for example, a clang selfhost?

- Dave

--paulr

From: David Blaikie [mailto:dblai...@gmail.com<mailto:dblai...@gmail.com>]
Sent: Wednesday, December 09, 2015 10:49 AM
To: Robinson, Paul
Cc: Marshall, Peter; llvm-dev; cfe-commits 
(cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>)

Subject: Re: [PATCH] D14358: DWARF's forward decl of a template should have 
template parameters.



On Wed, Dec 9, 2015 at 10:40 AM, Robinson, Paul 
<paul_robin...@playstation.sony.com<mailto:paul_robin...@playstation.sony.com>> 
wrote:
That doesn't seem to be the DWARF I'm seeing from Clang (& it'd be surprising 
if we used the typedef (or otherwise non-canonical) name in the class name):

Finally getting back to this…..  Ha.  We don't unwrap the typedefs ("name as it 
is in the source"), while the upstream compiler does.

Yeah, I imagine you'd want to fix that as I expect it would cause you other 
problems, no? (or is there some reason you have this change to the compiler? I 
imagine it'd be hard to have that divergence by accident?)

Providing the template-parameter DIEs is still the correct thing to do per the 
DWARF
spec.

I still don't agree that the DWARF we produce here is incorrect (the DWARF spec 
is pretty loose on "correctness" of DWARF). If there's some practical 
problem/use case it'd be useful to understand it so 

Re: [PATCH] D14358: DWARF's forward decl of a template should have template parameters.

[David Blaikie via cfe-commits]

On Thu, Nov 5, 2015 at 11:05 AM, Robinson, Paul <
paul_robin...@playstation.sony.com> wrote:

> | What was your primary motivation?
>
> A similar concern to PR20455 from our own debugger.  It much helps
> matching up the forward declaration and definition to have the parameters
> properly specified.
>

Why is matching by name insufficient/not correct?


>
>
> | maybe it's possible to remangle the template using just the string name
>
> I have no idea what you're talking about here.
>

Looking at PR20455 you linked, LLDB isn't finding the right function
because of mangling:

call to a function 'basic_string<char, char_traits
>::operator[](int) const'
('_ZNK12basic_stringIc17char_traitsEixEi') that is not present
in the target

It hasn't created the correct mangled name of operator[] - what I was
saying is it might be possible to parse the template parameter from the
pretty name, and use that to produce the mangled name. It /looks/ like GDB
can manage this. Maybe only because we also include the mangled name of the
member function? Not sure.


> | | Choosing to emit a forward/incomplete declaration in the first place
> fails source fidelity,
>
> | How so?
>
> When the source has a full definition but Clang chooses to emit only the
> declaration, per CGDebugInfo.cpp/shouldOmitDefinition().
>

Sure, in the same way that including unreferenced entities fails source
fidelity - all tradeoffs to reduce debug info size.

Though the behavior is visible in a simpler example that doesn't have that
failing (& if your change goes in, the test case should probably be
simplified like this):

template struct foo;
foo *f;


> --paulr
>
>
>
> *From:* David Blaikie [mailto:dblai...@gmail.com]
> *Sent:* Thursday, November 05, 2015 12:10 AM
> *To:* Robinson, Paul
> *Cc:* reviews+d14358+public+d3104135076f0...@reviews.llvm.org;
> cfe-commits (cfe-commits@lists.llvm.org)
>
> *Subject:* Re: [PATCH] D14358: DWARF's forward decl of a template should
> have template parameters.
>
>
>
>
>
>
>
> On Wed, Nov 4, 2015 at 11:32 PM, Robinson, Paul <
> paul_robin...@playstation.sony.com> wrote:
>
> Would citing PR20455 help?  It wasn't actually my primary motivation but
> it's not too far off.  Having the template parameters there lets you know
> what's going on in the DWARF, without having to fetch and parse the name
> string of every struct you come across.  Actually I'm not sure parsing the
> name string is unambiguous either; each parameter is either a typename, or
> an expression, but without the parameter DIEs you don't know which,
> a-priori.  (What does  mean? Depends on whether you think it should be
> a type name or a value; you can't tell, syntactically, you have to do some
> lookups.  Ah, but if you had the parameter DIEs, you would Just Know.)
>
>
>
> For LLDB's needs, I'm not sure it's sufficient either - but I wouldn't
> mind an answer before we use it as the basis for this change (it sounds
> like maybe it's possible to remangle the template using just the string
> name, rather than needing an explicit representation of the parameters)
>
> What was your primary motivation?
>
>
>
>  Choosing to emit a forward/incomplete declaration in the first place
> fails source fidelity,
>
>
>
> How so? You might have only a template declaration (template
> struct foo; foo *f;) or you may've only instantiated the declaration
> (the C++ language requires you to instantiate or avoid instantiating
> certain things in certain places, so in some contexts you /only/ have an
> instantiated declaration, not a definition)
>
>
>
> but it is a practical engineering tradeoff of compile/link performance
> against utility; and, after all, the source *could* have been written that
> way, with no semantic difference.  But, if we're going to emit a white-lie
> incomplete declaration, we should do so correctly.
>
>
>
> Again, "correct" in DWARF is a fairly nebulous concept.
>
>
>
> --paulr
>
>
>
> P.S. We should talk about this forward-declaration tactic wrt LTO
> sometime.  I have a case where a nested class got forward-declared; it's
> entirely conceivable that the outer class with the inner forward-declared
> class would end up being picked by LTO, leaving the user with no debug info
> for the inner class contents.
>
>
>
> I believe this Just Works(tm). The things that can vary per-insntance of a
> type (implicit special members, member template implicit specializations,
> and nested types*) are not added to the type's child list, but they
> reference the child as their parent. So they continue to apply no matter
> which instance of the type is picked for uniquing (because of the
> name-based referencing, so the 

Re: [PATCH] D14358: DWARF's forward decl of a template should have template parameters.

[David Blaikie via cfe-commits]

On Mon, Nov 9, 2015 at 5:08 PM, Robinson, Paul <
paul_robin...@playstation.sony.com> wrote:

> | when/where/why are types acquired from the mangled names of ELF
> symbols, rather than from corresponding DWARF?
>
>
>
> Pete, can you help me out here?  David seems to want an ironclad case for
> not being able to do something any other way, before he will let me put the
> template type parameters on the declaration of a template instantiation.
> (He does not deny that doing so would be valid DWARF, only that it can't
> possibly be *useful* DWARF.)
>

I'm not arguing that it can't possibly be useful - but there's lots of
things that could be useful that we don't put in the output. It doesn't
have to be iron clad either, but I think a reasonable idea of what this is
useful for doesn't seem too far fetched. (I've removed things like friend
declarations from the DWARF because they saved a couple of % even though we
could think of things they might be useful for, but no one is using them
right now & it seems unlikely anyone will in the near future, for example)

- Dave


> Thanks,
>
> --paulr
>
>
>
> *From:* David Blaikie [mailto:dblai...@gmail.com]
> *Sent:* Monday, November 09, 2015 4:08 PM
>
> *To:* Robinson, Paul
> *Cc:* reviews+d14358+public+d3104135076f0...@reviews.llvm.org;
> cfe-commits (cfe-commits@lists.llvm.org)
> *Subject:* Re: [PATCH] D14358: DWARF's forward decl of a template should
> have template parameters.
>
>
>
>
>
>
>
> On Mon, Nov 9, 2015 at 3:55 PM, Robinson, Paul <
> paul_robin...@playstation.sony.com> wrote:
>
> | Why is matching by name insufficient/not correct?
>
> I'm told we look at the mangled names in the ELF symbol table, demangle
> them, and look in the DWARF for the corresponding types.
>
>
>
> Not quite sure I follow that - when/where/why are types acquired from the
> mangled names of ELF symbols, rather than from corresponding DWARF? (eg:
> the DWARF describing the type of a function's parameter?)
>
>
>
>   Now, the mangled name (for predefined types in particular) provides a
> type description, not the name-as-emitted-by-Clang, and in fact the same
> type can have more than one name ('const int' and 'int const' for a trivial
> example).  The name Clang provides in the DWARF does not necessarily match
> the name produced by the demangler; this makes name-matching way more
> trouble than you'd think.  We're not interested in teaching the debugger
> how to parse template instantiation names.
>
> Having the template type parameter means we have an unambiguous
> description of the type, and can match it easily.
>
>
>
> | including unreferenced entities fails source fidelity
>
> I'll assume you meant to say _excluding_ unreferenced entities fails
> source fidelity,
>
>
>
> Indeed
>
>
>
> which is quite true, but there is a valid engineering tradeoff in that
> what the DWARF actually contains (or not, in the case of, say, unused
> function declarations or unreferenced class contents) represents one
> possible valid source that could have produced the same object.  (I'm
> curious why an unreferenced formal parameter of a function still gets
> described, if this is your argument for omitting template parameters.)
>
>
>
> Omitting parameters would make the function description unusable for
> callers, for example - so there's some value in describing them so that a
> debugger can evaluate expressions involving calls to the function, yes?
>
>
>
> Omitting template parameters however is not the same as omitting
> unreferenced entities, because the template parameters *are* referenced—by
> the template instantiation itself;
>
>
>
> Not quite sure I follow that logic. It's quite possible to have
> unreferenced template parameters:
>
>   template
>   void f() { }
>
>
>
> and, omitting them from the source does not produce a valid program.
>
>
> Omitting the names still produces a valid program - though I'm not quite
> sure which omission you're referring to. (& even if we omit the names, we
> still describe the parameters - as we do for unused/unnamed function
> parameters)
>
>
>   Now, one of the 3 debuggers Clang explicitly supports (i.e. gdb) seems
> not to mind that they're missing, but the other two would benefit from
> having these things, and I would really like to have Clang produce these
> things.
>
>
>
> It sounds like the LLDB bug you cited is being treated as an LLDB bug, not
> a Clang one, for now. So I'm not sure it's relevant to justifying Clang
> changes just yet, unless they come back & suggest that they don't actually
> have enough information to implement the features they would like to
> i

RE: [PATCH] D14358: DWARF's forward decl of a template should have template parameters.

[Robinson, Paul via cfe-commits]

| Why is matching by name insufficient/not correct?
I'm told we look at the mangled names in the ELF symbol table, demangle them, 
and look in the DWARF for the corresponding types.  Now, the mangled name (for 
predefined types in particular) provides a type description, not the 
name-as-emitted-by-Clang, and in fact the same type can have more than one name 
('const int' and 'int const' for a trivial example).  The name Clang provides 
in the DWARF does not necessarily match the name produced by the demangler; 
this makes name-matching way more trouble than you'd think.  We're not 
interested in teaching the debugger how to parse template instantiation names.
Having the template type parameter means we have an unambiguous description of 
the type, and can match it easily.

| including unreferenced entities fails source fidelity
I'll assume you meant to say _excluding_ unreferenced entities fails source 
fidelity, which is quite true, but there is a valid engineering tradeoff in 
that what the DWARF actually contains (or not, in the case of, say, unused 
function declarations or unreferenced class contents) represents one possible 
valid source that could have produced the same object.  (I'm curious why an 
unreferenced formal parameter of a function still gets described, if this is 
your argument for omitting template parameters.)
Omitting template parameters however is not the same as omitting unreferenced 
entities, because the template parameters *are* referenced—by the template 
instantiation itself; and, omitting them from the source does not produce a 
valid program.  Now, one of the 3 debuggers Clang explicitly supports (i.e. 
gdb) seems not to mind that they're missing, but the other two would benefit 
from having these things, and I would really like to have Clang produce these 
things.

Thanks,
--paulr

From: David Blaikie [mailto:dblai...@gmail.com]
Sent: Monday, November 09, 2015 1:46 PM
To: Robinson, Paul
Cc: reviews+d14358+public+d3104135076f0...@reviews.llvm.org; cfe-commits 
(cfe-commits@lists.llvm.org)
Subject: Re: [PATCH] D14358: DWARF's forward decl of a template should have 
template parameters.



On Thu, Nov 5, 2015 at 11:05 AM, Robinson, Paul 
<paul_robin...@playstation.sony.com<mailto:paul_robin...@playstation.sony.com>> 
wrote:
| What was your primary motivation?
A similar concern to PR20455 from our own debugger.  It much helps matching up 
the forward declaration and definition to have the parameters properly 
specified.

Why is matching by name insufficient/not correct?


| maybe it's possible to remangle the template using just the string name
I have no idea what you're talking about here.

Looking at PR20455 you linked, LLDB isn't finding the right function because of 
mangling:


call to a function 'basic_string<char, char_traits >::operator[](int) 
const' ('_ZNK12basic_stringIc17char_traitsEixEi') that is not present in 
the target
It hasn't created the correct mangled name of operator[] - what I was saying is 
it might be possible to parse the template parameter from the pretty name, and 
use that to produce the mangled name. It /looks/ like GDB can manage this. 
Maybe only because we also include the mangled name of the member function? Not 
sure.

| | Choosing to emit a forward/incomplete declaration in the first place fails 
source fidelity,
| How so?
When the source has a full definition but Clang chooses to emit only the 
declaration, per CGDebugInfo.cpp/shouldOmitDefinition().

Sure, in the same way that including unreferenced entities fails source 
fidelity - all tradeoffs to reduce debug info size.

Though the behavior is visible in a simpler example that doesn't have that 
failing (& if your change goes in, the test case should probably be simplified 
like this):

template struct foo;
foo *f;

--paulr

From: David Blaikie [mailto:dblai...@gmail.com<mailto:dblai...@gmail.com>]
Sent: Thursday, November 05, 2015 12:10 AM
To: Robinson, Paul
Cc: 
reviews+d14358+public+d3104135076f0...@reviews.llvm.org<mailto:reviews%2bd14358%2bpublic%2bd3104135076f0...@reviews.llvm.org>;
 cfe-commits (cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>)

Subject: Re: [PATCH] D14358: DWARF's forward decl of a template should have 
template parameters.



On Wed, Nov 4, 2015 at 11:32 PM, Robinson, Paul 
<paul_robin...@playstation.sony.com<mailto:paul_robin...@playstation.sony.com>> 
wrote:
Would citing PR20455 help?  It wasn't actually my primary motivation but it's 
not too far off.  Having the template parameters there lets you know what's 
going on in the DWARF, without having to fetch and parse the name string of 
every struct you come across.  Actually I'm not sure parsing the name string is 
unambiguous either; each parameter is either a typename, or an expression, but 
without the parameter DIEs you don't know which, a-priori.  (What does  
mean? Depends on whether you think it should be a type name or a value; you 
ca

Re: [PATCH] D14358: DWARF's forward decl of a template should have template parameters.

[David Blaikie via cfe-commits]

On Mon, Nov 9, 2015 at 3:55 PM, Robinson, Paul <
paul_robin...@playstation.sony.com> wrote:

> | Why is matching by name insufficient/not correct?
>
> I'm told we look at the mangled names in the ELF symbol table, demangle
> them, and look in the DWARF for the corresponding types.
>

Not quite sure I follow that - when/where/why are types acquired from the
mangled names of ELF symbols, rather than from corresponding DWARF? (eg:
the DWARF describing the type of a function's parameter?)


>   Now, the mangled name (for predefined types in particular) provides a
> type description, not the name-as-emitted-by-Clang, and in fact the same
> type can have more than one name ('const int' and 'int const' for a trivial
> example).  The name Clang provides in the DWARF does not necessarily match
> the name produced by the demangler; this makes name-matching way more
> trouble than you'd think.  We're not interested in teaching the debugger
> how to parse template instantiation names.
>
> Having the template type parameter means we have an unambiguous
> description of the type, and can match it easily.
>
>
>
> | including unreferenced entities fails source fidelity
>
> I'll assume you meant to say _excluding_ unreferenced entities fails
> source fidelity,
>

Indeed


> which is quite true, but there is a valid engineering tradeoff in that
> what the DWARF actually contains (or not, in the case of, say, unused
> function declarations or unreferenced class contents) represents one
> possible valid source that could have produced the same object.  (I'm
> curious why an unreferenced formal parameter of a function still gets
> described, if this is your argument for omitting template parameters.)
>

Omitting parameters would make the function description unusable for
callers, for example - so there's some value in describing them so that a
debugger can evaluate expressions involving calls to the function, yes?


> Omitting template parameters however is not the same as omitting
> unreferenced entities, because the template parameters *are* referenced—by
> the template instantiation itself;
>

Not quite sure I follow that logic. It's quite possible to have
unreferenced template parameters:

  template
  void f() { }


> and, omitting them from the source does not produce a valid program.
>

Omitting the names still produces a valid program - though I'm not quite
sure which omission you're referring to. (& even if we omit the names, we
still describe the parameters - as we do for unused/unnamed function
parameters)


>   Now, one of the 3 debuggers Clang explicitly supports (i.e. gdb) seems
> not to mind that they're missing, but the other two would benefit from
> having these things, and I would really like to have Clang produce these
> things.
>

It sounds like the LLDB bug you cited is being treated as an LLDB bug, not
a Clang one, for now. So I'm not sure it's relevant to justifying Clang
changes just yet, unless they come back & suggest that they don't actually
have enough information to implement the features they would like to
implement.

& equally I'd like to understand the features that you'd like to build with
this info that can't be built without it (as a minimum: features that GDB
doesn't support, since any features GDB does support seem to be
implementable with the current info Clang and GCC emit)

- David


>
>
> Thanks,
>
> --paulr
>
>
>
> *From:* David Blaikie [mailto:dblai...@gmail.com]
> *Sent:* Monday, November 09, 2015 1:46 PM
>
> *To:* Robinson, Paul
> *Cc:* reviews+d14358+public+d3104135076f0...@reviews.llvm.org;
> cfe-commits (cfe-commits@lists.llvm.org)
> *Subject:* Re: [PATCH] D14358: DWARF's forward decl of a template should
> have template parameters.
>
>
>
>
>
>
>
> On Thu, Nov 5, 2015 at 11:05 AM, Robinson, Paul <
> paul_robin...@playstation.sony.com> wrote:
>
> | What was your primary motivation?
>
> A similar concern to PR20455 from our own debugger.  It much helps
> matching up the forward declaration and definition to have the parameters
> properly specified.
>
>
>
> Why is matching by name insufficient/not correct?
>
>
>
>
>
> | maybe it's possible to remangle the template using just the string name
>
> I have no idea what you're talking about here.
>
>
>
> Looking at PR20455 you linked, LLDB isn't finding the right function
> because of mangling:
>
> call to a function 'basic_string<char, char_traits >::operator[](int) 
> const' ('_ZNK12basic_stringIc17char_traitsEixEi') that is not present 
> in the target
>
> It hasn't created the correct mangled name of operator[] - what I was
> saying is it might be possible to parse the template parameter from the
> pretty name, and use

RE: [PATCH] D14358: DWARF's forward decl of a template should have template parameters.

[Robinson, Paul via cfe-commits]

| when/where/why are types acquired from the mangled names of ELF symbols, 
rather than from corresponding DWARF?

Pete, can you help me out here?  David seems to want an ironclad case for not 
being able to do something any other way, before he will let me put the 
template type parameters on the declaration of a template instantiation.  (He 
does not deny that doing so would be valid DWARF, only that it can't possibly 
be *useful* DWARF.)
Thanks,
--paulr

From: David Blaikie [mailto:dblai...@gmail.com]
Sent: Monday, November 09, 2015 4:08 PM
To: Robinson, Paul
Cc: reviews+d14358+public+d3104135076f0...@reviews.llvm.org; cfe-commits 
(cfe-commits@lists.llvm.org)
Subject: Re: [PATCH] D14358: DWARF's forward decl of a template should have 
template parameters.



On Mon, Nov 9, 2015 at 3:55 PM, Robinson, Paul 
<paul_robin...@playstation.sony.com<mailto:paul_robin...@playstation.sony.com>> 
wrote:
| Why is matching by name insufficient/not correct?
I'm told we look at the mangled names in the ELF symbol table, demangle them, 
and look in the DWARF for the corresponding types.

Not quite sure I follow that - when/where/why are types acquired from the 
mangled names of ELF symbols, rather than from corresponding DWARF? (eg: the 
DWARF describing the type of a function's parameter?)

  Now, the mangled name (for predefined types in particular) provides a type 
description, not the name-as-emitted-by-Clang, and in fact the same type can 
have more than one name ('const int' and 'int const' for a trivial example).  
The name Clang provides in the DWARF does not necessarily match the name 
produced by the demangler; this makes name-matching way more trouble than you'd 
think.  We're not interested in teaching the debugger how to parse template 
instantiation names.
Having the template type parameter means we have an unambiguous description of 
the type, and can match it easily.

| including unreferenced entities fails source fidelity
I'll assume you meant to say _excluding_ unreferenced entities fails source 
fidelity,

Indeed

which is quite true, but there is a valid engineering tradeoff in that what the 
DWARF actually contains (or not, in the case of, say, unused function 
declarations or unreferenced class contents) represents one possible valid 
source that could have produced the same object.  (I'm curious why an 
unreferenced formal parameter of a function still gets described, if this is 
your argument for omitting template parameters.)

Omitting parameters would make the function description unusable for callers, 
for example - so there's some value in describing them so that a debugger can 
evaluate expressions involving calls to the function, yes?

Omitting template parameters however is not the same as omitting unreferenced 
entities, because the template parameters *are* referenced—by the template 
instantiation itself;

Not quite sure I follow that logic. It's quite possible to have unreferenced 
template parameters:

  template
  void f() { }

and, omitting them from the source does not produce a valid program.

Omitting the names still produces a valid program - though I'm not quite sure 
which omission you're referring to. (& even if we omit the names, we still 
describe the parameters - as we do for unused/unnamed function parameters)

  Now, one of the 3 debuggers Clang explicitly supports (i.e. gdb) seems not to 
mind that they're missing, but the other two would benefit from having these 
things, and I would really like to have Clang produce these things.

It sounds like the LLDB bug you cited is being treated as an LLDB bug, not a 
Clang one, for now. So I'm not sure it's relevant to justifying Clang changes 
just yet, unless they come back & suggest that they don't actually have enough 
information to implement the features they would like to implement.

& equally I'd like to understand the features that you'd like to build with 
this info that can't be built without it (as a minimum: features that GDB 
doesn't support, since any features GDB does support seem to be implementable 
with the current info Clang and GCC emit)

- David


Thanks,
--paulr

From: David Blaikie [mailto:dblai...@gmail.com<mailto:dblai...@gmail.com>]
Sent: Monday, November 09, 2015 1:46 PM

To: Robinson, Paul
Cc: 
reviews+d14358+public+d3104135076f0...@reviews.llvm.org<mailto:reviews%2bd14358%2bpublic%2bd3104135076f0...@reviews.llvm.org>;
 cfe-commits (cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>)
Subject: Re: [PATCH] D14358: DWARF's forward decl of a template should have 
template parameters.



On Thu, Nov 5, 2015 at 11:05 AM, Robinson, Paul 
<paul_robin...@playstation.sony.com<mailto:paul_robin...@playstation.sony.com>> 
wrote:
| What was your primary motivation?
A similar concern to PR20455 from our own debugger.  It much helps matching up 
the forward declaration and definition to have the parameters properly 
specified.

Why is matching by 

Re: [PATCH] D14358: DWARF's forward decl of a template should have template parameters.

[David Blaikie via cfe-commits]

On Wed, Nov 4, 2015 at 11:32 PM, Robinson, Paul <
paul_robin...@playstation.sony.com> wrote:

> Would citing PR20455 help?  It wasn't actually my primary motivation but
> it's not too far off.  Having the template parameters there lets you know
> what's going on in the DWARF, without having to fetch and parse the name
> string of every struct you come across.  Actually I'm not sure parsing the
> name string is unambiguous either; each parameter is either a typename, or
> an expression, but without the parameter DIEs you don't know which,
> a-priori.  (What does  mean? Depends on whether you think it should be
> a type name or a value; you can't tell, syntactically, you have to do some
> lookups.  Ah, but if you had the parameter DIEs, you would Just Know.)
>

For LLDB's needs, I'm not sure it's sufficient either - but I wouldn't mind
an answer before we use it as the basis for this change (it sounds like
maybe it's possible to remangle the template using just the string name,
rather than needing an explicit representation of the parameters)

What was your primary motivation?


>  Choosing to emit a forward/incomplete declaration in the first place
> fails source fidelity,
>

How so? You might have only a template declaration (template
struct foo; foo *f;) or you may've only instantiated the declaration
(the C++ language requires you to instantiate or avoid instantiating
certain things in certain places, so in some contexts you /only/ have an
instantiated declaration, not a definition)


> but it is a practical engineering tradeoff of compile/link performance
> against utility; and, after all, the source *could* have been written that
> way, with no semantic difference.  But, if we're going to emit a white-lie
> incomplete declaration, we should do so correctly.
>

Again, "correct" in DWARF is a fairly nebulous concept.


> --paulr
>
>
>
> P.S. We should talk about this forward-declaration tactic wrt LTO
> sometime.  I have a case where a nested class got forward-declared; it's
> entirely conceivable that the outer class with the inner forward-declared
> class would end up being picked by LTO, leaving the user with no debug info
> for the inner class contents.
>

I believe this Just Works(tm). The things that can vary per-insntance of a
type (implicit special members, member template implicit specializations,
and nested types*) are not added to the type's child list, but they
reference the child as their parent. So they continue to apply no matter
which instance of the type is picked for uniquing (because of the
name-based referencing, so the nested type definition just says "my parent
is _Zfoo" and whatever _Zfoo we end up picking in the LTO linking/metadata
deduplication will serve that role just fine)

* we could just do a better job of modelling nested types (& other
non-globally scoped types) in a way that more closely models the source by
emitting a declaration where they were declared, and a definition where
they are defined (with the usual DW_AT_specification to wire them up)


>
>
> *From:* David Blaikie [mailto:dblai...@gmail.com]
> *Sent:* Wednesday, November 04, 2015 8:30 PM
> *To:* reviews+d14358+public+d3104135076f0...@reviews.llvm.org; Robinson,
> Paul
> *Subject:* Re: [PATCH] D14358: DWARF's forward decl of a template should
> have template parameters.
>
>
>
>
>
>
>
> On Wed, Nov 4, 2015 at 5:53 PM, Paul Robinson via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
> probinson added a comment.
>
> GCC 4.8.4 on Linux doesn't produce these, but DWARF 4 section 5.5.8 says a
> class template instantiation is just like the equivalent non-template class
> entry, with the exception of the template parameter entries.  I read that
> as meaning an incomplete description (i.e. with DW_AT_declaration) lets you
> omit all the other children, but not the template parameters.
>
>
>
> As usual, I think it's pretty hard to argue that DWARF /requires/ anything
> (permissive & all that). And I'm not sure that having these is particularly
> valuable/useful - what use do you have in mind for them?
>
> Wouldn't hurt to have some size info about the cost here, though I don't
> imagine it's massive, it does open us up to emitting a whole slew of new
> types (the types the template is instantiated with, and anything that
> depends on - breaking/avoiding type edges can, in my experience, be quite
> beneficial (I described an example of this in my lightning talk last week)).
>
>
>
>
> I don't think omitting the template DIEs was an intentional optimization,
> in the sense of being a decision separate from deciding to emit the
> incomplete/forward declaration in the first place.  They were just omitted
> because we were omitting everything, 

RE: [PATCH] D14358: DWARF's forward decl of a template should have template parameters.

[Robinson, Paul via cfe-commits]

| What was your primary motivation?
A similar concern to PR20455 from our own debugger.  It much helps matching up 
the forward declaration and definition to have the parameters properly 
specified.

| maybe it's possible to remangle the template using just the string name
I have no idea what you're talking about here.
| | Choosing to emit a forward/incomplete declaration in the first place fails 
source fidelity,
| How so?
When the source has a full definition but Clang chooses to emit only the 
declaration, per CGDebugInfo.cpp/shouldOmitDefinition().
--paulr

From: David Blaikie [mailto:dblai...@gmail.com]
Sent: Thursday, November 05, 2015 12:10 AM
To: Robinson, Paul
Cc: reviews+d14358+public+d3104135076f0...@reviews.llvm.org; cfe-commits 
(cfe-commits@lists.llvm.org)
Subject: Re: [PATCH] D14358: DWARF's forward decl of a template should have 
template parameters.



On Wed, Nov 4, 2015 at 11:32 PM, Robinson, Paul 
<paul_robin...@playstation.sony.com<mailto:paul_robin...@playstation.sony.com>> 
wrote:
Would citing PR20455 help?  It wasn't actually my primary motivation but it's 
not too far off.  Having the template parameters there lets you know what's 
going on in the DWARF, without having to fetch and parse the name string of 
every struct you come across.  Actually I'm not sure parsing the name string is 
unambiguous either; each parameter is either a typename, or an expression, but 
without the parameter DIEs you don't know which, a-priori.  (What does  
mean? Depends on whether you think it should be a type name or a value; you 
can't tell, syntactically, you have to do some lookups.  Ah, but if you had the 
parameter DIEs, you would Just Know.)

For LLDB's needs, I'm not sure it's sufficient either - but I wouldn't mind an 
answer before we use it as the basis for this change (it sounds like maybe it's 
possible to remangle the template using just the string name, rather than 
needing an explicit representation of the parameters)

What was your primary motivation?

 Choosing to emit a forward/incomplete declaration in the first place fails 
source fidelity,

How so? You might have only a template declaration (template struct 
foo; foo *f;) or you may've only instantiated the declaration (the C++ 
language requires you to instantiate or avoid instantiating certain things in 
certain places, so in some contexts you /only/ have an instantiated 
declaration, not a definition)

but it is a practical engineering tradeoff of compile/link performance against 
utility; and, after all, the source *could* have been written that way, with no 
semantic difference.  But, if we're going to emit a white-lie incomplete 
declaration, we should do so correctly.

Again, "correct" in DWARF is a fairly nebulous concept.

--paulr

P.S. We should talk about this forward-declaration tactic wrt LTO sometime.  I 
have a case where a nested class got forward-declared; it's entirely 
conceivable that the outer class with the inner forward-declared class would 
end up being picked by LTO, leaving the user with no debug info for the inner 
class contents.

I believe this Just Works(tm). The things that can vary per-insntance of a type 
(implicit special members, member template implicit specializations, and nested 
types*) are not added to the type's child list, but they reference the child as 
their parent. So they continue to apply no matter which instance of the type is 
picked for uniquing (because of the name-based referencing, so the nested type 
definition just says "my parent is _Zfoo" and whatever _Zfoo we end up picking 
in the LTO linking/metadata deduplication will serve that role just fine)

* we could just do a better job of modelling nested types (& other non-globally 
scoped types) in a way that more closely models the source by emitting a 
declaration where they were declared, and a definition where they are defined 
(with the usual DW_AT_specification to wire them up)


From: David Blaikie [mailto:dblai...@gmail.com<mailto:dblai...@gmail.com>]
Sent: Wednesday, November 04, 2015 8:30 PM
To: 
reviews+d14358+public+d3104135076f0...@reviews.llvm.org<mailto:reviews%2bd14358%2bpublic%2bd3104135076f0...@reviews.llvm.org>;
 Robinson, Paul
Subject: Re: [PATCH] D14358: DWARF's forward decl of a template should have 
template parameters.



On Wed, Nov 4, 2015 at 5:53 PM, Paul Robinson via cfe-commits 
<cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>> wrote:
probinson added a comment.

GCC 4.8.4 on Linux doesn't produce these, but DWARF 4 section 5.5.8 says a 
class template instantiation is just like the equivalent non-template class 
entry, with the exception of the template parameter entries.  I read that as 
meaning an incomplete description (i.e. with DW_AT_declaration) lets you omit 
all the other children, but not the template parameters.

As usual, I think it's pretty hard to argue that DWARF /requires/ anything 
(permissive & all tha

Re: [PATCH] D14358: DWARF's forward decl of a template should have template parameters.

[Paul Robinson via cfe-commits]

probinson added a comment.

GCC 4.8.4 on Linux doesn't produce these, but DWARF 4 section 5.5.8 says a 
class template instantiation is just like the equivalent non-template class 
entry, with the exception of the template parameter entries.  I read that as 
meaning an incomplete description (i.e. with DW_AT_declaration) lets you omit 
all the other children, but not the template parameters.

I don't think omitting the template DIEs was an intentional optimization, in 
the sense of being a decision separate from deciding to emit the 
incomplete/forward declaration in the first place.  They were just omitted 
because we were omitting everything, but everything turns out to be 
non-compliant.


http://reviews.llvm.org/D14358



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D14358: DWARF's forward decl of a template should have template parameters.

[Adrian Prantl via cfe-commits]

aprantl added a comment.

Unless this is some kind of optimization that we specifically added to minimize 
debug info size (I have never looked at our template support in detail), this 
looks totally reasonable. Do other compilers do the same here?


http://reviews.llvm.org/D14358



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D14358: DWARF's forward decl of a template should have template parameters.

[Paul Robinson via cfe-commits]

probinson added a comment.

In debug-info-template-member.cpp, some things came out in a different order; 
that's the only change there.


http://reviews.llvm.org/D14358



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

RE: [PATCH] D14358: DWARF's forward decl of a template should have template parameters.

[Robinson, Paul via cfe-commits]

Would citing PR20455 help?  It wasn't actually my primary motivation but it's 
not too far off.  Having the template parameters there lets you know what's 
going on in the DWARF, without having to fetch and parse the name string of 
every struct you come across.  Actually I'm not sure parsing the name string is 
unambiguous either; each parameter is either a typename, or an expression, but 
without the parameter DIEs you don't know which, a-priori.  (What does  
mean? Depends on whether you think it should be a type name or a value; you 
can't tell, syntactically, you have to do some lookups.  Ah, but if you had the 
parameter DIEs, you would Just Know.)

Choosing to emit a forward/incomplete declaration in the first place fails 
source fidelity, but it is a practical engineering tradeoff of compile/link 
performance against utility; and, after all, the source *could* have been 
written that way, with no semantic difference.  But, if we're going to emit a 
white-lie incomplete declaration, we should do so correctly.
--paulr

P.S. We should talk about this forward-declaration tactic wrt LTO sometime.  I 
have a case where a nested class got forward-declared; it's entirely 
conceivable that the outer class with the inner forward-declared class would 
end up being picked by LTO, leaving the user with no debug info for the inner 
class contents.

From: David Blaikie [mailto:dblai...@gmail.com]
Sent: Wednesday, November 04, 2015 8:30 PM
To: reviews+d14358+public+d3104135076f0...@reviews.llvm.org; Robinson, Paul
Subject: Re: [PATCH] D14358: DWARF's forward decl of a template should have 
template parameters.



On Wed, Nov 4, 2015 at 5:53 PM, Paul Robinson via cfe-commits 
<cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>> wrote:
probinson added a comment.

GCC 4.8.4 on Linux doesn't produce these, but DWARF 4 section 5.5.8 says a 
class template instantiation is just like the equivalent non-template class 
entry, with the exception of the template parameter entries.  I read that as 
meaning an incomplete description (i.e. with DW_AT_declaration) lets you omit 
all the other children, but not the template parameters.

As usual, I think it's pretty hard to argue that DWARF /requires/ anything 
(permissive & all that). And I'm not sure that having these is particularly 
valuable/useful - what use do you have in mind for them?

Wouldn't hurt to have some size info about the cost here, though I don't 
imagine it's massive, it does open us up to emitting a whole slew of new types 
(the types the template is instantiated with, and anything that depends on - 
breaking/avoiding type edges can, in my experience, be quite beneficial (I 
described an example of this in my lightning talk last week)).


I don't think omitting the template DIEs was an intentional optimization, in 
the sense of being a decision separate from deciding to emit the 
incomplete/forward declaration in the first place.  They were just omitted 
because we were omitting everything, but everything turns out to be 
non-compliant.


http://reviews.llvm.org/D14358



___
cfe-commits mailing list
cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits