Change how we handle auto return types for lambda operator() to be consistent with gcc

[shafik]

Summary

D70524 added support for auto return types for C++ member functions. I was implementing support on the LLDB side for looking up the deduced type.

I ran into trouble with some cases with respect to lambdas. I looked into how gcc was handling these cases and it appears gcc emits the deduced return type for lambdas.

So I am changing out behavior to match that.

[shafik]

I did some experiments where I did not restrict this to lambdas case and I could not come up with a test case where we emit debug info and do not have the deduced type.

Are there cases where we emit debug info and we don't have the deduced type?

If not what do we gain by using the auto return type?

[aprantl]

I think this is reasonable, but could you add a testcase?

clang/lib/CodeGen/CGDebugInfo.cpp

1684

Can you add a comment here, explaining why lambdas are special?

[aprantl]

Are there cases where we emit debug info and we don't have the deduced type?

I don't think that would happen for a lambda.
https://dwarfstd.org/ShowIssue.php?issue=131217.1 specifically calls out method declarations without definitions, which makes sense to me.

[dblaikie]

(@probinson as someone I've disagreed with about this before)

Personally I think there's limited value in expressing 'auto' in DWARF at all - we could omit function declarations if the return type is not known (undeduced auto) and wouldn't lose much - basically treating them the same as templates that aren't instantiated yet. (& I believe Sony does this for all functions anyway - only including them when they're defined, not including an exhaustive list of member functions in class definitions)

Does anyone have particular DWARF consumer features they have built/would like to build that benefit from/require knowing that a function was defined with an auto return type?

Previously discussed/debated here: https://reviews.llvm.org/D70524

[probinson]

In D123319#3437250, @dblaikie wrote:

(@probinson as someone I've disagreed with about this before)

Personally I think there's limited value in expressing 'auto' in DWARF at all - we could omit function declarations if the return type is not known (undeduced auto) and wouldn't lose much - basically treating them the same as templates that aren't instantiated yet.

The problem there is that then the CU where the function is defined is the only one where the type description would include that function; and if we are doing e.g. ctor homing, and that CU doesn't happen to construct any instances, then the function will still exist but not be described anywhere. If you're not doing ctor homing, then only one CU's description will mention the auto function; so, whether the debugger knows about the auto function will randomly depend on which CU's description the debugger decided to keep.

I do understand the analogy to templates. The difference I see is that DWARF actually has a way to describe auto function declarations, where it does not have a way to describe template declarations.

(& I believe Sony does this for all functions anyway - only including them when they're defined, not including an exhaustive list of member functions in class definitions)

We have a private feature that (if you're not doing something like ctor homing, or using type units) will suppress mention of _unused_ functions, which works for us because our debugger will merge type descriptions from different units. Most debuggers don't do that, generally keeping just whichever one they found first, and assuming the rest are duplicates. Including auto functions only when defined means the various descriptions aren't actually duplicates, and so the debugger will randomly know or not know about the function depending on which description it picks.

It's true that omitting auto function declarations wouldn't affect Sony because we know how to merge descriptions anyway, but OTOH we never upstreamed the suppress-unused-declarations feature because AFAIK no other debugger understands how to handle the situation. Deliberately introducing omitted declarations seems kinda wrong. With templates we don't have a choice, but with auto functions we do.

I mean, if you're willing to omit auto functions, why wouldn't you be willing to omit other unused functions? The argument that "we don't know the whole type" seems a bit weak, given that DWARF gives you a way to describe what's missing.

[shafik]

Adding test case

[aprantl]

clang/lib/CodeGen/CGDebugInfo.cpp

1684

this is still missing ^

clang/test/CodeGenCXX/no_auto_return_lambda.cpp

2

It would be nice to also have a comment in here, about what is being tested.
Ie.: // Test that clang emits the deduced return type for lambdas.

9

This test is very fragile. Instead of checking that no unspecified type *follows* the first declaration of int and hoping that no other int is emitted in this CU, it would be better to explictly check for the return type of the DISubprogram for g using FileCheck variable substitutions.

[shafik]

clang/test/CodeGenCXX/no_auto_return_lambda.cpp

9

I thought about that but there are a couple of steps to get there e.g.:

!10 = distinct !DISubprogram(name: "g", linkageName: "_Z1gv", scope: !8, file: !8, line: 1, type: !11, scopeLine: 1, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !7, retainedNodes: !14)
!11 = !DISubroutineType(types: !12)
!12 = !{!13}
!13 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)

I have to go from 11 to 12 to 13, is there a better way?

[shafik]

• Making test more robust
• Adding comments

[aprantl]

The new test looks good (I would replace the CHECK-NEXT with CHECK though).
It sounds like there was no objections for doing this for lambdas.

[shafik]

-Replacing CHECK-NEXT with CHECK

[erichkeane]

clang/test/CodeGenCXX/no_auto_return_lambda.cpp

1

So this CodeGen test without a triple is a bit troublesome. The one we've noticed that this fails on the 32 bit windows build:

FAIL: Clang :: CodeGenCXX/no_auto_return_lambda.cpp (41841 of 57667)
******************** TEST 'Clang :: CodeGenCXX/no_auto_return_lambda.cpp' FAILED ********************
Script:
--
: 'RUN: at line 1';   d:\netbatch\build_comp02320_00\ics_top\builds\xmainoclx86win_prod\llvm\bin\clang.exe -cc1 -internal-isystem d:\netbatch\build_comp02320_00\ics_top\builds\xmainoclx86win_prod\llvm\lib\clang\15.0.0\include -nostdsysteminc -emit-llvm -debug-info-kind=limited D:\netbatch\build_comp02320_00\ics_top\llvm\clang\test\CodeGenCXX\no_auto_return_lambda.cpp -o - | d:\netbatch\build_comp02320_00\ics_top\builds\xmainoclx86win_prod\llvm\bin\filecheck.exe D:\netbatch\build_comp02320_00\ics_top\llvm\clang\test\CodeGenCXX\no_auto_return_lambda.cpp
--
Exit Code: 1Command Output (stdout):
--
$ ":" "RUN: at line 1"
$ "d:\netbatch\build_comp02320_00\ics_top\builds\xmainoclx86win_prod\llvm\bin\clang.exe" "-cc1" "-internal-isystem" "d:\netbatch\build_comp02320_00\ics_top\builds\xmainoclx86win_prod\llvm\lib\clang\15.0.0\include" "-nostdsysteminc" "-emit-llvm" "-debug-info-kind=limited" "D:\netbatch\build_comp02320_00\ics_top\llvm\clang\test\CodeGenCXX\no_auto_return_lambda.cpp" "-o" "-"
$ "d:\netbatch\build_comp02320_00\ics_top\builds\xmainoclx86win_prod\llvm\bin\filecheck.exe" "D:\netbatch\build_comp02320_00\ics_top\llvm\clang\test\CodeGenCXX\no_auto_return_lambda.cpp"
# command stderr:
D:\netbatch\build_comp02320_00\ics_top\llvm\clang\test\CodeGenCXX\no_auto_return_lambda.cpp:24:11: error: CHECK: expected string not found in input
// CHECK: ![[FUN_TYPE_LAMBDA]] = !DISubroutineType(types: ![[TYPE_NODE_LAMBDA:[0-9]+]])
          ^
<stdin>:56:289: note: scanning from here
!17 = distinct !DISubprogram(name: "operator()", linkageName: "??R<lambda_0>@?0??g@@YAHXZ@QBE?A?<auto>@@XZ", scope: !13, file: !7, line: 9, type: !18, scopeLine: 9, flags: DIFlagPrototyped, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition, unit: !0, declaration: !22, retainedNodes: !11)                                                                                                   ^
<stdin>:56:289: note: with "FUN_TYPE_LAMBDA" equal to "18"
!17 = distinct !DISubprogram(name: "operator()", linkageName: "??R<lambda_0>@?0??g@@YAHXZ@QBE?A?<auto>@@XZ", scope: !13, file: !7, line: 9, type: !18, scopeLine: 9, flags: DIFlagPrototyped, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition, unit: !0, declaration: !22, retainedNodes: !11)                                                                                                   ^
<stdin>:57:1: note: possible intended match here
!18 = !DISubroutineType(cc: DW_CC_BORLAND_thiscall, types: !19)
^Input file: <stdin>
Check file: D:\netbatch\build_comp02320_00\ics_top\llvm\clang\test\CodeGenCXX\no_auto_return_lambda.cpp
-dump-input=help explains the following input dump.Input was:
<<<<<<
            .
            .
            .
           51: !12 = !DILocalVariable(name: "f", scope: !6, file: !7, line: 9, type: !13)
           52: !13 = distinct !DICompositeType(tag: DW_TAG_class_type, scope: !6, file: !7, line: 9, size: 8, flags: DIFlagTypePassByValue | DIFlagNonTrivial, elements: !11)
           53: !14 = !DILocation(line: 9, column: 8, scope: !6)
           54: !15 = !DILocation(line: 10, column: 10, scope: !6)
           55: !16 = !DILocation(line: 10, column: 3, scope: !6)
           56: !17 = distinct !DISubprogram(name: "operator()", linkageName: "??R<lambda_0>@?0??g@@YAHXZ@QBE?A?<auto>@@XZ", scope: !13, file: !7, line: 9, type: !18, scopeLine: 9, flags: DIFlagPrototyped, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition, unit: !0, declaration: !22, retainedNodes: !11)
check:24'0                                                                                                                                                                                                                                                                                                     X error: no match found
check:24'1                                                                                                                                                                                                                                                                                                       with "FUN_TYPE_LAMBDA" equal to "18"
           57: !18 = !DISubroutineType(cc: DW_CC_BORLAND_thiscall, types: !19)
check:24'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
check:24'2     ?                                                                possible intended match
           58: !19 = !{!10, !20}
check:24'0     ~~~~~~~~~~~~~~~~~~
           59: !20 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !21, size: 32, flags: DIFlagArtificial | DIFlagObjectPointer)
check:24'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           60: !21 = !DIDerivedType(tag: DW_TAG_const_type, baseType: !13)
check:24'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           61: !22 = !DISubprogram(name: "operator()", scope: !13, file: !7, line: 9, type: !18, scopeLine: 9, flags: DIFlagPublic | DIFlagPrototyped, spFlags: DISPFlagLocalToUnit)
check:24'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           62: !23 = !DILocalVariable(name: "this", arg: 1, scope: !17, type: !24, flags: DIFlagArtificial | DIFlagObjectPointer)
check:24'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            .
            .
            .
>>>>>>error: command failed with exit status: 1--********************
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..1 warning(s) in tests
********************
Failed Tests (1): 
Clang :: CodeGenCXX/no_auto_return_lambda.cpp

See the failure on the check of line 23, which needs a calling-convention (since members are default-this-call in 32 bit windows).

I ALSO note that the line you match against "TYPE_NODE" ends up having 2 things on it when you are checking against '1'.

25

Did you mean the above?

I note that defining the INT_TYPE variable here results in it accepting ANYTHING for TYPE_NODE_LAMBDA

[erichkeane]

clang/test/CodeGenCXX/no_auto_return_lambda.cpp

1

disregard the 2nd part about TYPE_NODE, that passes, and I was misreading.

However, the issue with FUN_TYPE_LAMBDA is still there. I repro'd it locally by adding a triple runline, and doing the following change to 'fix it'.

I just am not sure that it still properly matches the intent of your patch:

diff --git a/clang/test/CodeGenCXX/no_auto_return_lambda.cpp b/clang/test/CodeGenCXX/no_auto_return_lambda.cpp
index 76a98263f766..690dcad52826 100644
--- a/clang/test/CodeGenCXX/no_auto_return_lambda.cpp
+++ b/clang/test/CodeGenCXX/no_auto_return_lambda.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple i386-windows-pc -emit-llvm -debug-info-kind=limited %s -o - | FileCheck %s

 // We emit "auto" for deduced return types for member functions but we should
 // not emitting "auto" for deduced return types for lambdas call function which
@@ -21,5 +22,5 @@ __attribute__((used)) int g() {
 // operator() of the local lambda should have the same return type as g()
 //
 // CHECK: distinct !DISubprogram(name: "operator()",{{.*}}, type: ![[FUN_TYPE_LAMBDA:[0-9]+]],{{.*}}
-// CHECK: ![[FUN_TYPE_LAMBDA]] = !DISubroutineType(types: ![[TYPE_NODE_LAMBDA:[0-9]+]])
-// CHECK: ![[TYPE_NODE_LAMBDA]] = !{![[INT_TYPE:[0-9]+]], {{.*}}
+// CHECK: ![[FUN_TYPE_LAMBDA]] = !DISubroutineType({{.*}}types: ![[TYPE_NODE_LAMBDA:[0-9]+]])
+// CHECK: ![[TYPE_NODE_LAMBDA]] = !{![[INT_TYPE]], {{.*}}

Additionally, I changed the last 'match' block for INT_TYPE, which seems to pass in both configs as well. However, I didn't want to commit it unless you confirmed that it is the 'right' thing.

[dblaikie]

('scuse the delay)

Baseline: I'm still not really sure this is the right direction. Is there a sound argument for why this change is suitable for lambdas, but not for other types? I believe all the situations that can happen with other types can happen with lambdas (& the other way around) with sufficiently interestingly crafted inputs.

In D123319#3439146, @probinson wrote:

In D123319#3437250, @dblaikie wrote:

(@probinson as someone I've disagreed with about this before)

Personally I think there's limited value in expressing 'auto' in DWARF at all - we could omit function declarations if the return type is not known (undeduced auto) and wouldn't lose much - basically treating them the same as templates that aren't instantiated yet.

The problem there is that then the CU where the function is defined is the only one where the type description would include that function; and if we are doing e.g. ctor homing, and that CU doesn't happen to construct any instances, then the function will still exist but not be described anywhere.

Type homing in the non-homed CUs works more like how your private Sony minimizing feature works - the member functions are included in those translation units where they're defined (eg: if you have a type homed in one CU, but a member function template instantiated in another - you get a declaration of the type with a member function declared over in that second CU) - so it is described.

If you're not doing ctor homing, then only one CU's description will mention the auto function; so, whether the debugger knows about the auto function will randomly depend on which CU's description the debugger decided to keep.

True - though in CUs without the definition of the function instantiated - it's of limited value to know the function exists. Unlike an uninstantiated member function template (which we just can't adequately describe in DWARF), at least knowing the auto-returning function exists, what it's name is, and what its parameters are - would allow a consumer to include it when performing overload resolution. But once resolved, the consumer couldn't produce a call to the function - without knowing the return type you couldn't implement the necessary calling convention (eg: it could depend on whether the type is non-trivially copyable, which you couldn't know).

I do understand the analogy to templates. The difference I see is that DWARF actually has a way to describe auto function declarations, where it does not have a way to describe template declarations.

Describing a function without its return type limits the value - not zero value, but not hugely valuable either.

(whereas a function where you know the return type (and the parameters) you might be able to determine the mangled name of the function and produce a valid call to the function - even if the function's definition has no DWARF)

(& I believe Sony does this for all functions anyway - only including them when they're defined, not including an exhaustive list of member functions in class definitions)

We have a private feature that (if you're not doing something like ctor homing, or using type units) will suppress mention of _unused_ functions, which works for us because our debugger will merge type descriptions from different units. Most debuggers don't do that, generally keeping just whichever one they found first, and assuming the rest are duplicates. Including auto functions only when defined means the various descriptions aren't actually duplicates, and so the debugger will randomly know or not know about the function depending on which description it picks.

It's true that omitting auto function declarations wouldn't affect Sony because we know how to merge descriptions anyway, but OTOH we never upstreamed the suppress-unused-declarations feature because AFAIK no other debugger understands how to handle the situation. Deliberately introducing omitted declarations seems kinda wrong. With templates we don't have a choice, but with auto functions we do.

I mean, if you're willing to omit auto functions, why wouldn't you be willing to omit other unused functions? The argument that "we don't know the whole type" seems a bit weak, given that DWARF gives you a way to describe what's missing.

I think the major difference is a consumer can call a function if it knows the whole signature, it can't if it doesn't know the return value. (at least on any ABI I can think of - maybe there are some ABIs where not knowing the return type would still be viable to call)

[shafik]

clang/test/CodeGenCXX/no_auto_return_lambda.cpp

1

Thank you for catching, I was trying to be too specific, your change looks good but I think you can leave off the triple. If it still causes you issues w/o the triple then use %itanium_abi_triple

25

No, that was a mistake, your fix above is correct.

[shafik]

In D123319#3473283, @dblaikie wrote:

('scuse the delay)

Baseline: I'm still not really sure this is the right direction. Is there a sound argument for why this change is suitable for lambdas, but not for other types? I believe all the situations that can happen with other types can happen with lambdas (& the other way around) with sufficiently interestingly crafted inputs.

I had a couple of approaches but once I saw how gcc was handling it, I just went with consistency with gcc. I might have been missing some cases but I did not have other test case that I ran into issues with.

[dblaikie]

In D123319#3473693, @shafik wrote:

In D123319#3473283, @dblaikie wrote:

('scuse the delay)

Baseline: I'm still not really sure this is the right direction. Is there a sound argument for why this change is suitable for lambdas, but not for other types? I believe all the situations that can happen with other types can happen with lambdas (& the other way around) with sufficiently interestingly crafted inputs.

I had a couple of approaches but once I saw how gcc was handling it, I just went with consistency with gcc. I might have been missing some cases but I did not have other test case that I ran into issues with.

What's the basic reproduction of the issue? Using that I can probably produce a non-lambda example that tickles the same bug & demonstrates why this should be generalized and/or fixed in lldb instead.

[dblaikie]

In D123319#3474997, @dblaikie wrote:

In D123319#3473693, @shafik wrote:

In D123319#3473283, @dblaikie wrote:

('scuse the delay)

Baseline: I'm still not really sure this is the right direction. Is there a sound argument for why this change is suitable for lambdas, but not for other types? I believe all the situations that can happen with other types can happen with lambdas (& the other way around) with sufficiently interestingly crafted inputs.

I had a couple of approaches but once I saw how gcc was handling it, I just went with consistency with gcc. I might have been missing some cases but I did not have other test case that I ran into issues with.

What's the basic reproduction of the issue? Using that I can probably produce a non-lambda example that tickles the same bug & demonstrates why this should be generalized and/or fixed in lldb instead.

Ping on this ^

[dblaikie]

In D123319#3486544, @dblaikie wrote:

In D123319#3474997, @dblaikie wrote:

In D123319#3473693, @shafik wrote:

In D123319#3473283, @dblaikie wrote:

('scuse the delay)

Baseline: I'm still not really sure this is the right direction. Is there a sound argument for why this change is suitable for lambdas, but not for other types? I believe all the situations that can happen with other types can happen with lambdas (& the other way around) with sufficiently interestingly crafted inputs.

I had a couple of approaches but once I saw how gcc was handling it, I just went with consistency with gcc. I might have been missing some cases but I did not have other test case that I ran into issues with.

What's the basic reproduction of the issue? Using that I can probably produce a non-lambda example that tickles the same bug & demonstrates why this should be generalized and/or fixed in lldb instead.

Ping on this ^

Any update/further details here?

[shafik]

Any update/further details here?

David, apologies for not getting back to you sooner. The context is D105564 which I started working on again recently. I was having difficulties finding a solution that also worked for local lambdas. I discovered eventually that what I had worked with debug-info that gcc generated and realized that gcc was not emitting auto for lambdas and decided to match gcc's behavior here since we often do that. I think an alternative approach would have been to emit DW_AT_linkage_name for local lambdas but when I dug into that it looked like we were dropping the linkage name several step before where would emit DW_AT_linkage_name and it was not clear to me how easy a fix that was.

I am happy to consider other approaches as well to solving lookup for local lambdas for D105564. Let me know what you think.

[dblaikie]

In D123319#3506745, @shafik wrote:

Any update/further details here?

David, apologies for not getting back to you sooner. The context is D105564 which I started working on again recently. I was having difficulties finding a solution that also worked for local lambdas.

But what I don't understand is what makes local lambdas special. You can produce the same DWARF (so far as I can think of - maybe there's some distinction I'm missing?) without lambdas:

void f1() {
  auto x = [](){ return 3; };
  x();
}
void f2() {
  struct t1 {
    auto operator()() {
      return 3;
    }
  };
  t1 v1;
  v1();
}

$ clang++-tot test.cpp -g -c && llvm-dwarfdump-tot test.o | grep "DW_TAG\|DW_AT_type\|DW_AT_name" | sed -e "s/^...........//"
...
   DW_TAG_subprogram
     DW_AT_name ("f1")
     DW_TAG_variable
       DW_AT_name       ("x")
       DW_AT_type       (0x0000003a "class ")
     DW_TAG_class_type
       DW_TAG_subprogram
         DW_AT_name     ("operator()")
         DW_AT_type     (0x00000050 "int")
         DW_TAG_formal_parameter
           DW_AT_type   (0x00000054 "const class  *")
...
   DW_TAG_subprogram
     DW_AT_specification        (0x0000003f "operator()")
     DW_TAG_formal_parameter
       DW_AT_name       ("this")
       DW_AT_type       (0x000000cc "const class  *")
   DW_TAG_subprogram
     DW_AT_name ("f2")
     DW_TAG_variable
       DW_AT_name       ("v1")
       DW_AT_type       (0x00000090 "t1")
     DW_TAG_structure_type
       DW_AT_name       ("t1")
       DW_TAG_subprogram
         DW_AT_name     ("operator()")
         DW_AT_type     (0x000000a6 "auto")
         DW_TAG_formal_parameter
           DW_AT_type   (0x000000a8 "t1 *")
   DW_TAG_subprogram
     DW_AT_type (0x00000050 "int")
     DW_AT_specification        (0x00000096 "operator()")
     DW_TAG_formal_parameter
       DW_AT_name       ("this")
       DW_AT_type       (0x000000d1 "t1 *")
   DW_TAG_pointer_type
     DW_AT_type (0x00000059 "const class ")
   DW_TAG_pointer_type
     DW_AT_type (0x00000090 "t1")

So if there's particularly an issue with lambdas, it's probably also with the above local type example.

I discovered eventually that what I had worked with debug-info that gcc generated

Yep, at least testing GCC 11 just now - it seems GCC doesn't use "auto" for local types, be they named or unnamed.

and realized that gcc was not emitting auto for lambdas and decided to match gcc's behavior here since we often do that.

I think that's only a really sound argument when it's GDB compatibility/somewhat outside our control on the consumer side. I'm not sure there's a good argument for doing this for lldb when we can fix lldb to handle the right DWARF instead.

I think an alternative approach would have been to emit DW_AT_linkage_name for local lambdas but when I dug into that it looked like we were dropping the linkage name several step before where would emit DW_AT_linkage_name and it was not clear to me how easy a fix that was.

What does the linkage name do for your use case? Which cases are missing linkage names/where do they go missing?

I am happy to consider other approaches as well to solving lookup for local lambdas for D105564. Let me know what you think.

Why does the return type help perform lookup? What kind of lookup?

(again, my take is that "auto" return types probably shouldn't be described at all - we should just not describe functions where we don't know their return types because they're not very useful to know about (overload resolution, yes, but then you can't call them anyway) - but that's a broader argument to make/change to make)

[dblaikie]

Ping

[dblaikie]

In D123319#3532811, @dblaikie wrote:

Ping

@aprantl thoughts on this?

[dblaikie]

In D123319#3546535, @dblaikie wrote:

In D123319#3532811, @dblaikie wrote:

Ping

@aprantl thoughts on this?

Ping

[aprantl]

In D123319#3517966, @dblaikie wrote:

In D123319#3506745, @shafik wrote:

What does the linkage name do for your use case? Which cases are missing linkage names/where do they go missing?

I am happy to consider other approaches as well to solving lookup for local lambdas for D105564. Let me know what you think.

Why does the return type help perform lookup? What kind of lookup?

(again, my take is that "auto" return types probably shouldn't be described at all - we should just not describe functions where we don't know their return types because they're not very useful to know about (overload resolution, yes, but then you can't call them anyway) - but that's a broader argument to make/change to make)

IIUC, the motivating problem is (@shafik please correct me if this isn't it) this:

$ cat /tmp/lambda.cpp 
#include <cstdio>
int main() {
  auto f = [](){ printf("hi from lambda\n"); return 1;} ;
  f();
  f();
  return f();
}
$ clang++ -g /tmp/lambda.cpp -o /tmp/a.out
$ lldb /tmp/a.out
(lldb) b 5
(lldb) r
(lldb) p f()
hi from lambda
(lldb) p auto val = f()
error: expression failed to parse:
error: <user expression 1>:1:6: variable has incomplete type 'void'
auto val = f()
     ^

LLDB can't determine the return type of the lambda, because it has trouble matching up the abstract specification (with the auto return type) with the concrete definition (with the int return type):

$ dwarfdump --name "operator()" /tmp/a.out.dSYM -p -c
/tmp/a.out.dSYM/Contents/Resources/DWARF/a.out:	file format Mach-O arm64

0x0000000b: DW_TAG_compile_unit
              DW_AT_producer	("Apple clang version 13.1.6 (clang-1316.0.21.2)")
              DW_AT_language	(DW_LANG_C_plus_plus_14)
              DW_AT_name	("/tmp/lambda.cpp")
              DW_AT_LLVM_sysroot	("/Library/Developer/CommandLineTools/SDKs/MacOSX13.0.sdk")
              DW_AT_APPLE_sdk	("MacOSX13.0.sdk")
              DW_AT_stmt_list	(0x00000000)
              DW_AT_comp_dir	("/Volumes/Data/swift")
              DW_AT_low_pc	(0x0000000100003f28)
              DW_AT_high_pc	(0x0000000100003f98)

0x000007c6:   DW_TAG_subprogram
                DW_AT_low_pc	(0x0000000100003f28)
                DW_AT_high_pc	(0x0000000100003f6c)
                DW_AT_frame_base	(DW_OP_reg29 W29)
                DW_AT_name	("main")
                DW_AT_decl_file	("/tmp/lambda.cpp")
                DW_AT_decl_line	(2)
                DW_AT_type	(0x000000000000029f "int")
                DW_AT_external	(true)

0x000007ed:     DW_TAG_class_type
                  DW_AT_calling_convention	(DW_CC_pass_by_value)
                  DW_AT_byte_size	(0x01)
                  DW_AT_decl_file	("/tmp/lambda.cpp")
                  DW_AT_decl_line	(3)

0x000007f2:       DW_TAG_subprogram
                    DW_AT_name	("operator()")
                    DW_AT_decl_file	("/tmp/lambda.cpp")
                    DW_AT_decl_line	(3)
                    DW_AT_type	(0x0000000000000806 "auto")
                    DW_AT_declaration	(true)
                    DW_AT_accessibility	(DW_ACCESS_public)

0x000007fe:         DW_TAG_formal_parameter
                      DW_AT_type	(0x000000000000080b "const class *")
                      DW_AT_artificial	(true)

0x00000803:         NULL

0x0000000b: DW_TAG_compile_unit
              DW_AT_producer	("Apple clang version 13.1.6 (clang-1316.0.21.2)")
              DW_AT_language	(DW_LANG_C_plus_plus_14)
              DW_AT_name	("/tmp/lambda.cpp")
              DW_AT_LLVM_sysroot	("/Library/Developer/CommandLineTools/SDKs/MacOSX13.0.sdk")
              DW_AT_APPLE_sdk	("MacOSX13.0.sdk")
              DW_AT_stmt_list	(0x00000000)
              DW_AT_comp_dir	("/Volumes/Data/swift")
              DW_AT_low_pc	(0x0000000100003f28)
              DW_AT_high_pc	(0x0000000100003f98)

0x00000815:   DW_TAG_subprogram
                DW_AT_low_pc	(0x0000000100003f6c)
                DW_AT_high_pc	(0x0000000100003f98)
                DW_AT_frame_base	(DW_OP_reg29 W29)
                DW_AT_object_pointer	(0x00000834)
                DW_AT_type	(0x000000000000029f "int")
                DW_AT_linkage_name	("_ZZ4mainENK3$_0clEv")
                DW_AT_specification	(0x00000000000007f2 "operator()")

0x00000834:     DW_TAG_formal_parameter
                  DW_AT_location	(DW_OP_breg31 WSP+8)
                  DW_AT_name	("this")
                  DW_AT_type	(0x0000000000000841 "const class *")
                  DW_AT_artificial	(true)

0x00000840:     NULL

[dblaikie]

In D123319#3593644, @aprantl wrote:

In D123319#3517966, @dblaikie wrote:

In D123319#3506745, @shafik wrote:

What does the linkage name do for your use case? Which cases are missing linkage names/where do they go missing?

I am happy to consider other approaches as well to solving lookup for local lambdas for D105564. Let me know what you think.

Why does the return type help perform lookup? What kind of lookup?

(again, my take is that "auto" return types probably shouldn't be described at all - we should just not describe functions where we don't know their return types because they're not very useful to know about (overload resolution, yes, but then you can't call them anyway) - but that's a broader argument to make/change to make)

IIUC, the motivating problem is (@shafik please correct me if this isn't it) this:

$ cat /tmp/lambda.cpp 
#include <cstdio>
int main() {
  auto f = [](){ printf("hi from lambda\n"); return 1;} ;
  f();
  f();
  return f();
}
$ clang++ -g /tmp/lambda.cpp -o /tmp/a.out
$ lldb /tmp/a.out
(lldb) b 5
(lldb) r
(lldb) p f()
hi from lambda
(lldb) p auto val = f()
error: expression failed to parse:
error: <user expression 1>:1:6: variable has incomplete type 'void'
auto val = f()
     ^

OK. Thanks - I can reproduce this. Though it seems like the issue isn't about lambdas - but about member functions in general, lambdas or not:

$ cat test.cpp
#include <cstdio>
auto g = []() {
  printf("hi from non-local lambda\n");
  return 1;
};
struct t2 {
  auto operator()() {
    printf("hi from non-local type\n");
    return 1;
  }
  auto func() {
    printf("hi from non-local named func\n");
    return 1;
  }
};
int main() {
  auto f = []() {
    printf("hi from lambda\n");
    return 1;
  };
  struct t1 {
    auto operator()() {
      printf("hi from local type\n");
      return 1;
    }
  };
  f();
  g();
  t1 v1;
  v1();
  t2 v2;
  v2();
  v2.func();
}
$ lldb ./a.out
(lldb) p f()
hi from lambda
(int) $0 = 1
(lldb) p g()
hi from non-local lambda
(int) $1 = 1
(lldb) p v1()
hi from local type
(lldb) p v2()
hi from non-local type
(lldb) p v2.func()
hi from non-local named func
(lldb) p auto x = f()
hi from lambda
(lldb) p auto x = g()
hi from non-local lambda
(lldb) p auto x = v1()
error: expression failed to parse:
error: <user expression 7>:1:6: variable has incomplete type 'void'
auto x = v1()
     ^
(lldb) p auto x = v2()
error: expression failed to parse:
error: <user expression 8>:1:6: variable has incomplete type 'void'
auto x = v2()
     ^
(lldb) p auto x = v2.func()
error: expression failed to parse:
error: <user expression 9>:1:6: variable has incomplete type 'void'
auto x = v2.func()
     ^

(but a standalone auto function doesn't have the problem... because we don't produce declarations for those functions (even in a namespace, we just define them in the namespace)

So I think this is a problem with lldb's handling of auto return in general - not with lambdas in particular.

And I still have two related questions

1. Why is Clang being changed here, rather than fixing lldb to handle correct DWARF?
2. Alternatively: why are we emitting 'auto' return types at all (see previous comments/discussion) instead of treating these functions the same way we do for member function templates - omit them entirely (don't even emit declarations) if the return type isn't known.

[dblaikie]

ping

[probinson]

Seems like for an "auto"-returning function/lambda with a definition, the front-end should have deduced the return type, and so we should emit that in the DWARF, even if we end up emitting DWARF with both a declaration and a separate definition.

I accept that a member function declaration (no definition) returning "auto" is not hugely useful, although I reserve the right to make pedantic grousing noises. :)

[dblaikie]

@aprantl ^ ?

[dblaikie]

Ping

[dblaikie]

Ping

[aprantl]

Sorry for the silence — I was out on vacation.

@Michael137 has recently started looking into improving support for C++ lambdas in LLDB.
Michael, would you be interested in taking a fresh look at this and figure out what the requirements for LLDB are here and how to answer the questions @dblaikie raised specifically?

[Michael137]

In D123319#3680169, @aprantl wrote:

Sorry for the silence — I was out on vacation.

@Michael137 has recently started looking into improving support for C++ lambdas in LLDB.
Michael, would you be interested in taking a fresh look at this and figure out what the requirements for LLDB are here and how to answer the questions @dblaikie raised specifically?

Sure will have a look at what's missing from https://reviews.llvm.org/D105564 and whether these clang changes are indeed necessary

[dblaikie]

In D123319#3681884, @Michael137 wrote:

In D123319#3680169, @aprantl wrote:

Sorry for the silence — I was out on vacation.

@Michael137 has recently started looking into improving support for C++ lambdas in LLDB.
Michael, would you be interested in taking a fresh look at this and figure out what the requirements for LLDB are here and how to answer the questions @dblaikie raised specifically?

Sure will have a look at what's missing from https://reviews.llvm.org/D105564 and whether these clang changes are indeed necessary

Ah, the plan is to fix lldb and then revert this clang patch?

[Michael137]

In D123319#3682929, @dblaikie wrote:

In D123319#3681884, @Michael137 wrote:

In D123319#3680169, @aprantl wrote:

Sorry for the silence — I was out on vacation.

@Michael137 has recently started looking into improving support for C++ lambdas in LLDB.
Michael, would you be interested in taking a fresh look at this and figure out what the requirements for LLDB are here and how to answer the questions @dblaikie raised specifically?

Sure will have a look at what's missing from https://reviews.llvm.org/D105564 and whether these clang changes are indeed necessary

Ah, the plan is to fix lldb and then revert this clang patch?

At least will want to see why this wasn't easy to fix on the LLDB side and whether it ties into some of the other lambda bugs floating around

[dblaikie]

Any update on this? (& it seems like maybe the "why use auto at all" question got lost? Might be worth engaging in that question - if we decide it's not worth using then maybe we don't need any lldb fixes and can remove the functionality from clang entirely? (though I guess maybe lldb wants fixing for all the code already being compiled with shipping versions of clang that are using auto?))

[Michael137]

FYI, had an offline chat with @dblaikie and we decided that the best way forward would be to try stop emitting auto return types in DWARF, instead of relying on lambda/member-function specific hacks just for LLDB. Not sure what the timeline on this will be but tracking it internally

[dblaikie]

In D123319#3723648, @Michael137 wrote:

FYI, had an offline chat with @dblaikie and we decided that the best way forward would be to try stop emitting auto return types in DWARF, instead of relying on lambda/member-function specific hacks just for LLDB. Not sure what the timeline on this will be but tracking it internally

Posted D131933 to revert this and the original (D70524) implementation of the auto return functionality.

clang/lib/CodeGen/CGDebugInfo.cpp

const auto *OriginalFunc = cast<llvm::DISubroutineType>(
                           Func->getReturnType(), Func->getParamTypes(), EPI),
                       Unit));
   llvm::DITypeRefArray Args = OriginalFunc->getTypeArray();
   assert(Args.size() && "Invalid number of arguments!");
 
   SmallVector<llvm::Metadata *, 16> Elts;
   // First element is always return type. For 'void' functions it is NULL.
   QualType temp = Func->getReturnType();
-  if (temp->getTypeClass() == Type::Auto && decl)
-    Elts.push_back(CreateType(cast<AutoType>(temp)));
+  if (temp->getTypeClass() == Type::Auto && decl) {
+    const AutoType *AT = cast<AutoType>(temp);
+
+    // It may be tricky in some cases to link the specification back the lambda
+    // call operator and so we skip emitting "auto" for lambdas. This is
+    // consistent with gcc as well.
+    if (AT->isDeduced() && ThisPtr->getPointeeCXXRecordDecl()->isLambda())
+      Elts.push_back(getOrCreateType(AT->getDeducedType(), Unit));
     else
+      Elts.push_back(CreateType(AT));
+  } else
     Elts.push_back(Args[0]);
 
   // "this" pointer is always first argument.
   const CXXRecordDecl *RD = ThisPtr->getPointeeCXXRecordDecl();
   if (isa<ClassTemplateSpecializationDecl>(RD)) {
     // Create pointer type directly in this case.
     const PointerType *ThisPtrTy = cast<PointerType>(ThisPtr);
     QualType PointeeTy = ThisPtrTy->getPointeeType();

clang/test/CodeGenCXX/no_auto_return_lambda.cpp

+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited %s -o - | FileCheck %s
+
+// We emit "auto" for deduced return types for member functions but we should
+// not emitting "auto" for deduced return types for lambdas call function which
+// will be implmented as operator() in a class type. This test will verify that
+// behavior.
+
+__attribute__((used)) int g() {
+  auto f = []() { return 10; };
+  return f();
+}
+
+// g() is not a member function so we should not emit "auto" for the deduced
+// return type.
+//
+// CHECK: !DISubprogram(name: "g",{{.*}}, type: ![[FUN_TYPE:[0-9]+]],{{.*}}
+// CHECK: ![[FUN_TYPE]] = !DISubroutineType(types: ![[TYPE_NODE:[0-9]+]])
+// CHECK: ![[TYPE_NODE]] = !{![[INT_TYPE:[0-9]+]]}
+// CHECK: ![[INT_TYPE]] = !DIBasicType(name: "int", {{.*}})
+
+// operator() of the local lambda should have the same return type as g()
+//
+// CHECK: distinct !DISubprogram(name: "operator()",{{.*}}, type: ![[FUN_TYPE_LAMBDA:[0-9]+]],{{.*}}
+// CHECK: ![[FUN_TYPE_LAMBDA]] = !DISubroutineType(types: ![[TYPE_NODE_LAMBDA:[0-9]+]])
+// CHECK: ![[TYPE_NODE_LAMBDA]] = !{![[INT_TYPE:[0-9]+]], {{.*}}