[PATCH] D76801: [AST] Print a> without extra spaces in C++11 or later.

2020-05-26 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. In D76801#2052270 , @dyung wrote: > Hi, we noticed an issue with the GDB test suite that was bisected back to > this change and I have put the details in PR46052. Can you take a look? Sorry this never got clearly resolved. I

[PATCH] D76801: [AST] Print a> without extra spaces in C++11 or later.

2020-05-25 Thread Douglas Yung via Phabricator via cfe-commits
dyung added a comment. Hi, we noticed an issue with the GDB test suite that was bisected back to this change and I have put the details in PR46052. Can you take a look? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76801/new/

[PATCH] D76801: [AST] Print a> without extra spaces in C++11 or later.

2020-04-27 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. In D76801#2005264 , @labath wrote: > In D76801#1997451 , @dblaikie wrote: > > > Yeah, points all taken - as for this actual issue... I'm kind of inclined > > to say "hey, our template

[PATCH] D76801: [AST] Print a> without extra spaces in C++11 or later.

2020-04-27 Thread Pavel Labath via Phabricator via cfe-commits
labath added a comment. In D76801#1997451 , @dblaikie wrote: > Yeah, points all taken - as for this actual issue... I'm kind of inclined to > say "hey, our template names already diverge somewhat - and this divergence > is in the realm of acceptable by

Re: [PATCH] D76801: [AST] Print a> without extra spaces in C++11 or later.

2020-04-25 Thread Adrian McCarthy via cfe-commits
I completed the bisect, and it is indeed the change in spacing between the `>`s that triggers the problem with the visualizers. That said, I'm still suspicious that the std::map visualizer isn't quite right with how it uses the wild cards and the template parameter placeholder(s), but I don't

[PATCH] D76801: [AST] Print a> without extra spaces in C++11 or later.

2020-04-23 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. Thanks David, I'm comfortable with that stance for DWARF. I know @amccarth is looking into the recent VS visualizer issue, and I would like him to confirm if it was this change or not when he gets a chance. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D76801: [AST] Print a> without extra spaces in C++11 or later.

2020-04-22 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. In D76801#1996376 , @labath wrote: > In D76801#1995058 , @dblaikie wrote: > > > > It becomes a gdb-index problem because with an index the debugger will do > > > a (hashed?) string lookup

[PATCH] D76801: [AST] Print a> without extra spaces in C++11 or later.

2020-04-22 Thread Pavel Labath via Phabricator via cfe-commits
labath added a comment. In D76801#1995058 , @dblaikie wrote: > > It becomes a gdb-index problem because with an index the debugger will do a > > (hashed?) string lookup and expect the string to be there. If the strings > > differ, the lookup won't find

[PATCH] D76801: [AST] Print a> without extra spaces in C++11 or later.

2020-04-21 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. In D76801#1994003 , @labath wrote: > In D76801#1993337 , @dblaikie wrote: > > > In D76801#1991904 , @labath wrote: > > > > > David's example does

[PATCH] D76801: [AST] Print a> without extra spaces in C++11 or later.

2020-04-21 Thread Pavel Labath via Phabricator via cfe-commits
labath added a comment. In D76801#1993337 , @dblaikie wrote: > In D76801#1991904 , @labath wrote: > > > David's example does work with gdb without -Wl,--gdb-index (the member > > variable is shown), presumably due

[PATCH] D76801: [AST] Print a> without extra spaces in C++11 or later.

2020-04-20 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. In D76801#1991904 , @labath wrote: > David's example does work with gdb without -Wl,--gdb-index (the member > variable is shown), presumably due to the aforementioned fuzzy matching. > However, it does not work if gdb-index is

[PATCH] D76801: [AST] Print a> without extra spaces in C++11 or later.

2020-04-20 Thread Pavel Labath via Phabricator via cfe-commits
labath added a comment. David's example does work with gdb without -Wl,--gdb-index (the member variable is shown), presumably due to the aforementioned fuzzy matching. However, it does not work if gdb-index is enabled, nor with lldb (as lldb is always very index-oriented and assumes equality

[PATCH] D76801: [AST] Print a> without extra spaces in C++11 or later.

2020-04-17 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added subscribers: tamur, cmtice, JDevlieghere, labath, probinson, aprantl. dblaikie added a comment. In D76801#1989641 , @sammccall wrote: > Sorry about the problems here, and thanks for letting me know... > > In D76801#1989421

[PATCH] D76801: [AST] Print a> without extra spaces in C++11 or later.

2020-04-17 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. In D76801#1989718 , @rnk wrote: > Hang on, I may have been too quick. We *do* have a test for this, at the end > of clang/test/CodeGenCXX/debug-info-codeview-display-names.cpp: > >

[PATCH] D76801: [AST] Print a> without extra spaces in C++11 or later.

2020-04-17 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a subscriber: amccarth. rnk added a comment. In D76801#1989641 , @sammccall wrote: > I've got a sinking feeling that I misunderstood the implications of updating > CodeGenCXX/debug-info-template-explicit-specialization.cpp and the >

[PATCH] D76801: [AST] Print a> without extra spaces in C++11 or later.

2020-04-17 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a subscriber: dblaikie. sammccall added a comment. Sorry about the problems here, and thanks for letting me know... In D76801#1989421 , @rnk wrote: > We updated the compiler and break some VS std::map visualizers: >

[PATCH] D76801: [AST] Print a> without extra spaces in C++11 or later.

2020-04-17 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added subscribers: tstellar, hans, rnk. rnk added a comment. We ran into three distinct issues with this change. We updated the compiler and break some VS std::map visualizers: https://crbug.com/1068394 I haven't 100% confirmed that this caused the issue, but looking at the code suggests

[PATCH] D76801: [AST] Print a> without extra spaces in C++11 or later.

2020-03-26 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG159a9f7e7630: [AST] Print abc without extra spaces in C++11 or later. (authored by sammccall). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76801/new/

[PATCH] D76801: [AST] Print a> without extra spaces in C++11 or later.

2020-03-26 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet marked an inline comment as done. kadircet added a comment. This revision is now accepted and ready to land. LGTM, thanks ! Comment at: clang/unittests/AST/DeclPrinterTest.cpp:1161 "A", -"Z > A")); -// Should be: with

[PATCH] D76801: [AST] Print a> without extra spaces in C++11 or later.

2020-03-25 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 252671. sammccall added a comment. update/move comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76801/new/ https://reviews.llvm.org/D76801 Files: clang-tools-extra/clangd/unittests/HoverTests.cpp

[PATCH] D76801: [AST] Print a> without extra spaces in C++11 or later.

2020-03-25 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. @kadircet sending this to you as I noticed it as a clangd hover bug :-) The highest impact is probably in diagnostic messages though. AFAICT the state before this patch is: - the type printer always printed the space - the stmt printer calls into the type printer to

[PATCH] D76801: [AST] Print a> without extra spaces in C++11 or later.

2020-03-25 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision. sammccall added a reviewer: kadircet. Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous. Herald added a project: clang. It's not 1998 anymore. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D76801 Files: