[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
Michael137 wrote: > @Michael137 said: > > > Btw, as a follow-up to this patch should we check that this is compatible > > with dsymutil (i.e., running dsymutil --verify)? I suspect it might need a > > fixup (given LLDB doesn't even support this tag) Yup, dsymutil looks good now, thanks > There's a couple of switches in llvm/lib/DWARFLinker that look like they want > a DW_TAG_template_alias: DependencyTracker::isTypeTableCandidate in DependencyTracker.cpp AcceleratorRecordsSaver::save in AcceleratorRecordsSaver.cpp. I'm not sure what a test for those would look like but I can look into it if you'd like? Good question, I'm not familiar with these either, but would make sense to address these eventually. Though I don't think it's urgent atm since `-gtemplate-alias` isn't the default https://github.com/llvm/llvm-project/pull/87623 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
OCHyams wrote: @Michael137 said: > Btw, as a follow-up to this patch should we check that this is compatible > with dsymutil (i.e., running dsymutil --verify)? I suspect it might need a > fixup (given LLDB doesn't even support this tag) `dsymutil --verify` seems to be happy with the tag. Upon closer inspection, I think it uses the same "verifier" code as llvm-dwarfdump so that makes sense (#89589). There's a couple of switches in llvm/lib/DWARFLinker that look like they want a DW_TAG_template_alias: `DependencyTracker::isTypeTableCandidate` in `DependencyTracker.cpp` `AcceleratorRecordsSaver::save` in `AcceleratorRecordsSaver.cpp`. I'm not sure what a test for those would look like but I can look into it if you'd like? https://github.com/llvm/llvm-project/pull/87623 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
OCHyams wrote:
Dependent expressions strike again - https://godbolt.org/z/W381837vr
```
template
using A = int;
template
struct S {
using AA = A;
AA aa;
};
S<0> s;
```
` clang++ -c test.cpp -g -gtemplate-alias`
`clang/lib/AST/ExprConstant.cpp:15721: bool
clang::Expr::EvaluateAsRValue(clang::Expr::EvalResult&, const
clang::ASTContext&, bool) const: Assertion '!isValueDependent() && "Expression
evaluator can't be called on a dependent expression."' failed.`
The issue is `I` is a dependent expression. We have an actual instantiation of
S with the value of `I` clearly being `0` here, but I'm not sure whether that
information exists in the AST for the `TemplateSpecializationType *`.
Looking into this now (will revert if I can't find a solution).
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
https://github.com/OCHyams closed https://github.com/llvm/llvm-project/pull/87623 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
OCHyams wrote: Thanks everyone. I'll land this now then to see what the bots have to say about it. https://github.com/llvm/llvm-project/pull/87623 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
pogo59 wrote: Yes, driver part LGTM. https://github.com/llvm/llvm-project/pull/87623 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -triple x86_64-unk-unk -o - -emit-llvm
-debug-info-kind=standalone -gtemplate-alias %s -gsimple-template-names=simple \
+// RUN: | FileCheck %s
+
+ Check that -gtemplate-alias causes DW_TAG_template_alias emission for
+ template aliases with default parameter values. See template-alias.cpp for
+ more template alias tests.
+ FIXME: We currently do not emit defaulted arguments.
+
+template
+struct X {
+ char m;
+};
+
+template
+struct Y {
+ char n;
+};
+
+template class T = Y, int I = 5,
typename... Ts>
+using A = X;
+
+ We should be able to emit type alias metadata which describes all the
+ values, including the defaulted parameters and empty parameter pack.
+A a;
+
+// CHECK: !DIDerivedType(tag: DW_TAG_template_alias, name: "A", file: ![[#]],
line: [[#]], baseType: ![[baseType:[0-9]+]], extraData: ![[extraData:[0-9]+]])
+// CHECK: ![[baseType]] = distinct !DICompositeType(tag:
DW_TAG_structure_type, name: "X",
+// CHECK: ![[int:[0-9]+]] = !DIBasicType(name: "int", size: 32, encoding:
DW_ATE_signed)
+// CHECK: ![[extraData]] = !{![[NonDefault:[0-9]+]]}
+// CHECK: ![[NonDefault]] = !DITemplateTypeParameter(name: "NonDefault", type:
![[int]])
+
+ FIXME: Ideally, we would describe the deafulted args, like this:
+// : ![[extraData]] = !{![[NonDefault:[0-9]+]], ![[T:[0-9]+]], ![[I:[0-9]+]],
![[Ts:[0-9]+]]}
Michael137 wrote:
ah right, that makes sense, happy keeping it as is
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -triple x86_64-unk-unk -o - -emit-llvm
-debug-info-kind=standalone -gtemplate-alias %s -gsimple-template-names=simple \
+// RUN: | FileCheck %s
+
+ Check that -gtemplate-alias causes DW_TAG_template_alias emission for
+ template aliases with default parameter values. See template-alias.cpp for
+ more template alias tests.
+ FIXME: We currently do not emit defaulted arguments.
+
+template
+struct X {
+ char m;
+};
+
+template
+struct Y {
+ char n;
+};
+
+template class T = Y, int I = 5,
typename... Ts>
+using A = X;
+
+ We should be able to emit type alias metadata which describes all the
+ values, including the defaulted parameters and empty parameter pack.
+A a;
+
+// CHECK: !DIDerivedType(tag: DW_TAG_template_alias, name: "A", file: ![[#]],
line: [[#]], baseType: ![[baseType:[0-9]+]], extraData: ![[extraData:[0-9]+]])
+// CHECK: ![[baseType]] = distinct !DICompositeType(tag:
DW_TAG_structure_type, name: "X",
+// CHECK: ![[int:[0-9]+]] = !DIBasicType(name: "int", size: 32, encoding:
DW_ATE_signed)
+// CHECK: ![[extraData]] = !{![[NonDefault:[0-9]+]]}
+// CHECK: ![[NonDefault]] = !DITemplateTypeParameter(name: "NonDefault", type:
![[int]])
+
+ FIXME: Ideally, we would describe the deafulted args, like this:
+// : ![[extraData]] = !{![[NonDefault:[0-9]+]], ![[T:[0-9]+]], ![[I:[0-9]+]],
![[Ts:[0-9]+]]}
OCHyams wrote:
I think the test will already start failing if this gets fixed because the
current `extraData` metadata tuple contents check will fail (it'll have more
than one element). I'm still happy to add CHECK-NOTs if you'd like, just
thought I'd raise this in case it changes your stance.
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
https://github.com/Michael137 approved this pull request. LGTM, if @pogo59 and @dwblaikie are happy with the driver changes https://github.com/llvm/llvm-project/pull/87623 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -triple x86_64-unk-unk -o - -emit-llvm
-debug-info-kind=standalone -gtemplate-alias %s -gsimple-template-names=simple \
+// RUN: | FileCheck %s
+
+ Check that -gtemplate-alias causes DW_TAG_template_alias emission for
+ template aliases with default parameter values. See template-alias.cpp for
+ more template alias tests.
+ FIXME: We currently do not emit defaulted arguments.
+
+template
+struct X {
+ char m;
+};
+
+template
+struct Y {
+ char n;
+};
+
+template class T = Y, int I = 5,
typename... Ts>
+using A = X;
+
+ We should be able to emit type alias metadata which describes all the
+ values, including the defaulted parameters and empty parameter pack.
+A a;
+
+// CHECK: !DIDerivedType(tag: DW_TAG_template_alias, name: "A", file: ![[#]],
line: [[#]], baseType: ![[baseType:[0-9]+]], extraData: ![[extraData:[0-9]+]])
+// CHECK: ![[baseType]] = distinct !DICompositeType(tag:
DW_TAG_structure_type, name: "X",
+// CHECK: ![[int:[0-9]+]] = !DIBasicType(name: "int", size: 32, encoding:
DW_ATE_signed)
+// CHECK: ![[extraData]] = !{![[NonDefault:[0-9]+]]}
+// CHECK: ![[NonDefault]] = !DITemplateTypeParameter(name: "NonDefault", type:
![[int]])
+
+ FIXME: Ideally, we would describe the deafulted args, like this:
+// : ![[extraData]] = !{![[NonDefault:[0-9]+]], ![[T:[0-9]+]], ![[I:[0-9]+]],
![[Ts:[0-9]+]]}
Michael137 wrote:
Minor: Is it worth turning these into `CHECK-NOT`? So when this is fixed in the
future we remember that we have these tests ready
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -1332,6 +1332,54 @@ llvm::DIType *CGDebugInfo::CreateType(const
TemplateSpecializationType *Ty,
auto PP = getPrintingPolicy();
Ty->getTemplateName().print(OS, PP, TemplateName::Qualified::None);
+ SourceLocation Loc = AliasDecl->getLocation();
+
+ if (CGM.getCodeGenOpts().DebugTemplateAlias) {
+// TemplateSpecializationType doesn't know if its template args are
+// being substituted into a parameter pack. We can find out if that's
+// the case now by inspecting the TypeAliasTemplateDecl template
+// parameters. Insert Ty's template args into SpecArgs, bundling args
+// passed to a parameter pack into a TemplateArgument::Pack.
+SmallVector SpecArgs;
+{
+ ArrayRef SubstArgs = Ty->template_arguments();
+ for (const NamedDecl *P : TD->getTemplateParameters()->asArray()) {
+if (P->isParameterPack()) {
+ SpecArgs.push_back(TemplateArgument(SubstArgs));
+ break;
+}
+// Skip defaulted args.
+if (SubstArgs.empty()) {
+ // If SubstArgs is now empty (we're taking from it each iteration)
and
+ // this template parameter isn't a pack, then that should mean we're
+ // using default values for the remaining template parameters.
+ break;
Michael137 wrote:
Sounds good! FWIW, LLDB would currently omit the defaulted parameters too, by
choice, since usually they're just noise (especially with many of the STL
types).
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
OCHyams wrote: I've rebased this so now this pull request contains only the Clang changes (LLVM side is here #88943). There's a discussion starting [here](https://github.com/llvm/llvm-project/pull/87623#discussion_r1567968127) that discusses whether or not we should include defaulted arguments in the list. The very short summary is that I tried this by scraping the defaults from the template parameter list, but it turns out this is difficult in the face of parameter defaults that are dependent types and values. So the current implementation ignores defaulted arguments, i.e., doesn't include them in the argument list (and as a consequence also omits empty parameter packs that come after defaulted arguments). This is not ideal, but not a regression from the DW_TAG_typedef names which also do not include defaulted arg values. https://github.com/llvm/llvm-project/pull/87623 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -1332,6 +1332,54 @@ llvm::DIType *CGDebugInfo::CreateType(const
TemplateSpecializationType *Ty,
auto PP = getPrintingPolicy();
Ty->getTemplateName().print(OS, PP, TemplateName::Qualified::None);
+ SourceLocation Loc = AliasDecl->getLocation();
+
+ if (CGM.getCodeGenOpts().DebugTemplateAlias) {
+// TemplateSpecializationType doesn't know if its template args are
+// being substituted into a parameter pack. We can find out if that's
+// the case now by inspecting the TypeAliasTemplateDecl template
+// parameters. Insert Ty's template args into SpecArgs, bundling args
+// passed to a parameter pack into a TemplateArgument::Pack.
+SmallVector SpecArgs;
+{
+ ArrayRef SubstArgs = Ty->template_arguments();
+ for (const NamedDecl *P : TD->getTemplateParameters()->asArray()) {
+if (P->isParameterPack()) {
+ SpecArgs.push_back(TemplateArgument(SubstArgs));
+ break;
+}
+// Skip defaulted args.
+if (SubstArgs.empty()) {
+ // If SubstArgs is now empty (we're taking from it each iteration)
and
+ // this template parameter isn't a pack, then that should mean we're
+ // using default values for the remaining template parameters.
+ break;
OCHyams wrote:
>Sorry for diverting you down this path, maybe ignoring the default arguments
>as you did in your first attempt is sufficient if this is just not feasible.
Not at all, I think it was worth exploring and I very much appreciate the
detailed review and the help that's come with it.
I agree that ignoring default arguments might be the way forward then. It isn't
a regression in terms of name reconstruction, since the existing typedef
approach for template aliases omits defaulted arguments from the DW_AT_name
anyway:
```
template
using A = int;
A<> a;
```
`$ clang test.cpp -g -o - | llvm-dwarfdump -o -`
```
[...]
0x002e: DW_TAG_typedef
DW_AT_type (0x0036 "int")
DW_AT_name ("A<>") <-- no defaulted arguments for
template aliases :-(
DW_AT_decl_file ("test.cpp")
DW_AT_decl_line (41)
```
I have pushed that change (to omit defaulted arguments). Thanks for bearing
with me on this one!
I've chatted to our debugger folks and they're ok with omitting defaulted args
(though ideally this would be fixed in the future).
I think moving this discussion onto discourse might be useful - I'll look at
doing that later in the week or next week.
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -1332,6 +1332,54 @@ llvm::DIType *CGDebugInfo::CreateType(const
TemplateSpecializationType *Ty,
auto PP = getPrintingPolicy();
Ty->getTemplateName().print(OS, PP, TemplateName::Qualified::None);
+ SourceLocation Loc = AliasDecl->getLocation();
+
+ if (CGM.getCodeGenOpts().DebugTemplateAlias) {
+// TemplateSpecializationType doesn't know if its template args are
+// being substituted into a parameter pack. We can find out if that's
+// the case now by inspecting the TypeAliasTemplateDecl template
+// parameters. Insert Ty's template args into SpecArgs, bundling args
+// passed to a parameter pack into a TemplateArgument::Pack.
+SmallVector SpecArgs;
+{
+ ArrayRef SubstArgs = Ty->template_arguments();
+ for (const NamedDecl *P : TD->getTemplateParameters()->asArray()) {
+if (P->isParameterPack()) {
+ SpecArgs.push_back(TemplateArgument(SubstArgs));
+ break;
+}
+// Skip defaulted args.
+if (SubstArgs.empty()) {
+ // If SubstArgs is now empty (we're taking from it each iteration)
and
+ // this template parameter isn't a pack, then that should mean we're
+ // using default values for the remaining template parameters.
+ break;
Michael137 wrote:
> The constant value default appears to be working, it's the dependent value
> int I2 = I1 that is causing problems for me.
Aaah I see. Yea for a`TypeAliasTemplateDecl` we don't get the argument
instantiations. Hmmm I don't see a good way of getting the information you need
here. Perhaps @dwblaikie or @AaronBallman have some better ideas on how to
resolve the dependent default arguments here.
Sorry for diverting you down this path, maybe ignoring the default arguments as
you did in your first attempt is sufficient if this is just not feasible.
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -1332,6 +1332,54 @@ llvm::DIType *CGDebugInfo::CreateType(const
TemplateSpecializationType *Ty,
auto PP = getPrintingPolicy();
Ty->getTemplateName().print(OS, PP, TemplateName::Qualified::None);
+ SourceLocation Loc = AliasDecl->getLocation();
+
+ if (CGM.getCodeGenOpts().DebugTemplateAlias) {
+// TemplateSpecializationType doesn't know if its template args are
+// being substituted into a parameter pack. We can find out if that's
+// the case now by inspecting the TypeAliasTemplateDecl template
+// parameters. Insert Ty's template args into SpecArgs, bundling args
+// passed to a parameter pack into a TemplateArgument::Pack.
+SmallVector SpecArgs;
+{
+ ArrayRef SubstArgs = Ty->template_arguments();
+ for (const NamedDecl *P : TD->getTemplateParameters()->asArray()) {
+if (P->isParameterPack()) {
+ SpecArgs.push_back(TemplateArgument(SubstArgs));
+ break;
+}
+// Skip defaulted args.
+if (SubstArgs.empty()) {
+ // If SubstArgs is now empty (we're taking from it each iteration)
and
+ // this template parameter isn't a pack, then that should mean we're
+ // using default values for the remaining template parameters.
+ break;
OCHyams wrote:
> I suppose you could call EvaluateKnownConstInt (on the TemplateArgument
> expression, if it's !isValueDependent() && isIntegerConstantExpr(), similar
> to what templateArgumentExpressionsEqual does). Maybe we can do this from
> within CollectTemplateParams where it asserts that the expression is a
> Constant?
The constant value default appears to be working, it's the dependent value `int
I2 = I1` that is causing problems for me.
> Seems like you're tripping over the assert now because you're calling
> CollectTemplateParams on the arguments to a TemplateAliasDecl which don't
> have their arguments evaluated in the front-end it looks like.
That sounds like it might be the problem. It feels like we have all the parts
needed to instantiate the dependent expression at this point... (because all
the types/values it depends on exist as non-default argument values), I'm just
not sure how to ask Clang to do that.
Here's an example using type parameters rather than values (you can also do
`typename C = X` if you want to hit yet another assertion...)
```
template
using A = int;
A<> a;
```
```
Dependent types cannot show up in debug information
UNREACHABLE executed at clang/lib/CodeGen/CGDebugInfo.cpp:3686!
...
CGDebugInfo::CreateTypeNode(clang::QualType, llvm::DIFile*)
CGDebugInfo::getOrCreateType(clang::QualType, llvm::DIFile*)
CollectTemplateParams # from the TemplateArgument::Type case processing the
type of the argument for C
```
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -1332,6 +1332,54 @@ llvm::DIType *CGDebugInfo::CreateType(const
TemplateSpecializationType *Ty,
auto PP = getPrintingPolicy();
Ty->getTemplateName().print(OS, PP, TemplateName::Qualified::None);
+ SourceLocation Loc = AliasDecl->getLocation();
+
+ if (CGM.getCodeGenOpts().DebugTemplateAlias) {
+// TemplateSpecializationType doesn't know if its template args are
+// being substituted into a parameter pack. We can find out if that's
+// the case now by inspecting the TypeAliasTemplateDecl template
+// parameters. Insert Ty's template args into SpecArgs, bundling args
+// passed to a parameter pack into a TemplateArgument::Pack.
+SmallVector SpecArgs;
+{
+ ArrayRef SubstArgs = Ty->template_arguments();
+ for (const NamedDecl *P : TD->getTemplateParameters()->asArray()) {
+if (P->isParameterPack()) {
+ SpecArgs.push_back(TemplateArgument(SubstArgs));
+ break;
+}
+// Skip defaulted args.
+if (SubstArgs.empty()) {
+ // If SubstArgs is now empty (we're taking from it each iteration)
and
+ // this template parameter isn't a pack, then that should mean we're
+ // using default values for the remaining template parameters.
+ break;
OCHyams wrote:
(defaulted values... and types*)
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -1332,6 +1332,54 @@ llvm::DIType *CGDebugInfo::CreateType(const
TemplateSpecializationType *Ty,
auto PP = getPrintingPolicy();
Ty->getTemplateName().print(OS, PP, TemplateName::Qualified::None);
+ SourceLocation Loc = AliasDecl->getLocation();
+
+ if (CGM.getCodeGenOpts().DebugTemplateAlias) {
+// TemplateSpecializationType doesn't know if its template args are
+// being substituted into a parameter pack. We can find out if that's
+// the case now by inspecting the TypeAliasTemplateDecl template
+// parameters. Insert Ty's template args into SpecArgs, bundling args
+// passed to a parameter pack into a TemplateArgument::Pack.
+SmallVector SpecArgs;
+{
+ ArrayRef SubstArgs = Ty->template_arguments();
+ for (const NamedDecl *P : TD->getTemplateParameters()->asArray()) {
+if (P->isParameterPack()) {
+ SpecArgs.push_back(TemplateArgument(SubstArgs));
+ break;
+}
+// Skip defaulted args.
+if (SubstArgs.empty()) {
+ // If SubstArgs is now empty (we're taking from it each iteration)
and
+ // this template parameter isn't a pack, then that should mean we're
+ // using default values for the remaining template parameters.
+ break;
OCHyams wrote:
> Yup, would default values ever matter for name reconstruction?
I'd have thought not too, which is reflected in my initial approach of just
leaving off defaulted args. However, it turns out our debugger displays
defaulted values (this is intentional & desirable according to the team), and
so does GDB.
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -1332,6 +1332,54 @@ llvm::DIType *CGDebugInfo::CreateType(const
TemplateSpecializationType *Ty,
auto PP = getPrintingPolicy();
Ty->getTemplateName().print(OS, PP, TemplateName::Qualified::None);
+ SourceLocation Loc = AliasDecl->getLocation();
+
+ if (CGM.getCodeGenOpts().DebugTemplateAlias) {
+// TemplateSpecializationType doesn't know if its template args are
+// being substituted into a parameter pack. We can find out if that's
+// the case now by inspecting the TypeAliasTemplateDecl template
+// parameters. Insert Ty's template args into SpecArgs, bundling args
+// passed to a parameter pack into a TemplateArgument::Pack.
+SmallVector SpecArgs;
+{
+ ArrayRef SubstArgs = Ty->template_arguments();
+ for (const NamedDecl *P : TD->getTemplateParameters()->asArray()) {
+if (P->isParameterPack()) {
+ SpecArgs.push_back(TemplateArgument(SubstArgs));
+ break;
+}
+// Skip defaulted args.
+if (SubstArgs.empty()) {
+ // If SubstArgs is now empty (we're taking from it each iteration)
and
+ // this template parameter isn't a pack, then that should mean we're
+ // using default values for the remaining template parameters.
+ break;
Michael137 wrote:
I suppose you could call `EvaluateKnownConstInt` (on the TemplateArgument
expression, if it's `!isValueDependent() && isIntegerConstantExpr()`, similar
to what `templateArgumentExpressionsEqual` does). Maybe we can do this from
within `CollectTemplateParams` where it asserts that the expression is a
Constant?
I'm not actually sure how we get to the `TemplateArgument::Expression` case in
`CollectTemplateParams` for constant expressions. AFAICT in the AST such
expressions get evaluated into `TemplateArgument::Integral` before we ever get
here. Seems like you're tripping over the assert now because you're calling
`CollectTemplateParams` on the arguments to a `TemplateAliasDecl` which don't
have their arguments evaluated in the front-end it looks like.
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/87623 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -1332,6 +1332,54 @@ llvm::DIType *CGDebugInfo::CreateType(const
TemplateSpecializationType *Ty,
auto PP = getPrintingPolicy();
Ty->getTemplateName().print(OS, PP, TemplateName::Qualified::None);
+ SourceLocation Loc = AliasDecl->getLocation();
+
+ if (CGM.getCodeGenOpts().DebugTemplateAlias) {
+// TemplateSpecializationType doesn't know if its template args are
+// being substituted into a parameter pack. We can find out if that's
+// the case now by inspecting the TypeAliasTemplateDecl template
+// parameters. Insert Ty's template args into SpecArgs, bundling args
+// passed to a parameter pack into a TemplateArgument::Pack.
+SmallVector SpecArgs;
+{
+ ArrayRef SubstArgs = Ty->template_arguments();
+ for (const NamedDecl *P : TD->getTemplateParameters()->asArray()) {
+if (P->isParameterPack()) {
+ SpecArgs.push_back(TemplateArgument(SubstArgs));
+ break;
+}
+// Skip defaulted args.
+if (SubstArgs.empty()) {
+ // If SubstArgs is now empty (we're taking from it each iteration)
and
+ // this template parameter isn't a pack, then that should mean we're
+ // using default values for the remaining template parameters.
+ break;
Michael137 wrote:
Ah apologies! I must've misremembered, I could've sworn there were cases where
we don't handle the defaulted-ness.
> Is your suggestion to emit something like this for the template alias case
> (from my previous comment)?
Yup, would default values ever matter for name reconstruction? The only thing I
know of that uses `DW_AT_default_value` in LLDB is when formatting the typename
of a variable when printing it (we omit defaulted template parameters).
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -1332,6 +1332,54 @@ llvm::DIType *CGDebugInfo::CreateType(const
TemplateSpecializationType *Ty,
auto PP = getPrintingPolicy();
Ty->getTemplateName().print(OS, PP, TemplateName::Qualified::None);
+ SourceLocation Loc = AliasDecl->getLocation();
+
+ if (CGM.getCodeGenOpts().DebugTemplateAlias) {
+// TemplateSpecializationType doesn't know if its template args are
+// being substituted into a parameter pack. We can find out if that's
+// the case now by inspecting the TypeAliasTemplateDecl template
+// parameters. Insert Ty's template args into SpecArgs, bundling args
+// passed to a parameter pack into a TemplateArgument::Pack.
+SmallVector SpecArgs;
+{
+ ArrayRef SubstArgs = Ty->template_arguments();
+ for (const NamedDecl *P : TD->getTemplateParameters()->asArray()) {
+if (P->isParameterPack()) {
+ SpecArgs.push_back(TemplateArgument(SubstArgs));
+ break;
+}
+// Skip defaulted args.
+if (SubstArgs.empty()) {
+ // If SubstArgs is now empty (we're taking from it each iteration)
and
+ // this template parameter isn't a pack, then that should mean we're
+ // using default values for the remaining template parameters.
+ break;
OCHyams wrote:
Looking at a class template instantiation with a default dependent value:
```
template
struct B {};
B<> b;
```
We get:
```
0x0029: DW_TAG_structure_type
DW_AT_calling_convention(DW_CC_pass_by_value)
DW_AT_name ("B<5, 5>")
DW_AT_byte_size (0x01)
DW_AT_decl_file ("/home/och/scratch/test11.cpp")
DW_AT_decl_line (2)
0x002f: DW_TAG_template_value_parameter
DW_AT_type(0x003e "int")
DW_AT_name("I1")
DW_AT_default_value (true)
DW_AT_const_value (5)
0x0036: DW_TAG_template_value_parameter
DW_AT_type(0x003e "int")
DW_AT_name("I2")
DW_AT_default_value (true)
DW_AT_const_value (5)
0x003d: NULL
```
Is your suggestion to emit something like this for the template alias case
(from my previous comment)?
```
0x0029: DW_TAG_template_alias
DW_AT_name ("B<5, ?>")
...
0x002f: DW_TAG_template_value_parameter
DW_AT_type(0x003e "int")
DW_AT_name("I1")
DW_AT_default_value (true)
DW_AT_const_value (5)
0x0036: DW_TAG_template_value_parameter
DW_AT_type(0x003e "int")
DW_AT_name("I2")
0x003d: NULL
```
I think that is doable but I'm not sure how that would interact with name
reconstruction (in Clang debug info without -gsimple-template-names, and in
debuggers/tools with it).
tyvm for your ongoing help with this.
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -1332,6 +1332,54 @@ llvm::DIType *CGDebugInfo::CreateType(const
TemplateSpecializationType *Ty,
auto PP = getPrintingPolicy();
Ty->getTemplateName().print(OS, PP, TemplateName::Qualified::None);
+ SourceLocation Loc = AliasDecl->getLocation();
+
+ if (CGM.getCodeGenOpts().DebugTemplateAlias) {
+// TemplateSpecializationType doesn't know if its template args are
+// being substituted into a parameter pack. We can find out if that's
+// the case now by inspecting the TypeAliasTemplateDecl template
+// parameters. Insert Ty's template args into SpecArgs, bundling args
+// passed to a parameter pack into a TemplateArgument::Pack.
+SmallVector SpecArgs;
+{
+ ArrayRef SubstArgs = Ty->template_arguments();
+ for (const NamedDecl *P : TD->getTemplateParameters()->asArray()) {
+if (P->isParameterPack()) {
+ SpecArgs.push_back(TemplateArgument(SubstArgs));
+ break;
+}
+// Skip defaulted args.
+if (SubstArgs.empty()) {
+ // If SubstArgs is now empty (we're taking from it each iteration)
and
+ // this template parameter isn't a pack, then that should mean we're
+ // using default values for the remaining template parameters.
+ break;
Michael137 wrote:
Clang doesn't resolve defaultedness for dependent arguments either. As a
consequence, when a template parameter has a dependent default argument then
its `DW_TAG_template_value_parameter` won't have a `DW_AT_default_value`. So I
don't think we need to handle this if it's going to be hard to do. Can we just
arrange for the parameter to be emitted as a non-defaulted
`DW_TAG_template_value_parameter`?
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -1332,6 +1332,54 @@ llvm::DIType *CGDebugInfo::CreateType(const
TemplateSpecializationType *Ty,
auto PP = getPrintingPolicy();
Ty->getTemplateName().print(OS, PP, TemplateName::Qualified::None);
+ SourceLocation Loc = AliasDecl->getLocation();
+
+ if (CGM.getCodeGenOpts().DebugTemplateAlias) {
+// TemplateSpecializationType doesn't know if its template args are
+// being substituted into a parameter pack. We can find out if that's
+// the case now by inspecting the TypeAliasTemplateDecl template
+// parameters. Insert Ty's template args into SpecArgs, bundling args
+// passed to a parameter pack into a TemplateArgument::Pack.
+SmallVector SpecArgs;
+{
+ ArrayRef SubstArgs = Ty->template_arguments();
+ for (const NamedDecl *P : TD->getTemplateParameters()->asArray()) {
+if (P->isParameterPack()) {
+ SpecArgs.push_back(TemplateArgument(SubstArgs));
+ break;
+}
+// Skip defaulted args.
+if (SubstArgs.empty()) {
+ // If SubstArgs is now empty (we're taking from it each iteration)
and
+ // this template parameter isn't a pack, then that should mean we're
+ // using default values for the remaining template parameters.
+ break;
OCHyams wrote:
> The three kinds you check for should be sufficient afaik. Places around clang
> consistently only deal with these three types of parameter decls.
Excellent, that is reassuring, thanks.
I'm wondering if you've got any ideas about my dependent value situation:
```
template
using B = int;
B<> b;
```
The current default-argument-gathering tactic can't cope with dependent default
expressions/types. Do you know if there's any way that I can "resolve", for
lack of a better word, the template parameter default expression to get an
expression `= 5` rather than the dependent `= I1`? Presumably this is doable,
because we've got a specialisation that defines what `I1` is (are there cases
where this could never work... maybe with SFINAE? I'm not sure).
I'm continuing to look into it, I'm only asking in case you know off the top of
your head, no problem if not.
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -1313,6 +1313,61 @@ llvm::DIType *CGDebugInfo::CreateType(const
BlockPointerType *Ty,
return DBuilder.createPointerType(EltTy, Size);
}
+static llvm::SmallVector
+GetTemplateArgs(const TemplateDecl *TD, const TemplateSpecializationType *Ty) {
+ assert(Ty->isTypeAlias());
+ // TemplateSpecializationType doesn't know if its template args are
+ // being substituted into a parameter pack. We can find out if that's
+ // the case now by inspecting the TypeAliasTemplateDecl template
+ // parameters. Insert Ty's template args into SpecArgs, bundling args
+ // passed to a parameter pack into a TemplateArgument::Pack. It also
+ // doesn't know the value of any defaulted args, so collect those now
+ // too.
+ SmallVector SpecArgs;
+ ArrayRef SubstArgs = Ty->template_arguments();
+ for (const NamedDecl *Param : TD->getTemplateParameters()->asArray()) {
+// If Param is a parameter pack, pack the remaining arguments.
+if (Param->isParameterPack()) {
+ SpecArgs.push_back(TemplateArgument(SubstArgs));
+ break;
+}
+
+// Take the next argument.
+if (!SubstArgs.empty()) {
+ SpecArgs.push_back(SubstArgs.front());
+ SubstArgs = SubstArgs.drop_front();
+ continue;
+}
+
+// If SubstArgs is now empty (we're taking from it each iteration) and
+// this template parameter isn't a pack, then that should mean we're
+// using default values for the remaining template parameters. Push the
+// default value for each parameter.
+if (auto *P = dyn_cast(Param)) {
+ assert(P->hasDefaultArgument() &&
+ "expected defaulted template type parameter");
+ SpecArgs.push_back(TemplateArgument(P->getDefaultArgument(),
+ /*IsNullPtr=*/false,
+ /*IsDefaulted=*/true));
+} else if (auto *P = dyn_cast(Param)) {
+ assert(P->hasDefaultArgument() &&
+ "expected defaulted template non-type parameter");
+ SpecArgs.push_back(TemplateArgument(P->getDefaultArgument(),
+ /*IsDefaulted=*/true));
+} else if (auto *P = dyn_cast(Param)) {
+ assert(P->hasDefaultArgument() &&
+ "expected defaulted template template parameter");
+ SpecArgs.push_back(TemplateArgument(
+ P->getDefaultArgument().getArgument().getAsTemplate(),
+ /*IsDefaulted=*/true));
Michael137 wrote:
Could we get away with just doing
`SpecArgs.push_back(P->getDefaultArgument())`? Or were there some lifetime
concerns?
Otherwise LGTM
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -1332,6 +1332,54 @@ llvm::DIType *CGDebugInfo::CreateType(const
TemplateSpecializationType *Ty,
auto PP = getPrintingPolicy();
Ty->getTemplateName().print(OS, PP, TemplateName::Qualified::None);
+ SourceLocation Loc = AliasDecl->getLocation();
+
+ if (CGM.getCodeGenOpts().DebugTemplateAlias) {
+// TemplateSpecializationType doesn't know if its template args are
+// being substituted into a parameter pack. We can find out if that's
+// the case now by inspecting the TypeAliasTemplateDecl template
+// parameters. Insert Ty's template args into SpecArgs, bundling args
+// passed to a parameter pack into a TemplateArgument::Pack.
+SmallVector SpecArgs;
+{
+ ArrayRef SubstArgs = Ty->template_arguments();
+ for (const NamedDecl *P : TD->getTemplateParameters()->asArray()) {
+if (P->isParameterPack()) {
+ SpecArgs.push_back(TemplateArgument(SubstArgs));
+ break;
+}
+// Skip defaulted args.
+if (SubstArgs.empty()) {
+ // If SubstArgs is now empty (we're taking from it each iteration)
and
+ // this template parameter isn't a pack, then that should mean we're
+ // using default values for the remaining template parameters.
+ break;
Michael137 wrote:
> lightly worried there are other template parameter kinds I've not thought of,
> currently guarded by an llvm_unrecahable. Maybe there's something better to
> do there - do you have any thoughts on this?
The three kinds you check for should be sufficient afaik. Places around clang
consistently only deal with these three types of parameter decls.
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -310,6 +310,24 @@ namespace llvm {
DINode::DIFlags Flags = DINode::FlagZero,
DINodeArray Annotations = nullptr);
+/// Create debugging information entry for a template alias.
+/// \param Ty Original type.
+/// \param NameAlias name.
+/// \param FileFile where this type is defined.
+/// \param LineNo Line number.
+/// \param Context The surrounding context for the alias.
+/// \param TParams The template arguments.
+/// \param AlignInBits Alignment. (optional)
+/// \param Flags Flags to describe inheritance attribute (optional),
+///e.g. private.
pogo59 wrote:
Ah sorry missed that.
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -1332,6 +1332,54 @@ llvm::DIType *CGDebugInfo::CreateType(const
TemplateSpecializationType *Ty,
auto PP = getPrintingPolicy();
Ty->getTemplateName().print(OS, PP, TemplateName::Qualified::None);
+ SourceLocation Loc = AliasDecl->getLocation();
+
+ if (CGM.getCodeGenOpts().DebugTemplateAlias) {
+// TemplateSpecializationType doesn't know if its template args are
+// being substituted into a parameter pack. We can find out if that's
+// the case now by inspecting the TypeAliasTemplateDecl template
+// parameters. Insert Ty's template args into SpecArgs, bundling args
+// passed to a parameter pack into a TemplateArgument::Pack.
+SmallVector SpecArgs;
+{
+ ArrayRef SubstArgs = Ty->template_arguments();
+ for (const NamedDecl *P : TD->getTemplateParameters()->asArray()) {
+if (P->isParameterPack()) {
+ SpecArgs.push_back(TemplateArgument(SubstArgs));
+ break;
+}
+// Skip defaulted args.
+if (SubstArgs.empty()) {
+ // If SubstArgs is now empty (we're taking from it each iteration)
and
+ // this template parameter isn't a pack, then that should mean we're
+ // using default values for the remaining template parameters.
+ break;
OCHyams wrote:
Ah - this implementation appears to cause an assertion when one of the template
parameters is a dependant type... (working on it)
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -triple x86_64-unk-unk -o - -emit-llvm
-debug-info-kind=standalone -gtemplate-alias %s -gsimple-template-names=simple \
+// RUN: | FileCheck %s
+
+ Check that -gtemplate-alias causes DW_TAG_template_alias emission for
+ template aliases with default parameter values. See template-alias.cpp for
+ more template alias tests.
+
+template
+struct X {
+ char m;
+};
+
+template
+struct Y {
+ char n;
+};
+
+template class T = Y, int I = 5,
typename... Ts>
+using A = X;
+
+ We should be able to emit type alias metadata which describes all the
+ values, including the defaulted parameters and empty parameter pack.
+A a;
+
+// CHECK: !DIDerivedType(tag: DW_TAG_template_alias, name: "A", file: ![[#]],
line: [[#]], baseType: ![[baseType:[0-9]+]], extraData: ![[extraData:[0-9]+]])
+// CHECK: ![[baseType]] = distinct !DICompositeType(tag:
DW_TAG_structure_type, name: "X",
+// CHECK: ![[int:[0-9]+]] = !DIBasicType(name: "int", size: 32, encoding:
DW_ATE_signed)
+// CHECK: ![[extraData]] = !{![[NonDefault:[0-9]+]], ![[T:[0-9]+]],
![[I:[0-9]+]], ![[Ts:[0-9]+]]}
+// CHECK: ![[NonDefault]] = !DITemplateTypeParameter(name: "NonDefault", type:
![[int]])
+// CHECK: ![[T]] = !DITemplateValueParameter(tag:
DW_TAG_GNU_template_template_param, name: "T", defaulted: true, value: !"Y")
+// CHECK: ![[I]] = !DITemplateValueParameter(name: "I", type: ![[int]],
defaulted: true, value: i32 5)
+// CHECK: ![[Ts]] = !DITemplateValueParameter(tag:
DW_TAG_GNU_template_parameter_pack, name: "Ts", value: ![[types:[0-9]+]])
+// CHECK: ![[types]] = !{}
OCHyams wrote:
For anyone interested, the dwarf for this test case looks like this:
```
$ clang -gsimple-template-names test9.cpp -g -c -o - -gtemplate-alias |
llvm-dwarfdump - --name A --show-children
-: file format elf64-x86-64
0x0029: DW_TAG_template_alias
DW_AT_type(0x0044 "X")
DW_AT_name("A")
DW_AT_decl_file ("/home/och/scratch/test9.cpp")
DW_AT_decl_line (13)
0x0031: DW_TAG_template_type_parameter
DW_AT_type (0x005a "int")
DW_AT_name ("NonDefault")
0x0037: DW_TAG_GNU_template_template_param
DW_AT_name ("T")
DW_AT_default_value (true)
DW_AT_GNU_template_name ("Y")
0x003a: DW_TAG_template_value_parameter
DW_AT_type (0x005a "int")
DW_AT_name ("I")
DW_AT_default_value (true)
DW_AT_const_value (5)
0x0041: DW_TAG_GNU_template_parameter_pack
DW_AT_name ("Ts")
0x0043: NULL
```
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -1332,6 +1332,54 @@ llvm::DIType *CGDebugInfo::CreateType(const
TemplateSpecializationType *Ty,
auto PP = getPrintingPolicy();
Ty->getTemplateName().print(OS, PP, TemplateName::Qualified::None);
+ SourceLocation Loc = AliasDecl->getLocation();
+
+ if (CGM.getCodeGenOpts().DebugTemplateAlias) {
+// TemplateSpecializationType doesn't know if its template args are
+// being substituted into a parameter pack. We can find out if that's
+// the case now by inspecting the TypeAliasTemplateDecl template
+// parameters. Insert Ty's template args into SpecArgs, bundling args
+// passed to a parameter pack into a TemplateArgument::Pack.
+SmallVector SpecArgs;
+{
+ ArrayRef SubstArgs = Ty->template_arguments();
+ for (const NamedDecl *P : TD->getTemplateParameters()->asArray()) {
+if (P->isParameterPack()) {
+ SpecArgs.push_back(TemplateArgument(SubstArgs));
+ break;
+}
+// Skip defaulted args.
+if (SubstArgs.empty()) {
+ // If SubstArgs is now empty (we're taking from it each iteration)
and
+ // this template parameter isn't a pack, then that should mean we're
+ // using default values for the remaining template parameters.
+ break;
OCHyams wrote:
I'm glad you pointed it out - it was doing what I wanted it to but I hadn't
tried very hard to get it to gather default argument values, and I agree
copying the class template instantiation debug info is the safest bet.
I've implemented that now. Slightly worried there are other template parameter
kinds I've not thought of, currently guarded by an llvm_unrecahable. Maybe
there's something better to do there - do you have any thoughts on this?
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/87623 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -1332,6 +1332,54 @@ llvm::DIType *CGDebugInfo::CreateType(const
TemplateSpecializationType *Ty,
auto PP = getPrintingPolicy();
Ty->getTemplateName().print(OS, PP, TemplateName::Qualified::None);
+ SourceLocation Loc = AliasDecl->getLocation();
+
+ if (CGM.getCodeGenOpts().DebugTemplateAlias) {
+// TemplateSpecializationType doesn't know if its template args are
+// being substituted into a parameter pack. We can find out if that's
+// the case now by inspecting the TypeAliasTemplateDecl template
+// parameters. Insert Ty's template args into SpecArgs, bundling args
+// passed to a parameter pack into a TemplateArgument::Pack.
+SmallVector SpecArgs;
+{
+ ArrayRef SubstArgs = Ty->template_arguments();
+ for (const NamedDecl *P : TD->getTemplateParameters()->asArray()) {
+if (P->isParameterPack()) {
+ SpecArgs.push_back(TemplateArgument(SubstArgs));
+ break;
+}
+// Skip defaulted args.
+if (SubstArgs.empty()) {
+ // If SubstArgs is now empty (we're taking from it each iteration)
and
+ // this template parameter isn't a pack, then that should mean we're
+ // using default values for the remaining template parameters.
+ break;
Michael137 wrote:
Ah I see, thanks for clarifying! Just thought I'd point it out in case this
wasn't expected. The DWARF spec has this to say about the template parameters
of an alias:
```
The template alias entry has child entries describing the template actual
parameters (see Section 2.23 on page 57).
```
Which to me implies that the parameters of an alias instantiation follow the
same rules as those for class template instantiations.
Regarding name reconstruction, defaulted template arguments should have their
`IsDefaulted` bit set, so when reconstructing the name we should be able to
drop defaulted arguments? If that's too much of a hassle maybe not emitting
them at all as you do now is also fine?
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -1332,6 +1332,54 @@ llvm::DIType *CGDebugInfo::CreateType(const
TemplateSpecializationType *Ty,
auto PP = getPrintingPolicy();
Ty->getTemplateName().print(OS, PP, TemplateName::Qualified::None);
+ SourceLocation Loc = AliasDecl->getLocation();
+
+ if (CGM.getCodeGenOpts().DebugTemplateAlias) {
+// TemplateSpecializationType doesn't know if its template args are
+// being substituted into a parameter pack. We can find out if that's
+// the case now by inspecting the TypeAliasTemplateDecl template
+// parameters. Insert Ty's template args into SpecArgs, bundling args
+// passed to a parameter pack into a TemplateArgument::Pack.
+SmallVector SpecArgs;
+{
+ ArrayRef SubstArgs = Ty->template_arguments();
+ for (const NamedDecl *P : TD->getTemplateParameters()->asArray()) {
+if (P->isParameterPack()) {
+ SpecArgs.push_back(TemplateArgument(SubstArgs));
+ break;
+}
+// Skip defaulted args.
+if (SubstArgs.empty()) {
+ // If SubstArgs is now empty (we're taking from it each iteration)
and
+ // this template parameter isn't a pack, then that should mean we're
+ // using default values for the remaining template parameters.
+ break;
OCHyams wrote:
> Does this mean we won't be emitting the defaulted template arguments as part
> of the DW_TAG_template_alias?
Yes. At the moment (without this patch, with typedefs) the names get
constructed without the default values which is different behaviour to structs:
```
template
struct X {
char n;
};
template
using B = X;
// DW_TAG_typedef
// DW_AT_name ("B<>")"
B<> a;
// DW_TAG_typedef
// DW_AT_name ("B")
B b;
template
struct F {
char n;
};
// DW_TAG_structure_type
// DW_AT_name ("F")
F<> f;
```
Using template parameters to reconstruct the name, without the default values,
would result in the same names as the typedefs above create. That said, it
does feel slightly "incomplete" to leave them off. I'll have a look today to
see whether its possible to do anything about that.
I'll add a test for default args either way, and move the code out into a
function.
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -310,6 +310,24 @@ namespace llvm {
DINode::DIFlags Flags = DINode::FlagZero,
DINodeArray Annotations = nullptr);
+/// Create debugging information entry for a template alias.
+/// \param Ty Original type.
+/// \param NameAlias name.
+/// \param FileFile where this type is defined.
+/// \param LineNo Line number.
+/// \param Context The surrounding context for the alias.
+/// \param TParams The template arguments.
+/// \param AlignInBits Alignment. (optional)
+/// \param Flags Flags to describe inheritance attribute (optional),
+///e.g. private.
OCHyams wrote:
It says so at the end of the line. I suppose it would be more in keeping if it
came after the full stop `e.g. private. (here)`, but it felt right where it is
at the moment too. ymmv.
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
https://github.com/OCHyams commented: (oops, didn't submit my inline replies...) https://github.com/llvm/llvm-project/pull/87623 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -1332,6 +1332,54 @@ llvm::DIType *CGDebugInfo::CreateType(const
TemplateSpecializationType *Ty,
auto PP = getPrintingPolicy();
Ty->getTemplateName().print(OS, PP, TemplateName::Qualified::None);
+ SourceLocation Loc = AliasDecl->getLocation();
+
+ if (CGM.getCodeGenOpts().DebugTemplateAlias) {
+// TemplateSpecializationType doesn't know if its template args are
+// being substitued into a parameter pack. We can find out if that's
OCHyams wrote:
I need a clang-format-spellcheck or something. Thanks, fixed.
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
https://github.com/OCHyams edited https://github.com/llvm/llvm-project/pull/87623 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
Michael137 wrote: Btw, as a follow-up to this patch should we check that this is compatible with dsymutil (i.e., running `dsymutil --verify`)? I suspect it might need a fixup (given LLDB doesn't even support this tag) https://github.com/llvm/llvm-project/pull/87623 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -1332,6 +1332,54 @@ llvm::DIType *CGDebugInfo::CreateType(const
TemplateSpecializationType *Ty,
auto PP = getPrintingPolicy();
Ty->getTemplateName().print(OS, PP, TemplateName::Qualified::None);
+ SourceLocation Loc = AliasDecl->getLocation();
+
+ if (CGM.getCodeGenOpts().DebugTemplateAlias) {
+// TemplateSpecializationType doesn't know if its template args are
+// being substituted into a parameter pack. We can find out if that's
+// the case now by inspecting the TypeAliasTemplateDecl template
+// parameters. Insert Ty's template args into SpecArgs, bundling args
+// passed to a parameter pack into a TemplateArgument::Pack.
+SmallVector SpecArgs;
+{
+ ArrayRef SubstArgs = Ty->template_arguments();
+ for (const NamedDecl *P : TD->getTemplateParameters()->asArray()) {
+if (P->isParameterPack()) {
+ SpecArgs.push_back(TemplateArgument(SubstArgs));
+ break;
+}
+// Skip defaulted args.
+if (SubstArgs.empty()) {
+ // If SubstArgs is now empty (we're taking from it each iteration)
and
+ // this template parameter isn't a pack, then that should mean we're
+ // using default values for the remaining template parameters.
+ break;
Michael137 wrote:
Does this mean we won't be emitting the defaulted template arguments as part of
the `DW_TAG_template_alias`? Would be good to add a test case for defaulted
arguments, so this logic is exercised (if you haven't already)
Also minor nit, is this logic getting sufficiently long to warrant a separate
helper function?
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -310,6 +310,24 @@ namespace llvm {
DINode::DIFlags Flags = DINode::FlagZero,
DINodeArray Annotations = nullptr);
+/// Create debugging information entry for a template alias.
+/// \param Ty Original type.
+/// \param NameAlias name.
+/// \param FileFile where this type is defined.
+/// \param LineNo Line number.
+/// \param Context The surrounding context for the alias.
+/// \param TParams The template arguments.
+/// \param AlignInBits Alignment. (optional)
+/// \param Flags Flags to describe inheritance attribute (optional),
+///e.g. private.
pogo59 wrote:
Flags is also (optional)
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -465,3 +465,15 @@ // MANGLED_TEMP_NAMES: error: unknown argument '-gsimple-template-names=mangled'; did you mean '-Xclang -gsimple-template-names=mangled' // RUN: %clang -### -target x86_64 -c -g %s 2>&1 | FileCheck --check-prefix=FULL_TEMP_NAMES --implicit-check-not=debug-forward-template-params %s // FULL_TEMP_NAMES-NOT: -gsimple-template-names + + Test -g[no-]template-alias (enabled by default with SCE debugger tuning). Requires DWARFv5. +// RUN: %clang -### -target x86_64 -c -gdwarf-5 -gsce %s 2>&1 | FileCheck %s --check-prefixes=TEMPLATE-ALIAS pogo59 wrote: Huh Coulda sworn that was changed... okay. https://github.com/llvm/llvm-project/pull/87623 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
https://github.com/pogo59 edited https://github.com/llvm/llvm-project/pull/87623 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -1332,6 +1332,54 @@ llvm::DIType *CGDebugInfo::CreateType(const
TemplateSpecializationType *Ty,
auto PP = getPrintingPolicy();
Ty->getTemplateName().print(OS, PP, TemplateName::Qualified::None);
+ SourceLocation Loc = AliasDecl->getLocation();
+
+ if (CGM.getCodeGenOpts().DebugTemplateAlias) {
+// TemplateSpecializationType doesn't know if its template args are
+// being substitued into a parameter pack. We can find out if that's
pogo59 wrote:
substituted
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
https://github.com/pogo59 commented: The flag situation is good. Tuning influences the default for the new option, which is how we want that to work. I'm happy, a couple of nits left. I'll leave it to others to do the final approval. https://github.com/llvm/llvm-project/pull/87623 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -1,4 +1,4 @@ -// RUN: %clang -g -std=c++11 -S -emit-llvm %s -o - | FileCheck %s +// RUN: %clang -ggdb -std=c++11 -S -emit-llvm %s -o - | FileCheck %s OCHyams wrote: Agreed, done https://github.com/llvm/llvm-project/pull/87623 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -465,3 +465,15 @@ // MANGLED_TEMP_NAMES: error: unknown argument '-gsimple-template-names=mangled'; did you mean '-Xclang -gsimple-template-names=mangled' // RUN: %clang -### -target x86_64 -c -g %s 2>&1 | FileCheck --check-prefix=FULL_TEMP_NAMES --implicit-check-not=debug-forward-template-params %s // FULL_TEMP_NAMES-NOT: -gsimple-template-names + + Test -g[no-]template-alias (enabled by default with SCE debugger tuning). Requires DWARFv5. +// RUN: %clang -### -target x86_64 -c -gdwarf-5 -gsce %s 2>&1 | FileCheck %s --check-prefixes=TEMPLATE-ALIAS OCHyams wrote: This gave `clang: error: unknown argument '--target'; did you mean '-target'?` https://github.com/llvm/llvm-project/pull/87623 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -4584,6 +4584,32 @@ renderDebugOptions(const ToolChain , const Driver ,
const llvm::Triple ,
}
}
+ // Emit DW_TAG_template_alias for template aliases? True by default for SCE.
+ const auto *DebugTemplateAlias = Args.getLastArg(
+ options::OPT_gtemplate_alias, options::OPT_gno_template_alias);
+ bool UseDebugTemplateAlias =
+ DebuggerTuning == llvm::DebuggerKind::SCE && RequestedDWARFVersion >= 5;
+ if (DebugTemplateAlias &&
+ checkDebugInfoOption(DebugTemplateAlias, Args, D, TC)) {
+const auto = DebugTemplateAlias->getOption();
+UseDebugTemplateAlias = Opt.matches(options::OPT_gtemplate_alias);
+ }
+ if (UseDebugTemplateAlias) {
+// DW_TAG_template_alias is a DWARFv5 feature. Warn if we can't use it.
+if (DebugTemplateAlias && RequestedDWARFVersion < 5)
+ D.Diag(diag::warn_drv_dwarf_feature_requires_version)
+ << DebugTemplateAlias->getAsString(Args) << 5
+ << RequestedDWARFVersion;
+else if (DebugTemplateAlias && EffectiveDWARFVersion < 5)
+ // The toolchain has reduced allowed dwarf version, so we can't enable
+ // -gtemplate-alias.
+ D.Diag(diag::warn_drv_dwarf_version_limited_by_target)
+ << DebugTemplateAlias->getAsString(Args) << TC.getTripleString() << 5
+ << EffectiveDWARFVersion;
+else
+ CmdArgs.push_back("-gtemplate-alias");
+ }
OCHyams wrote:
Done
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -5361,7 +5383,56 @@ static bool IsReconstitutableType(QualType QT) {
return T.Reconstitutable;
}
-std::string CGDebugInfo::GetName(const Decl *D, bool Qualified) const {
+bool CGDebugInfo::HasReconstitutableArgs(
+ArrayRef Args) const {
+ return llvm::all_of(Args, [&](const TemplateArgument ) {
+switch (TA.getKind()) {
+case TemplateArgument::Template:
+ // Easy to reconstitute - the value of the parameter in the debug
+ // info is the string name of the template. (so the template name
+ // itself won't benefit from any name rebuilding, but that's a
+ // representational limitation - maybe DWARF could be
+ // changed/improved to use some more structural representation)
OCHyams wrote:
Promoted the sentence out of parens.
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
OCHyams wrote: > But perhaps at least you could add named accessors to DIDerivedType for the > template params, same as DIGlobalVariable/DICompositeType have? Done. I couldn't add an assert (`getTag() == dwarf::DW_TAG_template_alias`) without including `Dwarf.h`. I've chosen to not do that as it looks like its been avoided (but possibly just not really needed here before), but I'd be happy to add it if the extra compile time isn't worth worrying about. > (oh, and in case anyone hasn't mentioned it already - this would generally be > committed in smaller pieces upstream - adding the LLVM functionality first, > then adding clang patches that use that functionality) Sure, no problem, I'll open separate pull requests once this has been accepted then. --- I think I've addressed all the concerns raised now except for the variadic issue I mentioned, which I'll look at tomorrow. https://github.com/llvm/llvm-project/pull/87623 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -465,3 +465,15 @@ // MANGLED_TEMP_NAMES: error: unknown argument '-gsimple-template-names=mangled'; did you mean '-Xclang -gsimple-template-names=mangled' // RUN: %clang -### -target x86_64 -c -g %s 2>&1 | FileCheck --check-prefix=FULL_TEMP_NAMES --implicit-check-not=debug-forward-template-params %s // FULL_TEMP_NAMES-NOT: -gsimple-template-names + + Test -g[no-]template-alias (enabled by default with SCE debugger tuning). Requires DWARFv5. +// RUN: %clang -### -target x86_64 -c -gdwarf-5 -gsce %s 2>&1 | FileCheck %s --check-prefixes=TEMPLATE-ALIAS pogo59 wrote: `--target` https://github.com/llvm/llvm-project/pull/87623 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -5361,7 +5383,56 @@ static bool IsReconstitutableType(QualType QT) {
return T.Reconstitutable;
}
-std::string CGDebugInfo::GetName(const Decl *D, bool Qualified) const {
+bool CGDebugInfo::HasReconstitutableArgs(
+ArrayRef Args) const {
+ return llvm::all_of(Args, [&](const TemplateArgument ) {
+switch (TA.getKind()) {
+case TemplateArgument::Template:
+ // Easy to reconstitute - the value of the parameter in the debug
+ // info is the string name of the template. (so the template name
+ // itself won't benefit from any name rebuilding, but that's a
+ // representational limitation - maybe DWARF could be
+ // changed/improved to use some more structural representation)
pogo59 wrote:
Ah, didn't notice it was a copy-paste. But it would still be nice to have the
comment follow our style guide. So, up to you.
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -4584,6 +4584,32 @@ renderDebugOptions(const ToolChain , const Driver ,
const llvm::Triple ,
}
}
+ // Emit DW_TAG_template_alias for template aliases? True by default for SCE.
+ const auto *DebugTemplateAlias = Args.getLastArg(
+ options::OPT_gtemplate_alias, options::OPT_gno_template_alias);
+ bool UseDebugTemplateAlias =
+ DebuggerTuning == llvm::DebuggerKind::SCE && RequestedDWARFVersion >= 5;
+ if (DebugTemplateAlias &&
+ checkDebugInfoOption(DebugTemplateAlias, Args, D, TC)) {
+const auto = DebugTemplateAlias->getOption();
+UseDebugTemplateAlias = Opt.matches(options::OPT_gtemplate_alias);
+ }
+ if (UseDebugTemplateAlias) {
+// DW_TAG_template_alias is a DWARFv5 feature. Warn if we can't use it.
+if (DebugTemplateAlias && RequestedDWARFVersion < 5)
+ D.Diag(diag::warn_drv_dwarf_feature_requires_version)
+ << DebugTemplateAlias->getAsString(Args) << 5
+ << RequestedDWARFVersion;
+else if (DebugTemplateAlias && EffectiveDWARFVersion < 5)
+ // The toolchain has reduced allowed dwarf version, so we can't enable
+ // -gtemplate-alias.
+ D.Diag(diag::warn_drv_dwarf_version_limited_by_target)
+ << DebugTemplateAlias->getAsString(Args) << TC.getTripleString() << 5
+ << EffectiveDWARFVersion;
+else
+ CmdArgs.push_back("-gtemplate-alias");
+ }
pogo59 wrote:
Please take out the diags here, if someone explicitly asks for something in
debug info we do it.
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
https://github.com/pogo59 deleted https://github.com/llvm/llvm-project/pull/87623 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -4584,6 +4584,32 @@ renderDebugOptions(const ToolChain , const Driver ,
const llvm::Triple ,
}
}
+ // Emit DW_TAG_template_alias for template aliases? True by default for SCE.
+ const auto *DebugTemplateAlias = Args.getLastArg(
+ options::OPT_gtemplate_alias, options::OPT_gno_template_alias);
+ bool UseDebugTemplateAlias =
+ DebuggerTuning == llvm::DebuggerKind::SCE && RequestedDWARFVersion >= 5;
+ if (DebugTemplateAlias &&
+ checkDebugInfoOption(DebugTemplateAlias, Args, D, TC)) {
+const auto = DebugTemplateAlias->getOption();
+UseDebugTemplateAlias = Opt.matches(options::OPT_gtemplate_alias);
+ }
+ if (UseDebugTemplateAlias) {
+// DW_TAG_template_alias is a DWARFv5 feature. Warn if we can't use it.
+if (DebugTemplateAlias && RequestedDWARFVersion < 5)
+ D.Diag(diag::warn_drv_dwarf_feature_requires_version)
+ << DebugTemplateAlias->getAsString(Args) << 5
+ << RequestedDWARFVersion;
+else if (DebugTemplateAlias && EffectiveDWARFVersion < 5)
+ // The toolchain has reduced allowed dwarf version, so we can't enable
+ // -gtemplate-alias.
+ D.Diag(diag::warn_drv_dwarf_version_limited_by_target)
+ << DebugTemplateAlias->getAsString(Args) << TC.getTripleString() << 5
+ << EffectiveDWARFVersion;
+else
pogo59 wrote:
```suggestion
else
```
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
dwblaikie wrote:
> Thanks @dwblaikie
>
> > Bit unfortunate to store template parameters in different ways (in the
> > extraData for the alias template, but in the templateParams for the
> > composite types) - but I guess it'd be more invasive to try to represent
> > alias templates as composite types?
>
> I hadn't actually tried using DICompsiteType. I didn't choose DIDerivedType
> for any particularly principled reasons: typedefs use derived types which
> made it very easy to find places that needed updating, and an alias "felt"
> more like a derived type than composite type. Having looked only briefly, I
> don't _think_ it'd be any more invasive to use composite types instead (not
> 100% sure without diving in). I'm happy to give that a go if that is your
> preference?
Eh, I guess if we've already got DIGlobalVariable having template params, we
aren't going to unify all 3 cases - not sure if unifying 2 out of three is
particularly valuable, especially if it ends up with a different conflict (that
alias templates look less like aliases & so need special casing in that
direction)
But perhaps at least you could add named accessors to DIDerivedType for the
template params, same as DIGlobalVariable/DICompositeType have?
(oh, and in case anyone hasn't mentioned it already - this would generally be
committed in smaller pieces upstream - adding the LLVM functionality first,
then adding clang patches that use that functionality)
> I've just found an input that causes an assertion failure in
> `CollectTemplateParams` with my implementation:
>
> ```
> template
> struct X {
> Y m1;
> Z m2;
> };
>
> template
> using A = X;
>
> A a;
> ```
>
> There's a mismatch between the template parameter list and template argument
> list sizes, created here:
>
> ```
> if (CGM.getCodeGenOpts().DebugTemplateAlias) {
> TemplateArgs Args = {TD->getTemplateParameters(),
> Ty->template_arguments()}; // <--- here
> ```
>
> I notice that we emit `DW_TAG_GNU_template_parameter_pack` for, e.g.,
> variadic template struct instantiations. I've not figured out how to fix this
> just yet, but thought I'd bring it up in case there's something obvious.
yeah, probably worth checking the APIs that we use for building the template
args for classes and variable templates, they probably have some handling for
this - would be good to share/reuse that code, rather than duplicating it, once
you find it
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
OCHyams wrote:
Thanks @dwblaikie
> Bit unfortunate to store template parameters in different ways (in the
> extraData for the alias template, but in the templateParams for the composite
> types) - but I guess it'd be more invasive to try to represent alias
> templates as composite types?
I hadn't actually tried using DICompsiteType. I didn't choose DIDerivedType for
any particularly principled reasons: typedefs use derived types which made it
very easy to find places that needed updating, and an alias "felt" more like a
derived type than composite type. Having looked only briefly, I don't _think_
it'd be any more invasive to use composite types instead (not 100% sure without
diving in). I'm happy to give that a go if that is your preference?
---
I've just found an input that causes an assertion failure in
`CollectTemplateParams` with my implementation:
```
template
struct X {
Y m1;
Z m2;
};
template
using A = X;
A a;
```
There's a mismatch between the template parameter list and template argument
list sizes, created here:
```
if (CGM.getCodeGenOpts().DebugTemplateAlias) {
TemplateArgs Args = {TD->getTemplateParameters(),
Ty->template_arguments()}; // <--- here
```
I notice that we emit `DW_TAG_GNU_template_parameter_pack` for, e.g., variadic
template struct instantiations. I've not figured out how to fix this just yet,
but thought I'd bring it up in case there's something obvious.
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
pogo59 wrote: > @pogo59 Hmm - I thought you held a strong preference that debugger tuning > never be the only way to access a certain feature, and that we should always > have direct control of these features via flags (but with default (or > explicit) debugger tuning setting some of these flag defaults based on the > preferred content for that debugger). Perhaps i'm misremembering? You are not misremembering. Ideally _tuning_ is not _gating_ (although we probably never expressed it that way). We probably have not fully held to that ideal, but if we're going to have knobs to tweak we should implement the knobs properly. I will be making another pass over this PR later today and pay attention to that. https://github.com/llvm/llvm-project/pull/87623 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -5361,7 +5383,56 @@ static bool IsReconstitutableType(QualType QT) {
return T.Reconstitutable;
}
-std::string CGDebugInfo::GetName(const Decl *D, bool Qualified) const {
+bool CGDebugInfo::HasReconstitutableArgs(
+ArrayRef Args) const {
+ return llvm::all_of(Args, [&](const TemplateArgument ) {
+switch (TA.getKind()) {
+case TemplateArgument::Template:
+ // Easy to reconstitute - the value of the parameter in the debug
+ // info is the string name of the template. (so the template name
+ // itself won't benefit from any name rebuilding, but that's a
+ // representational limitation - maybe DWARF could be
+ // changed/improved to use some more structural representation)
+ return true;
+case TemplateArgument::Declaration:
+ // Reference and pointer non-type template parameters point to
+ // variables, functions, etc and their value is, at best (for
+ // variables) represented as an address - not a reference to the
+ // DWARF describing the variable/function/etc. This makes it hard,
+ // possibly impossible to rebuild the original name - looking up
+ // the address in the executable file's symbol table would be
+ // needed.
+ return false;
+case TemplateArgument::NullPtr:
+ // These could be rebuilt, but figured they're close enough to the
+ // declaration case, and not worth rebuilding.
+ return false;
+case TemplateArgument::Pack:
+ // A pack is invalid if any of the elements of the pack are
+ // invalid.
+ return HasReconstitutableArgs(TA.getPackAsArray());
+case TemplateArgument::Integral:
+ // Larger integers get encoded as DWARF blocks which are a bit
+ // harder to parse back into a large integer, etc - so punting on
+ // this for now. Re-parsing the integers back into APInt is
+ // probably feasible some day.
+ return TA.getAsIntegral().getBitWidth() <= 64 &&
+ IsReconstitutableType(TA.getIntegralType());
+case TemplateArgument::StructuralValue:
+ return false;
+case TemplateArgument::Type:
+ return IsReconstitutableType(TA.getAsType());
+case TemplateArgument::Expression:
+ return IsReconstitutableType(TA.getAsExpr()->getType());
+default:
+ llvm_unreachable("Other, unresolved, template arguments should "
+ "not be seen here");
+}
+ });
+}
+
+std::string CGDebugInfo::GetName(const Decl *D, bool Qualified,
+ const Type *Ty) const {
Michael137 wrote:
Ah thanks for the additional context. Keeping it as is seems fine to me then. I
don't see a better way to get to template arguments from the alias decl. My
initial thought of special-casing this in `GetName` isn't necessarily great
either
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
dwblaikie wrote: Re: code: Looks about right. Bit unfortunate to store template parameters in different ways (in the `extraData` for the alias template, but in the `templateParams` for the composite types) - but I guess it'd be more invasive to try to represent alias templates as composite types? https://github.com/llvm/llvm-project/pull/87623 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
dwblaikie wrote: > > I'm a little uncomfortable with adding a new user-facing option for > > template aliases. Even with that in place, we should not warn and refuse to > > do what the user asked for, based on DWARF version. -gdwarf-2 -gsplit-dwarf > > generates a .dwo file claiming to be v2, for example. > > I don't have strong feelings about the flag setup and am happy to change it. > @dwblaikie mentioned that we might not want to have the debugger tuning be > the only way of controlling whether or not we emit this DIE, so adding > `-gtemplate-alias` which has its default set based on debugger tuning seemed > like a reasonable way of doing it. What would be the best way of controlling > this feature in your opinion (bearing in mind that GCC doesn't emit it, and > GDB and LLDB don't the DIE)? @pogo59 Hmm - I thought you held a strong preference that debugger tuning never be the only way to access a certain feature, and that we should always have direct control of these features via flags (but with default (or explicit) debugger tuning setting some of these flag defaults based on the preferred content for that debugger). Perhaps i'm misremembering? https://github.com/llvm/llvm-project/pull/87623 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
OCHyams wrote: Fixed https://github.com/llvm/llvm-project/pull/87623 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -152,9 +152,11 @@ uint64_t DebugHandlerBase::getBaseTypeSize(const DIType
*Ty) {
unsigned Tag = DDTy->getTag();
if (Tag != dwarf::DW_TAG_member && Tag != dwarf::DW_TAG_typedef &&
- Tag != dwarf::DW_TAG_const_type && Tag != dwarf::DW_TAG_volatile_type &&
+ Tag != dwarf::DW_TAG_template_alias && Tag != dwarf::DW_TAG_const_type &&
+ Tag != dwarf::DW_TAG_volatile_type &&
Tag != dwarf::DW_TAG_restrict_type && Tag != dwarf::DW_TAG_atomic_type &&
- Tag != dwarf::DW_TAG_immutable_type)
+ Tag != dwarf::DW_TAG_immutable_type &&
+ Tag != dwarf::DW_TAG_template_alias)
OCHyams wrote:
Fixed
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -5361,7 +5383,56 @@ static bool IsReconstitutableType(QualType QT) {
return T.Reconstitutable;
}
-std::string CGDebugInfo::GetName(const Decl *D, bool Qualified) const {
+bool CGDebugInfo::HasReconstitutableArgs(
+ArrayRef Args) const {
+ return llvm::all_of(Args, [&](const TemplateArgument ) {
+switch (TA.getKind()) {
+case TemplateArgument::Template:
+ // Easy to reconstitute - the value of the parameter in the debug
+ // info is the string name of the template. (so the template name
+ // itself won't benefit from any name rebuilding, but that's a
+ // representational limitation - maybe DWARF could be
+ // changed/improved to use some more structural representation)
+ return true;
+case TemplateArgument::Declaration:
+ // Reference and pointer non-type template parameters point to
+ // variables, functions, etc and their value is, at best (for
+ // variables) represented as an address - not a reference to the
+ // DWARF describing the variable/function/etc. This makes it hard,
+ // possibly impossible to rebuild the original name - looking up
+ // the address in the executable file's symbol table would be
+ // needed.
+ return false;
+case TemplateArgument::NullPtr:
+ // These could be rebuilt, but figured they're close enough to the
+ // declaration case, and not worth rebuilding.
+ return false;
+case TemplateArgument::Pack:
+ // A pack is invalid if any of the elements of the pack are
+ // invalid.
+ return HasReconstitutableArgs(TA.getPackAsArray());
+case TemplateArgument::Integral:
+ // Larger integers get encoded as DWARF blocks which are a bit
+ // harder to parse back into a large integer, etc - so punting on
+ // this for now. Re-parsing the integers back into APInt is
+ // probably feasible some day.
+ return TA.getAsIntegral().getBitWidth() <= 64 &&
+ IsReconstitutableType(TA.getIntegralType());
+case TemplateArgument::StructuralValue:
+ return false;
+case TemplateArgument::Type:
+ return IsReconstitutableType(TA.getAsType());
+case TemplateArgument::Expression:
+ return IsReconstitutableType(TA.getAsExpr()->getType());
+default:
+ llvm_unreachable("Other, unresolved, template arguments should "
+ "not be seen here");
+}
+ });
+}
+
+std::string CGDebugInfo::GetName(const Decl *D, bool Qualified,
+ const Type *Ty) const {
OCHyams wrote:
Aha, good catch. You're correct on both counts - it's not used in this patch
and that was one of the approaches I tried out.
This is discussed a bit on the issue starting here
https://github.com/llvm/llvm-project/issues/54624#issuecomment-2024754144.
IIRC, we _could_ pass in the `TemplateSpecializationType` to get the template
arguments for this case, but I think we'd need to introduce a special case for
building the name where we can't use `D`'s `getNameForDiagnostic` on line 5461
and 5481 of the original file.
My memory is slightly blurred by the conference - I think I was erring on the
side of caution given my unfamiliarity with the code, preferring to have a
special case localised at the usage site rather than inside a utility function.
I'd be happy to change the patch to use the `TemplateSpecializationType`
parameter if you'd like to see what it looks like?
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -310,6 +310,23 @@ namespace llvm {
DINode::DIFlags Flags = DINode::FlagZero,
DINodeArray Annotations = nullptr);
+/// Create debugging information entry for a typedef.
+/// \param Ty Original type.
+/// \param NameTypedef name.
+/// \param FileFile where this type is defined.
+/// \param LineNo Line number.
+/// \param Context The surrounding context for the typedef.
+/// \param TParams The template arguments.
+/// \param AlignInBits Alignment. (optional)
+/// \param Flags Flags to describe inheritance attribute, e.g.
private
OCHyams wrote:
Fixed
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -5361,7 +5383,56 @@ static bool IsReconstitutableType(QualType QT) {
return T.Reconstitutable;
}
-std::string CGDebugInfo::GetName(const Decl *D, bool Qualified) const {
+bool CGDebugInfo::HasReconstitutableArgs(
+ArrayRef Args) const {
+ return llvm::all_of(Args, [&](const TemplateArgument ) {
+switch (TA.getKind()) {
+case TemplateArgument::Template:
+ // Easy to reconstitute - the value of the parameter in the debug
+ // info is the string name of the template. (so the template name
+ // itself won't benefit from any name rebuilding, but that's a
+ // representational limitation - maybe DWARF could be
+ // changed/improved to use some more structural representation)
+ return true;
+case TemplateArgument::Declaration:
+ // Reference and pointer non-type template parameters point to
+ // variables, functions, etc and their value is, at best (for
+ // variables) represented as an address - not a reference to the
+ // DWARF describing the variable/function/etc. This makes it hard,
+ // possibly impossible to rebuild the original name - looking up
+ // the address in the executable file's symbol table would be
+ // needed.
+ return false;
+case TemplateArgument::NullPtr:
+ // These could be rebuilt, but figured they're close enough to the
+ // declaration case, and not worth rebuilding.
+ return false;
+case TemplateArgument::Pack:
+ // A pack is invalid if any of the elements of the pack are
+ // invalid.
+ return HasReconstitutableArgs(TA.getPackAsArray());
+case TemplateArgument::Integral:
+ // Larger integers get encoded as DWARF blocks which are a bit
+ // harder to parse back into a large integer, etc - so punting on
+ // this for now. Re-parsing the integers back into APInt is
+ // probably feasible some day.
+ return TA.getAsIntegral().getBitWidth() <= 64 &&
+ IsReconstitutableType(TA.getIntegralType());
+case TemplateArgument::StructuralValue:
+ return false;
+case TemplateArgument::Type:
+ return IsReconstitutableType(TA.getAsType());
+case TemplateArgument::Expression:
+ return IsReconstitutableType(TA.getAsExpr()->getType());
+default:
+ llvm_unreachable("Other, unresolved, template arguments should "
+ "not be seen here");
+}
+ });
+}
+
+std::string CGDebugInfo::GetName(const Decl *D, bool Qualified,
+ const Type *Ty) const {
Michael137 wrote:
Are we using this extra `Ty` parameter anywhere? I assume your original intent
was to pass the `TemplateSpecializationType` into `GetName` and get the
template arguments for the alias decl that way? Any reason you didn't end up
doing that?
https://github.com/llvm/llvm-project/pull/87623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
@@ -1,4 +1,4 @@ -// RUN: %clang -g -std=c++11 -S -emit-llvm %s -o - | FileCheck %s +// RUN: %clang -ggdb -std=c++11 -S -emit-llvm %s -o - | FileCheck %s Michael137 wrote: Do we now lose a tiny bit of coverage for `-glldb`? I assume this change is to make sure the test passes on platforms that are tuned for `sce`? Maybe we should just add `-gno-template-alias` instead? https://github.com/llvm/llvm-project/pull/87623 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang] Emit DW_TAG_template_alias for template aliases (PR #87623)
OCHyams wrote: > LLVM IR parts look good to me. Thanks! @pogo59 said: > Converting frame-types.s from v4 to v5 should be done as a separate commit. > Then adding the alias would be a simpler change to review. I've split that into two commits in this PR: upgrade to v5: [18446f2](https://github.com/llvm/llvm-project/pull/87623/commits/18446f27e31803057d9d90c957e5e191eb263b7b), add template alias: [49d6642](https://github.com/llvm/llvm-project/pull/87623/commits/49d66423ae61baf9cacd42c2b57333e09e4c1e81). It does improve readability a bit but its still quite noisy. I am happy to commit the former separately then rebase this PR, if you'd like? https://github.com/llvm/llvm-project/pull/87623 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
