[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123319/new/

https://reviews.llvm.org/D123319

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

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[Michael Buch via Phabricator via cfe-commits]

Michael137 added a comment.

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. Not sure what the timeline on 
this will be but tracking it internally


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123319/new/

https://reviews.llvm.org/D123319

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

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

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?))


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123319/new/

https://reviews.llvm.org/D123319

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

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[Michael Buch via Phabricator via cfe-commits]

Michael137 added a comment.

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 in to some of the other lambda bugs floating around


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123319/new/

https://reviews.llvm.org/D123319

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

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

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?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123319/new/

https://reviews.llvm.org/D123319

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

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[Michael Buch via Phabricator via cfe-commits]

Michael137 added a comment.

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 what's missing from https://reviews.llvm.org/D105564 and 
whether these clang changes are indeed necessary


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123319/new/

https://reviews.llvm.org/D123319

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

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a subscriber: Michael137.
aprantl added a comment.

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?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123319/new/

https://reviews.llvm.org/D123319

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

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123319/new/

https://reviews.llvm.org/D123319

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

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123319/new/

https://reviews.llvm.org/D123319

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

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

@aprantl ^ ?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123319/new/

https://reviews.llvm.org/D123319

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

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

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. :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123319/new/

https://reviews.llvm.org/D123319

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

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

ping


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123319/new/

https://reviews.llvm.org/D123319

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

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

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 
>   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: :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 
  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: :1:6: variable has incomplete type 'void'
  auto x = v1()
   ^
  (lldb) p auto x = v2()
  error: expression failed to parse:
  error: :1:6: variable has incomplete type 'void'
  auto x = v2()
   ^
  (lldb) p auto x = v2.func()
  error: expression failed to parse:
  error: :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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123319/new/

https://reviews.llvm.org/D123319

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

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

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 
  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: :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
  
  0x000b: 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 (0x)
DW_AT_comp_dir  ("/Volumes/Data/swift")
DW_AT_low_pc(0x00013f28)
DW_AT_high_pc   (0x00013f98)
  
  0x07c6:   DW_TAG_subprogram
  DW_AT_low_pc  (0x00013f28)
  DW_AT_high_pc (0x00013f6c)
  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(0x029f "int")
  DW_AT_external(true)
  
  0x07ed: 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)
  
  0x07f2:   DW_TAG_subprogram
  DW_AT_name("operator()")
  DW_AT_decl_file   ("/tmp/lambda.cpp")
  DW_AT_decl_line   (3)
  DW_AT_type(0x0806 "auto")
  DW_AT_declaration (true)
  DW_AT_accessibility   (DW_ACCESS_public)
  
  0x07fe: DW_TAG_formal_parameter
DW_AT_type  (0x080b "const class *")
DW_AT_artificial(true)
  
  0x0803: NULL
  
  0x000b: 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 (0x)
DW_AT_comp_dir  ("/Volumes/Data/swift")
DW_AT_low_pc(0x00013f28)
DW_AT_high_pc   (0x00013f98)
  
  0x0815:   DW_TAG_subprogram
  DW_AT_low_pc  (0x00013f6c)
  DW_AT_high_pc (0x00013f98)
  DW_AT_frame_base  (DW_OP_reg29 W29)
  DW_AT_object_pointer  (0x0834)
  DW_AT_type(0x029f "int")
  DW_AT_linkage_name("_ZZ4mainENK3$_0clEv")
  DW_AT_specification   (0x07f2 "operator()")
  
  0x0834: DW_TAG_formal_parameter
DW_AT_location  (DW_OP_breg31 WSP+8)
DW_AT_name  ("this")
DW_AT_type  (0x0841 "const class *")
DW_AT_artificial(true)
  
  0x0840: NULL


Repository:
  rG LLVM Github 

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D123319#3546535 , @dblaikie wrote:

> In D123319#3532811 , @dblaikie 
> wrote:
>
>> Ping
>
> @aprantl thoughts on this?

Ping


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123319/new/

https://reviews.llvm.org/D123319

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

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D123319#3532811 , @dblaikie wrote:

> Ping

@aprantl thoughts on this?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123319/new/

https://reviews.llvm.org/D123319

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

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123319/new/

https://reviews.llvm.org/D123319

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

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

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   (0x003a "class ")
   DW_TAG_class_type
 DW_TAG_subprogram
   DW_AT_name ("operator()")
   DW_AT_type (0x0050 "int")
   DW_TAG_formal_parameter
 DW_AT_type   (0x0054 "const class  *")
  ...
 DW_TAG_subprogram
   DW_AT_specification(0x003f "operator()")
   DW_TAG_formal_parameter
 DW_AT_name   ("this")
 DW_AT_type   (0x00cc "const class  *")
 DW_TAG_subprogram
   DW_AT_name ("f2")
   DW_TAG_variable
 DW_AT_name   ("v1")
 DW_AT_type   (0x0090 "t1")
   DW_TAG_structure_type
 DW_AT_name   ("t1")
 DW_TAG_subprogram
   DW_AT_name ("operator()")
   DW_AT_type (0x00a6 "auto")
   DW_TAG_formal_parameter
 DW_AT_type   (0x00a8 "t1 *")
 DW_TAG_subprogram
   DW_AT_type (0x0050 "int")
   DW_AT_specification(0x0096 "operator()")
   DW_TAG_formal_parameter
 DW_AT_name   ("this")
 DW_AT_type   (0x00d1 "t1 *")
 DW_TAG_pointer_type
   DW_AT_type (0x0059 "const class ")
 DW_TAG_pointer_type
   DW_AT_type (0x0090 "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)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123319/new/

https://reviews.llvm.org/D123319

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

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

> 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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123319/new/

https://reviews.llvm.org/D123319

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

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

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 ^


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123319/new/

https://reviews.llvm.org/D123319

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

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123319/new/

https://reviews.llvm.org/D123319

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

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123319/new/

https://reviews.llvm.org/D123319

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

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: clang/test/CodeGenCXX/no_auto_return_lambda.cpp:1
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited %s -o - | FileCheck %s
+

erichkeane wrote:
> erichkeane wrote:
> > 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]+]])
> >   ^
> > :56:289: note: scanning from here
> > !17 = distinct !DISubprogram(name: "operator()", linkageName: 
> > "??R@?0??g@@YAHXZ@QBE?A?@@XZ", scope: !13, file: !7, line: 
> > 9, type: !18, scopeLine: 9, flags: DIFlagPrototyped, spFlags: 
> > DISPFlagLocalToUnit | DISPFlagDefinition, unit: !0, declaration: !22, 
> > retainedNodes: !11) 
> >   ^
> > :56:289: note: with "FUN_TYPE_LAMBDA" equal to "18"
> > !17 = distinct !DISubprogram(name: "operator()", linkageName: 
> > "??R@?0??g@@YAHXZ@QBE?A?@@XZ", scope: !13, file: !7, line: 
> > 9, type: !18, scopeLine: 9, flags: DIFlagPrototyped, spFlags: 
> > DISPFlagLocalToUnit | DISPFlagDefinition, unit: !0, declaration: !22, 
> > retainedNodes: !11) 
> >   ^
> > :57:1: note: possible intended match here
> > !18 = !DISubroutineType(cc: DW_CC_BORLAND_thiscall, types: !19)
> > ^Input file: 
> > 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@?0??g@@YAHXZ@QBE?A?@@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"
> >   

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

('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)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123319/new/

https://reviews.llvm.org/D123319

___
cfe-commits 

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/test/CodeGenCXX/no_auto_return_lambda.cpp:1
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited %s -o - | FileCheck %s
+

erichkeane wrote:
> 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]+]])
>   ^
> :56:289: note: scanning from here
> !17 = distinct !DISubprogram(name: "operator()", linkageName: 
> "??R@?0??g@@YAHXZ@QBE?A?@@XZ", scope: !13, file: !7, line: 9, 
> type: !18, scopeLine: 9, flags: DIFlagPrototyped, spFlags: 
> DISPFlagLocalToUnit | DISPFlagDefinition, unit: !0, declaration: !22, 
> retainedNodes: !11)   
> ^
> :56:289: note: with "FUN_TYPE_LAMBDA" equal to "18"
> !17 = distinct !DISubprogram(name: "operator()", linkageName: 
> "??R@?0??g@@YAHXZ@QBE?A?@@XZ", scope: !13, file: !7, line: 9, 
> type: !18, scopeLine: 9, flags: DIFlagPrototyped, spFlags: 
> DISPFlagLocalToUnit | DISPFlagDefinition, unit: !0, declaration: !22, 
> retainedNodes: !11)   
> ^
> :57:1: note: possible intended match here
> !18 = !DISubroutineType(cc: DW_CC_BORLAND_thiscall, types: !19)
> ^Input file: 
> 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@?0??g@@YAHXZ@QBE?A?@@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 ?

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/test/CodeGenCXX/no_auto_return_lambda.cpp:1
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited %s -o - | FileCheck %s
+

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]+]])
  ^
:56:289: note: scanning from here
!17 = distinct !DISubprogram(name: "operator()", linkageName: 
"??R@?0??g@@YAHXZ@QBE?A?@@XZ", scope: !13, file: !7, line: 9, 
type: !18, scopeLine: 9, flags: DIFlagPrototyped, spFlags: DISPFlagLocalToUnit 
| DISPFlagDefinition, unit: !0, declaration: !22, retainedNodes: !11)   

^
:56:289: note: with "FUN_TYPE_LAMBDA" equal to "18"
!17 = distinct !DISubprogram(name: "operator()", linkageName: 
"??R@?0??g@@YAHXZ@QBE?A?@@XZ", scope: !13, file: !7, line: 9, 
type: !18, scopeLine: 9, flags: DIFlagPrototyped, spFlags: DISPFlagLocalToUnit 
| DISPFlagDefinition, unit: !0, declaration: !22, retainedNodes: !11)   

^
:57:1: note: possible intended match here
!18 = !DISubroutineType(cc: DW_CC_BORLAND_thiscall, types: !19)
^Input file: 
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@?0??g@@YAHXZ@QBE?A?@@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: 

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[Shafik Yaghmour via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5ff992bca208: [DEBUG-INFO] Change how we handle auto return 
types for lambda operator() to be… (authored by shafik).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123319/new/

https://reviews.llvm.org/D123319

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/no_auto_return_lambda.cpp


Index: clang/test/CodeGenCXX/no_auto_return_lambda.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/no_auto_return_lambda.cpp
@@ -0,0 +1,25 @@
+// 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]+]], {{.*}}
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1679,9 +1679,17 @@
   SmallVector 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(temp)));
-  else
+  if (temp->getTypeClass() == Type::Auto && decl) {
+const AutoType *AT = cast(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.


Index: clang/test/CodeGenCXX/no_auto_return_lambda.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/no_auto_return_lambda.cpp
@@ -0,0 +1,25 @@
+// 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]+]], {{.*}}
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1679,9 +1679,17 @@
   SmallVector 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(temp)));
-  else
+  if (temp->getTypeClass() == Type::Auto && decl) {
+const AutoType *AT = cast(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() && 

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik updated this revision to Diff 424300.
shafik added a comment.

-Replacing CHECK-NEXT with CHECK


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123319/new/

https://reviews.llvm.org/D123319

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/no_auto_return_lambda.cpp


Index: clang/test/CodeGenCXX/no_auto_return_lambda.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/no_auto_return_lambda.cpp
@@ -0,0 +1,25 @@
+// 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]+]], {{.*}}
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1679,9 +1679,17 @@
   SmallVector 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(temp)));
-  else
+  if (temp->getTypeClass() == Type::Auto && decl) {
+const AutoType *AT = cast(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.


Index: clang/test/CodeGenCXX/no_auto_return_lambda.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/no_auto_return_lambda.cpp
@@ -0,0 +1,25 @@
+// 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]+]], {{.*}}
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1679,9 +1679,17 @@
   SmallVector 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(temp)));
-  else
+  if (temp->getTypeClass() == Type::Auto && decl) {
+const AutoType *AT = cast(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 

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[Adrian Prantl via Phabricator via cfe-commits]

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

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.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123319/new/

https://reviews.llvm.org/D123319

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

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik updated this revision to Diff 423471.
shafik marked 4 inline comments as done.
shafik added a comment.

- Making test more robust
- Adding comments


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123319/new/

https://reviews.llvm.org/D123319

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/no_auto_return_lambda.cpp


Index: clang/test/CodeGenCXX/no_auto_return_lambda.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/no_auto_return_lambda.cpp
@@ -0,0 +1,25 @@
+// 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-NEXT: ![[TYPE_NODE]] = !{![[INT_TYPE:[0-9]+]]}
+// CHECK-NEXT: ![[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-NEXT: ![[TYPE_NODE_LAMBDA]] = !{![[INT_TYPE:[0-9]+]], {{.*}}
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1678,9 +1678,17 @@
   SmallVector 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(temp)));
-  else
+  if (temp->getTypeClass() == Type::Auto && decl) {
+const AutoType *AT = cast(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.


Index: clang/test/CodeGenCXX/no_auto_return_lambda.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/no_auto_return_lambda.cpp
@@ -0,0 +1,25 @@
+// 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-NEXT: ![[TYPE_NODE]] = !{![[INT_TYPE:[0-9]+]]}
+// CHECK-NEXT: ![[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-NEXT: ![[TYPE_NODE_LAMBDA]] = !{![[INT_TYPE:[0-9]+]], {{.*}}
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1678,9 +1678,17 @@
   SmallVector 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(temp)));
-  else
+  if (temp->getTypeClass() == Type::Auto && decl) {
+const AutoType *AT = cast(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));

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: clang/test/CodeGenCXX/no_auto_return_lambda.cpp:8
+
+// CHECK: !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+// CHECK-NOT: !DIBasicType(tag: DW_TAG_unspecified_type, name: "auto")

aprantl wrote:
> 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.
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?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123319/new/

https://reviews.llvm.org/D123319

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

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1684
+
+if (AT->isDeduced() && ThisPtr->getPointeeCXXRecordDecl()->isLambda())
+  Elts.push_back(getOrCreateType(AT->getDeducedType(),Unit));

aprantl wrote:
> Can you add a comment here, explaining why lambdas are special?
this is still missing ^



Comment at: clang/test/CodeGenCXX/no_auto_return_lambda.cpp:1
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited %s -o - | FileCheck %s
+

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.`



Comment at: clang/test/CodeGenCXX/no_auto_return_lambda.cpp:8
+
+// CHECK: !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+// CHECK-NOT: !DIBasicType(tag: DW_TAG_unspecified_type, name: "auto")

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.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123319/new/

https://reviews.llvm.org/D123319

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

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik updated this revision to Diff 422081.
shafik added a comment.

Adding test case


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123319/new/

https://reviews.llvm.org/D123319

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/no_auto_return_lambda.cpp


Index: clang/test/CodeGenCXX/no_auto_return_lambda.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/no_auto_return_lambda.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited %s -o - | FileCheck %s
+
+__attribute__((used)) int g() {
+  auto f = []() { return 10; };
+  return f();
+}
+
+// CHECK: !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+// CHECK-NOT: !DIBasicType(tag: DW_TAG_unspecified_type, name: "auto")
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1678,9 +1678,14 @@
   SmallVector 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(temp)));
-  else
+  if (temp->getTypeClass() == Type::Auto && decl) {
+const AutoType *AT = cast(temp);
+
+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.


Index: clang/test/CodeGenCXX/no_auto_return_lambda.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/no_auto_return_lambda.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited %s -o - | FileCheck %s
+
+__attribute__((used)) int g() {
+  auto f = []() { return 10; };
+  return f();
+}
+
+// CHECK: !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+// CHECK-NOT: !DIBasicType(tag: DW_TAG_unspecified_type, name: "auto")
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1678,9 +1678,14 @@
   SmallVector 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(temp)));
-  else
+  if (temp->getTypeClass() == Type::Auto && decl) {
+const AutoType *AT = cast(temp);
+
+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.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

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.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123319/new/

https://reviews.llvm.org/D123319

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

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

(@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


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123319/new/

https://reviews.llvm.org/D123319

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

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

> 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.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123319/new/

https://reviews.llvm.org/D123319

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

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

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




Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1684
+
+if (AT->isDeduced() && ThisPtr->getPointeeCXXRecordDecl()->isLambda())
+  Elts.push_back(getOrCreateType(AT->getDeducedType(),Unit));

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


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123319/new/

https://reviews.llvm.org/D123319

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

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

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?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123319/new/

https://reviews.llvm.org/D123319

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

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik created this revision.
shafik added reviewers: aprantl, dblaikie, probinson.
Herald added a project: All.
shafik requested review of this revision.

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.


https://reviews.llvm.org/D123319

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp


Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1678,8 +1678,14 @@
   SmallVector 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(temp)));
+  if (temp->getTypeClass() == Type::Auto && decl) {
+const AutoType *AT = cast(temp);
+
+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]);
 


Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1678,8 +1678,14 @@
   SmallVector 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(temp)));
+  if (temp->getTypeClass() == Type::Auto && decl) {
+const AutoType *AT = cast(temp);
+
+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]);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits