Преглед изворни кода

[NFC] Remove custom StringCchCopyEx (#5037)

This patch got a bit away from me. It started as me trying to remove
one of the Windows system functions we have a custom implementation of
(StringCchCopyEx). It didn't really make sense to me that we needed a
custom override of a function where we could have just used `strncpy`
or other LLVM utilities to provide the functionality.

After I rewrote the code calling `StringCchCopyEx`, I wanted to make
sure it was tested, which led me to discover that it was never called
in our existing tests (see:
https://microsoft.github.io/DirectXShaderCompiler/coverage/home/runner/w
ork/DirectXShaderCompiler/DirectXShaderCompiler/lib/DxcSupport/WinFuncti
ons.cpp.html#L27).

I then looked into the behavior of
`clang_getCursorSpellingWithFormatting`, which it turns out _does_
return fully qualified names. As a result the parent traversal we were
attempting always terminated immediately with one name segment found.

I wrote an additional test where the cursor name is _not_ fully
qualified in source to verify that the API works as expected in all
cases then started deleting code.

This patch deletes two functions (`StringCchCopyEx` &
`ConcatPartsReversed`), and simplifies the code in
`GetCursorQualifiedName`. It also adds the additional
non-fully-qualified name test case.
Chris B пре 2 година
родитељ
комит
143ab95e89

+ 0 - 2
include/dxc/Support/WinFunctions.h

@@ -19,8 +19,6 @@
 
 #ifndef _WIN32
 
-HRESULT StringCchCopyEx(LPSTR pszDest, size_t cchDest, LPCSTR pszSrc,
-                        LPSTR *ppszDestEnd, size_t *pcchRemaining, DWORD dwFlags);
 HRESULT StringCchPrintfA(char *dst, size_t dstSize, const char *format, ...);
 HRESULT UIntAdd(UINT uAugend, UINT uAddend, UINT *puResult);
 HRESULT IntToUInt(int in, UINT *out);

+ 0 - 16
lib/DxcSupport/WinFunctions.cpp

@@ -22,22 +22,6 @@
 #include "dxc/Support/WinFunctions.h"
 #include "dxc/Support/microcom.h"
 
-HRESULT StringCchCopyEx(LPSTR pszDest, size_t cchDest, LPCSTR pszSrc,
-                        LPSTR *ppszDestEnd, size_t *pcchRemaining, DWORD dwFlags) {
-  assert(dwFlags == 0 && "dwFlag values not supported in StringCchCopyEx");
-  char *zPtr = 0;
-
-  zPtr = stpncpy(pszDest, pszSrc, cchDest);
-
-  if (ppszDestEnd)
-    *ppszDestEnd = zPtr;
-
-  if (pcchRemaining)
-    *pcchRemaining = cchDest - (zPtr - pszDest);
-
-  return S_OK;
-}
-
 
 HRESULT StringCchPrintfA(char *dst, size_t dstSize, const char *format, ...) {
   va_list args;

+ 22 - 101
tools/clang/tools/libclang/dxcisenseimpl.cpp

@@ -114,47 +114,6 @@ HRESULT AnsiToBSTR(_In_opt_z_ const char* text, _Outptr_result_maybenull_ BSTR*
   return S_OK;
 }
 
-static
-HRESULT ConcatPartsReversed(_In_count_(count) CXString* first, int count, _COM_Outptr_ LPSTR* pResult) throw()
-{
-  HRESULT hr = S_OK;
-  size_t total = 0;
-  char* writeCursor;
-
-  // Count how big the result should be.
-  for (int i = 0; i < count; i++)
-  {
-    if (i != 0)
-    {
-      total += 2;
-    }
-    total += strlen(clang_getCString(first[i]));
-  }
-
-  total++; // null terminator
-
-  *pResult = new (std::nothrow) char[total];
-  writeCursor = *pResult;
-  for (int i = 0; i < count; i++)
-  {
-    if (i != 0)
-    {
-      hr = StringCchCopyEx(writeCursor, total, "::", &writeCursor, &total, 0);
-      if (FAILED(hr)) break;
-    }
-    hr = StringCchCopyEx(writeCursor, total, clang_getCString(first[count - 1 - i]), &writeCursor, &total, 0);
-    if (FAILED(hr)) break;
-  }
-
-  if (FAILED(hr))
-  {
-    delete[] *pResult;
-    *pResult = nullptr;
-  }
-
-  return hr;
-}
-
 static
 _Ret_opt_ _Post_readable_byte_size_(cb)  __drv_allocatesMem(Mem)
 LPVOID CoTaskMemAllocZero(SIZE_T cb) throw()
@@ -242,69 +201,31 @@ HRESULT CoTaskMemAllocString(_In_z_ const char* src, _Outptr_ LPSTR* pResult) th
   return S_OK;
 }
 
-static
-HRESULT GetCursorQualifiedName(CXCursor cursor, bool includeTemplateArgs, BSTR* pResult) throw()
-{
-  HRESULT hr = S_OK;
-  CSimpleArray<CXString> nameParts;
-  char* concatParts = nullptr;
-  const CXCursorFormatting DefaultFormatting = (CXCursorFormatting)
-    (CXCursorFormatting_SuppressTagKeyword);
-  const CXCursorFormatting VariableFormatting = (CXCursorFormatting)
-    (CXCursorFormatting_SuppressTagKeyword | CXCursorFormatting_SuppressSpecifiers);
-
+static HRESULT GetCursorQualifiedName(CXCursor cursor, bool includeTemplateArgs,
+                                      BSTR *pResult) throw() {
   *pResult = nullptr;
 
-  // To construct the qualifeid name, walk up the semantic parents until we are
-  // in the translation unit.
-  while (IsCursorKindQualifiedByParent(clang_getCursorKind(cursor)))
-  {
-    CXCursor parent;
+  if (IsCursorKindQualifiedByParent(clang_getCursorKind(cursor))) {
     CXString text;
-    text = clang_getCursorSpellingWithFormatting(cursor, DefaultFormatting);
-    nameParts.Add(text);
-    parent = clang_getCursorSemanticParent(cursor);
-    cursor = parent;
-  }
-
-  if (nameParts.GetSize() == 0)
-  {
-    if (clang_getCursorKind(cursor) == CXCursor_VarDecl ||
-        (clang_getCursorKind(cursor) == CXCursor_DeclRefExpr &&
-         clang_getCursorKind(clang_getCursorReferenced(cursor)) == CXCursor_VarDecl))
-    {
-      hr = CXStringToBSTRAndDispose(clang_getCursorSpellingWithFormatting(cursor, VariableFormatting), pResult);
-    }
-    else
-    {
-      hr = CXStringToBSTRAndDispose(clang_getCursorSpellingWithFormatting(cursor, DefaultFormatting), pResult);
-    }
-  }
-  else if (nameParts.GetSize() == 1)
-  {
-    hr = CXStringToBSTRAndDispose(nameParts[0], pResult);
-    nameParts.RemoveAll();
-  }
-  else
-  {
-    hr = ConcatPartsReversed(nameParts.GetData(), nameParts.GetSize(), &concatParts);
-    if (SUCCEEDED(hr))
-    {
-      hr = AnsiToBSTR(concatParts, pResult);
-    }
-  }
-
-  for (int i = 0; i < nameParts.GetSize(); i++)
-  {
-    clang_disposeString(nameParts[i]);
-  }
-
-  if (concatParts)
-  {
-    delete[] concatParts;
-  }
-
-  return hr;
+    text = clang_getCursorSpellingWithFormatting(
+        cursor, CXCursorFormatting_SuppressTagKeyword);
+    return CXStringToBSTRAndDispose(text, pResult);
+  }
+
+  if (clang_getCursorKind(cursor) == CXCursor_VarDecl ||
+      (clang_getCursorKind(cursor) == CXCursor_DeclRefExpr &&
+       clang_getCursorKind(clang_getCursorReferenced(cursor)) ==
+           CXCursor_VarDecl)) {
+    return CXStringToBSTRAndDispose(
+        clang_getCursorSpellingWithFormatting(
+            cursor, CXCursorFormatting_SuppressTagKeyword |
+                        CXCursorFormatting_SuppressSpecifiers),
+        pResult);
+  }
+  return CXStringToBSTRAndDispose(
+      clang_getCursorSpellingWithFormatting(
+          cursor, CXCursorFormatting_SuppressTagKeyword),
+      pResult);
 }
 
 static

+ 11 - 0
tools/clang/unittests/HLSL/DXIsenseTest.cpp

@@ -99,6 +99,7 @@ protected:
   TEST_METHOD(CursorWhenOverloadedResolvedThenDirectSymbol)
   TEST_METHOD(CursorWhenReferenceThenDefinitionAvailable)
   TEST_METHOD(CursorWhenTypeOfVariableDeclThenNamesHaveType)
+  TEST_METHOD(CursorTypeUsedNamespace)
   TEST_METHOD(CursorWhenVariableRefThenSimpleNames)
   TEST_METHOD(CursorWhenVariableUsedThenDeclarationAvailable)
 
@@ -540,6 +541,16 @@ TEST_F(DXIntellisenseTest, CursorWhenTypeOfVariableDeclThenNamesHaveType) {
   ExpectDeclarationText(result.TU, 3, 6, L"class Ns::TheClass");
 }
 
+TEST_F(DXIntellisenseTest, CursorTypeUsedNamespace) {
+  char program[] =
+    "namespace Ns { class TheClass {\n"
+    "}; }\n"
+    "using namespace Ns;\n"
+    "TheClass C;";
+  CompilationResult result(CompilationResult::CreateForProgram(program, _countof(program)));
+  ExpectQualifiedName(result.TU, 4, 4, L"Ns::TheClass");
+}
+
 TEST_F(DXIntellisenseTest, CursorWhenVariableRefThenSimpleNames) {
   char program[] =
     "namespace Ns { class TheClass {\r\n"