[PATCH] D126651: [clang-diff] Fix getStmtValue when dealing with wide chars

[Johannes Altmanninger via Phabricator via cfe-commits]

johannes requested changes to this revision.
johannes added a comment.
This revision now requires changes to proceed.

I've suggested some more refactorings but otherwise this should be good to go



================
Comment at: clang/lib/Tooling/ASTDiff/ASTDiff.cpp:497
+      return str;
+    }
     return std::string(String->getString());
----------------
Sorry for the delay. Looks pretty good already.

 think there's a bit of redundancy now, I tried to simplify it, WDYT?
I used uppercase variable names as this is the local convention. I'm not sure what's the global one.
`String->getByteLength() / String->getCharByteWidth();` adds an unnecessary division, we can just use getLength().

```
    if (String->isWide() || String->isUTF16() || String->isUTF32()) {
      std::string UTF8Str;
      unsigned int NumChars = String->getLength();
      const char *Bytes = String->getBytes().data();
      if (String->isWide()) {
        const auto *Chars = reinterpret_cast<const wchar_t *>(Bytes);
        if (!convertWideToUTF8({Chars, NumChars}, UTF8Str))
          return "";
      } else if (String->isUTF16()) {
        const auto *Chars = reinterpret_cast<const unsigned short *>(Bytes);
        if (!convertUTF16ToUTF8String({Chars, NumChars}, UTF8Str))
          return "";
      } else {
        assert(String->isUTF32() && "Unsupported string encoding.");
        const auto *Chars = reinterpret_cast<const unsigned int *>(Bytes);
        if (!convertUTF32ToUTF8String({Chars, NumChars}, UTF8Str))
          return "";
      }
      return UTF8Str;
    }
```


================
Comment at: clang/test/Tooling/clang-diff-ast.cpp:74
+      return U"foo";
+    // CHECK-NOT: ImplicitCastExpr
+    return 0;
----------------
I think two lines per encoding should be enough, so how about doing this right next to the existing string literal?

```
  const char *foo(int i) {
    if (i == 0)
      // CHECK: StringLiteral: foo(
      return "foo";
    // CHECK: StringLiteral: wide(
    (void)L"wide";
    // CHECK: StringLiteral: utf-16(
    (void)u"utf-16";
    // CHECK: StringLiteral: utf-32(
    (void)U"utf-32";
    // CHECK-NOT: ImplicitCastExpr
    return 0;
  }
```


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

https://reviews.llvm.org/D126651