This revision was automatically updated to reflect the committed changes.
Closed by commit rL312965: [codeview] omit debug locations for nested exprs
unless column info enabled (authored by inglorion).
Changed prior to commit:
https://reviews.llvm.org/D37529?vs=114715=114716#toc
Repository:
inglorion updated this revision to Diff 114715.
inglorion added a comment.
renamed get{Nested,}ExpressionLocationsEnabled and moved it into
CodeGetModule.cpp
https://reviews.llvm.org/D37529
Files:
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/CodeGen/CodeGenModule.cpp
rnk added inline comments.
Comment at: clang/lib/CodeGen/CodeGenModule.h:32
#include "clang/Basic/XRayLists.h"
+#include "clang/Frontend/CodeGenOptions.h"
#include "llvm/ADT/DenseMap.h"
majnemer wrote:
> Any reason to do this? I'd just keep
majnemer added inline comments.
Comment at: clang/lib/CodeGen/CodeGenModule.h:32
#include "clang/Basic/XRayLists.h"
+#include "clang/Frontend/CodeGenOptions.h"
#include "llvm/ADT/DenseMap.h"
Any reason to do this? I'd just keep
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
lgtm
Comment at: clang/lib/CodeGen/CodeGenModule.h:517
+ /// Return true if we should emit location information for nested
expressions.
+ bool
inglorion updated this revision to Diff 114465.
inglorion added a comment.
Of course, ApplyDebugLocation is also a perfectly legitimate way to add a debug
location to nodes that are not nested inside nodes that already have a
location. I updated the diff so that we do end up applying the
Well, if they worked I wasn't going to say we needed to add tests for them,
i just wanted to make sure they work before we move onto something else.
In any case, lgtm
On Fri, Sep 8, 2017 at 4:43 PM Bob Haarman via Phabricator <
revi...@reviews.llvm.org> wrote:
> inglorion updated this revision
inglorion updated this revision to Diff 114463.
inglorion added a comment.
added examples suggested by @zturner, verified step over and step into specific
behavior matches MSVC, and added tests for them
https://reviews.llvm.org/D37529
Files:
clang/lib/CodeGen/CGDebugInfo.cpp
zturner added a comment.
@inglorion : Since we're already in this code anyway, should we at least do due
diligence and check the basics, even if only manually? As in, at least we
should check in MSVC if any of those other types of examples that I came up
with present a problem. Otherwise
inglorion updated this revision to Diff 114299.
inglorion added a comment.
removed compound statement; that was only in there to double check that the
debugger stops on the first child statement, but that's true with or without
this change
https://reviews.llvm.org/D37529
Files:
inglorion added a comment.
@zturner: I am still thinking about your comment about other cases to test. My
concern is that there are very many possible combinations.
I'm actually not too concerned about not exactly matching cl's behavior in
every single case. The difference in behavior here is
inglorion updated this revision to Diff 114292.
inglorion added a comment.
Following conversation with @rnk, I managed to whittle this down to a very
small change that seems to do what we need. Step into specific works and single
stepping through the program behaves similarly whether compiled
inglorion planned changes to this revision.
inglorion added a comment.
rnk and I talked about a different approach. The idea is to explicitly emit
locations in some cases (e.g. inside compound statements, the braces of for
loops, ...), and otherwise emit locations only when emitting column info
inglorion updated this revision to Diff 114097.
inglorion marked 5 inline comments as done.
inglorion added a comment.
I limited the change to only calls, returns, and declarations. I also
updated the test case to include a multi-variable declaration, a while
loop, a for loop, and an if statement
zturner added inline comments.
Comment at: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp:44
+ int a = bar(x, y) +
+ baz(x, z) +
+ qux(y, z);
inglorion wrote:
> zturner wrote:
> > inglorion wrote:
> > > zturner wrote:
> > > > Can you make a
zturner added inline comments.
Comment at: clang/lib/CodeGen/CGDebugInfo.h:65
llvm::MDNode *CurInlinedAt = nullptr;
+ bool LocationEnabled = true;
llvm::DIType *VTablePtrType = nullptr;
Can you move this line up to put it next to another bool? Not a huge
inglorion added inline comments.
Comment at: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp:44
+ int a = bar(x, y) +
+ baz(x, z) +
+ qux(y, z);
zturner wrote:
> inglorion wrote:
> > zturner wrote:
> > > Can you make a function called `int
inglorion created this revision.
Herald added a subscriber: aprantl.
Microsoft Visual Studio expects debug locations to correspond to
statements. We used to emit locations for expressions nested inside statements.
This would confuse the debugger, causing it to stop multiple times on the
same line
inglorion added inline comments.
Comment at: clang/lib/CodeGen/CGStmt.cpp:45
+ }
+ return IDL;
+}
inglorion wrote:
> rnk wrote:
> > Does MSVC accept this? I think it will emit the copy ctor call in an -O0
> > build.
> I wrote this thinking that the right
zturner added inline comments.
Comment at: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp:44
+ int a = bar(x, y) +
+ baz(x, z) +
+ qux(y, z);
inglorion wrote:
> zturner wrote:
> > Can you make a function called `int foo()` and make this
rnk added inline comments.
Comment at: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp:12
+
+// NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[LOC:[0-9]+]]
+// NONEST: call i32 @{{.*}}baz{{.*}}, !dbg ![[LOC]]
inglorion wrote:
> rnk wrote:
> > This is pretty
rnk added a subscriber: echristo.
rnk added inline comments.
Comment at: clang/lib/CodeGen/CGStmt.cpp:38
+InhibitDebugLocation CodeGenFunction::EmitStmtStopPoint(const Stmt *S) {
+ InhibitDebugLocation IDL;
"Stop point" is a hold-over from the
inglorion added inline comments.
Comment at: clang/lib/CodeGen/CGStmt.cpp:45
+ }
+ return IDL;
+}
rnk wrote:
> Does MSVC accept this? I think it will emit the copy ctor call in an -O0
> build.
I wrote this thinking that the right thing would happen under copy
inglorion updated this revision to Diff 114060.
inglorion added a comment.
removed accidentally left in include and reformatted mangled comment
https://reviews.llvm.org/D37529
Files:
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/CodeGen/CGDebugInfo.h
clang/lib/CodeGen/CGStmt.cpp
24 matches
Mail list logo
