[PATCH] D19708: [CGDebugInfo] Generate debug info for member calls in the context of the callee expression

[David Blaikie via cfe-commits]

As mentioned in the bug, I /think/ the right thing to do here is to change
the preferred location of the CXXMemberCallExpr so that we improve
diagnostics as well. Any place where the preferred location of an
expression and the debug location of an expression are differing I'd really
like a pretty deep discussion of why those two uses cases should differ.

Eg, where this location turns up in diagnostics:

blaikie at blaikie-linux:~/dev$ cat loc.cpp
struct foo {
  const foo *x() const;
  void y();
};

void f(const foo *g) {
  g->x()->y();
  g->x()->x()->y();
}
blaikie at blaikie-linux:~/dev$ clang++-tot loc.cpp -fsyntax-only
loc.cpp:7:3: error: member function 'y' not viable: 'this' argument
has type 'const foo', but function is not marked const
  g->x()->y();
  ^~~~~~
...
loc.cpp:8:3: error: member function 'y' not viable: 'this' argument
has type 'const foo', but function is not marked const
  g->x()->x()->y();
  ^~~~~~~~~~~
...

It seems like pointing to the 'y' in both these cases would be an
improvement? The source range highlighting the 'this' part of the
expression is certainly helpful, but could still be confusing if the
functions had more similar/the same name, etc.



On Thu, Apr 28, 2016 at 8:42 PM, Hal Finkel via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> hfinkel created this revision.
> hfinkel added reviewers: rsmith, aprantl, dexonsmith, dblaikie, echristo.
> hfinkel added a subscriber: cfe-commits.
> Herald added subscribers: joker.eph, mcrosier.
>
> We currently generate debug info for member calls in the context of the
> call expression. For member calls, this has an odd effect, which becomes
> especially obvious when generating source locations for optimizer-feedback
> remarks regarding inlining.
>
> Given this:
>
>   $ cat -n /tmp/i.cpp
>      1  void ext();
>      2
>      3  struct Bar {
>      4    void bar() { ext(); }
>      5  };
>      6
>      7  struct Foo {
>      8    Bar *b;
>      9
>     10    Bar *foo() { return b; }
>     11  };
>     12
>     13  void test(Foo *f) {
>     14    f->foo()->bar();
>     15  }
>
>   $ clang -g /tmp/i.cpp -S -emit-llvm -o -
>
>   define void @_Z4testP3Foo(%struct.Foo* %f) #0 !dbg !6 {
>     ...
>     %call = call %struct.Bar* @_ZN3Foo3fooEv(%struct.Foo* %0), !dbg !27
>     call void @_ZN3Bar3barEv(%struct.Bar* %call), !dbg !28
>     ...
>   !27 = !DILocation(line: 14, column: 3, scope: !6)
>   !28 = !DILocation(line: 14, column: 3, scope: !29)
>
> but we want instead for the calls to point to the callee expressions
> (foo() and bar() in this case). With this change, that's what happens.
>
> Fixes PR27567.
>
> http://reviews.llvm.org/D19708
>
> Files:
>   lib/CodeGen/CGExprCXX.cpp
>   test/CodeGenCXX/debug-info-member-call.cpp
>
> Index: test/CodeGenCXX/debug-info-member-call.cpp
> ===================================================================
> --- /dev/null
> +++ test/CodeGenCXX/debug-info-member-call.cpp
> @@ -0,0 +1,24 @@
> +// RUN: %clang_cc1 -triple x86_64-unknown_unknown -emit-llvm
> -debug-info-kind=standalone -dwarf-column-info %s -o - | FileCheck %s
> +void ext();
> +
> +struct Bar {
> +  void bar() { ext(); }
> +};
> +
> +struct Foo {
> +  Bar *b;
> +
> +  Bar *foo() { return b; }
> +};
> +
> +void test(Foo *f) {
> +  f->foo()->bar();
> +}
> +
> +// CHECK-LABEL: @_Z4testP3Foo
> +// CHECK: call {{.*}} @_ZN3Foo3fooEv{{.*}}, !dbg ![[CALL1LOC:.*]]
> +// CHECK: call void @_ZN3Bar3barEv{{.*}}, !dbg ![[CALL2LOC:.*]]
> +
> +// CHECK: ![[CALL1LOC]] = !DILocation(line: [[LINE:[0-9]+]], column: 6,
> +// CHECK: ![[CALL2LOC]] = !DILocation(line: [[LINE]], column: 13,
> +
> Index: lib/CodeGen/CGExprCXX.cpp
> ===================================================================
> --- lib/CodeGen/CGExprCXX.cpp
> +++ lib/CodeGen/CGExprCXX.cpp
> @@ -107,6 +107,10 @@
>                                                ReturnValueSlot
> ReturnValue) {
>    const Expr *callee = CE->getCallee()->IgnoreParens();
>
> +  // The debug information for the call instruction should point to the
> callee
> +  // expression.
> +  ApplyDebugLocation DL(*this, callee);
> +
>    if (isa<BinaryOperator>(callee))
>      return EmitCXXMemberPointerCallExpr(CE, ReturnValue);
>
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160429/54811a3d/attachment.html>