cor3ntin added inline comments.

================
Comment at: clang/lib/AST/ExprConstant.cpp:16417-16418
+  }
+  if (!Scope.destroy())
+    return false;
+
----------------
aaron.ballman wrote:
> Rather than use an RAII object and destroy it manually, let's use `{}` to 
> scope the RAII object appropriately.
This seems to be the way to use this interface - mostly because destroy can 
fail and we want to detect that


================
Comment at: clang/lib/AST/ExprConstant.cpp:16413
+    APSInt C = Char.getInt();
+    Result.push_back(static_cast<char>(C.getExtValue()));
+    if (!HandleLValueArrayAdjustment(Info, PtrExpression, String, CharTy, 1))
----------------
aaron.ballman wrote:
> barannikov88 wrote:
> > This relies on host's CHAR_BIT >= target's CHAR_BIT, which isn't true for 
> > my target. Could you add an assertion?
> > 
> Wouldn't adding the assertion cause you problems then? (FWIW, we only support 
> `CHAR_BIT == 8` currently.)
I replied to that in one of the comment i made on the review, but CHAR_BIT is 
not relevant there.
This cast is valid as long as `C` carries some kind of UTF-8 code unit - even 
if that code unit was 0 extended to fill an arbitrary long storage.
This would only be problematic if somehow the literal encoding of narrow 
literal was a wide encoding like UTF-16 or dec-kanji?
The assertion we would want is "does the literal encoding has codepoints 
greater than the size of char", which is not something we are going to get.

That being said I realized while replying that there is a bug there: it should 
cast to unsigned.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154290

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to