cor3ntin added inline comments.

================
Comment at: clang/test/AST/Interp/arrays.cpp:143
+
+};
----------------
tahonermann wrote:
> As others already noted, additional testing of multicharacter literals and 
> UCNs (including named universal characters like `\N{LATIN_CAPITAL_LETTER_E}` 
> would be beneficial. Some tests of character escapes like `\t` wouldn't hurt 
> either.
> 
> Clang does not yet support use of `-fexec-charset` to set the literal 
> encoding (execution character set) to anything other than UTF-8 though work 
> on that has been done (see D93031). If such work was completed, it would be 
> useful to run some tests against a non-UTF-8 encoding. Maybe next year.
Yes, wide **multicharacter** literals, that's was important information 
missing, thanks for spotting that.

I have mixed feeling about adding tests for escape sequences.  Their 
replacement doesn't happen during constant evaluation.
We shouldn't replicate the lexing tests here.

but we should compare string literal with byte values. Testing a string literal 
against another one doesn't ensure the code units are correct if both are 
equally miss evaluated.

Also we could add explicit tests for null termination here as they are added as 
part of evaluation in theory - but then again that's also something clang does 
earlier.

If we want we could consider enabling the byte code interpreter on the existing 
lexing test files, i actually think that's the better way to deal with the 
escape sequences tests.


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

https://reviews.llvm.org/D135366

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

Reply via email to