tbaeder marked 3 inline comments as done.
tbaeder added inline comments.
================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:445-446
+ const CharacterLiteral *E) {
+ if (Optional<PrimType> T = classify(E->getType()))
+ return this->emitConst(E, E->getValue());
+
----------------
tahonermann wrote:
> `T` isn't used and `classify()` already returns an `Optional<PrimType>`, so
> no conversion coercion appear to be intended here. I think this can be
> simplified to:
> if (classify(E))
> this->emitConst(E, E->getValue());
Good point. Since the value of the expression is always an `APInt`, might was
well skip the `classify()` altogether. `emitConst()` does the classify anyway.
================
Comment at: clang/test/AST/Interp/arrays.cpp:143
+
+};
----------------
cor3ntin wrote:
> 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.
I changed the first test that inspects all characters of a string to comparing
with integers instead. Do you have a suggestion for what lexing tests to enable
the constant interpreter in?
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
