[PATCH] D79274: Fix template class debug info for Visual Studio visualizers

[Adrian McCarthy via Phabricator via cfe-commits]

amccarth marked 2 inline comments as done.
amccarth added inline comments.


================
Comment at: clang/test/CodeGenCXX/debug-info-codeview-display-name.cpp:10
+// RUN: %clang_cc1 -fblocks -debug-info-kind=limited -gcodeview -emit-llvm %s \
+// RUN:       -o - -triple=x86_64-pc-win32 -Wno-new-returns-null -fms-compatibility | \
+// RUN:    grep -E 'DISubprogram|DICompositeType' | sed -e 's/.*name: "\([^"]*\)".*/"\1"/' | \
----------------
rnk wrote:
> Why choose fms-compatibility? Does it have a side effect of raising the default standard version?
> Why choose fms-compatibility?

I thought that `-fms-compatibility` was the solution we had discussed.

With ms-compat, we retain the space.  Without ms-compat, clang does what clang does (which, currently, is to omit the space).

> Does it have a side effect of raising the default standard version?

That was not my intent.  Removing the `-std=c++98` raises the standard.


================
Comment at: clang/test/CodeGenCXX/debug-info-codeview-display-name.cpp:12
+// RUN:    grep -E 'DISubprogram|DICompositeType' | sed -e 's/.*name: "\([^"]*\)".*/"\1"/' | \
+// RUN:    FileCheck %s --check-prefix=CHECK --check-prefix=MSCOMPAT
 
----------------
rnk wrote:
> Any reason not to reuse `--check-prefix=UNQUAL`? This should be the same as the first RUN line, with a different standard.
This partly overlaps with the thread about ms-compat.  But apart from that, I was trying to be explicit about what's being tested, which, in this case, is the ms-compat.  The reason the old test got the space is somewhat tangential to the qualified/unqualified distinction.

If you wanted to not pin to C++98 and not use ms-compat, then you wouldn't expect the space.  Maybe that's something that I should also test.


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

https://reviews.llvm.org/D79274