tahonermann added a comment.
> Give some time for Aaron/Tom to have the chance to raise further comment but
> I'm happy with it otherwise.
I'm sorry for the delay getting back to this and I see this has already been
pushed (and a fix then pushed). The changes look good to me too.
Repository:
thakis added a comment.
One of your commits doesn't build on windows with clang-cl as host compiler:
http://45.33.8.238/win/68965/step_4.txt
Please take a look and revert for now if it takes a while to fix.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6fa1795d1fd4: [clang][Interp] Implement String- and
CharacterLiterals (authored by tbaeder).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135366/new/
https
tbaeder updated this revision to Diff 471144.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135366/new/
https://reviews.llvm.org/D135366
Files:
clang/lib/AST/Interp/ByteCodeExprGen.cpp
clang/lib/AST/Interp/ByteCodeExprGen.h
clang/test/AST/Interp/literals.cpp
clang/test/Lexer/char
aaron.ballman accepted this revision.
aaron.ballman added a comment.
LGTM aside from a testing request.
Comment at: clang/test/AST/Interp/literals.cpp:342
+ static_assert(mc == 'abc', "");
+ __WCHAR_TYPE__ wm = L'abc'; // ref-error{{wide character literals may not
contain mu
cor3ntin accepted this revision.
cor3ntin added a comment.
This revision is now accepted and ready to land.
Give some time for Aaron/Tom to have the chance to raise further comment but
I'm happy with it otherwise.
Thanks for doing that, and sorry I missed you earlier ping.
CHANGES SINCE LAST AC
tbaeder added a comment.
Ping
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
tbaeder added a comment.
I've moved the tests to `literals.cpp`, is there anything else left?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135366/new/
https://reviews.llvm.org/D135366
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
h
tbaeder updated this revision to Diff 467399.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135366/new/
https://reviews.llvm.org/D135366
Files:
clang/lib/AST/Interp/ByteCodeExprGen.cpp
clang/lib/AST/Interp/ByteCodeExprGen.h
clang/test/AST/Interp/literals.cpp
clang/test/Lexer/char
aaron.ballman added inline comments.
Comment at: clang/test/AST/Interp/arrays.cpp:143
+
+};
tahonermann wrote:
> tbaeder wrote:
> > cor3ntin wrote:
> > > tahonermann wrote:
> > > > cor3ntin wrote:
> > > > > tbaeder wrote:
> > > > > > cor3ntin wrote:
> > > > > > >
tahonermann added inline comments.
Comment at: clang/test/AST/Interp/arrays.cpp:143
+
+};
tbaeder wrote:
> cor3ntin wrote:
> > tahonermann wrote:
> > > cor3ntin wrote:
> > > > tbaeder wrote:
> > > > > cor3ntin wrote:
> > > > > > tahonermann wrote:
> > > > > > > A
tbaeder added inline comments.
Comment at: clang/test/AST/Interp/arrays.cpp:143
+
+};
cor3ntin wrote:
> tahonermann wrote:
> > cor3ntin wrote:
> > > tbaeder wrote:
> > > > cor3ntin wrote:
> > > > > tahonermann wrote:
> > > > > > As others already noted, additiona
cor3ntin added inline comments.
Comment at: clang/test/AST/Interp/arrays.cpp:143
+
+};
tahonermann wrote:
> cor3ntin wrote:
> > tbaeder wrote:
> > > cor3ntin wrote:
> > > > tahonermann wrote:
> > > > > As others already noted, additional testing of multicharacter
tahonermann added inline comments.
Comment at: clang/test/AST/Interp/arrays.cpp:143
+
+};
cor3ntin wrote:
> tbaeder wrote:
> > cor3ntin wrote:
> > > tahonermann wrote:
> > > > As others already noted, additional testing of multicharacter literals
> > > > and UCN
cor3ntin added inline comments.
Comment at: clang/test/AST/Interp/arrays.cpp:143
+
+};
tbaeder wrote:
> cor3ntin wrote:
> > tahonermann wrote:
> > > As others already noted, additional testing of multicharacter literals
> > > and UCNs (including named universal
tbaeder updated this revision to Diff 466700.
tbaeder marked an inline comment as done.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135366/new/
https://reviews.llvm.org/D135366
Files:
clang/lib/AST/Interp/ByteCodeExprGen.cpp
clang/lib/AST/Interp/ByteCodeExprGen.h
clang/test/AST/I
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 T = classify(E->getType()))
+return this->emitConst(E, E->getValue());
+
ta
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}`
tahonermann added inline comments.
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:445-446
+const CharacterLiteral *E) {
+ if (Optional T = classify(E->getType()))
+return this->emitConst(E, E->getValue());
+
`T` isn't used and `classify()` already
cor3ntin added inline comments.
Comment at: clang/test/AST/Interp/arrays.cpp:135
+ static_assert(u32[1] == U'b', "");
+};
+
aaron.ballman wrote:
> shafik wrote:
> > tbaeder wrote:
> > > aaron.ballman wrote:
> > > > I think you need a more coverage for character
aaron.ballman added a subscriber: cor3ntin.
aaron.ballman added inline comments.
Comment at: clang/test/AST/Interp/arrays.cpp:135
+ static_assert(u32[1] == U'b', "");
+};
+
shafik wrote:
> tbaeder wrote:
> > aaron.ballman wrote:
> > > I think you need a more cov
tbaeder updated this revision to Diff 466461.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135366/new/
https://reviews.llvm.org/D135366
Files:
clang/lib/AST/Interp/ByteCodeExprGen.cpp
clang/lib/AST/Interp/ByteCodeExprGen.h
clang/test/AST/Interp/arrays.cpp
Index: clang/test/AST/I
shafik added inline comments.
Comment at: clang/test/AST/Interp/arrays.cpp:128
+
+ static_assert("foobar"[2] == 'o', "");
+
Also `2["foobar"]`
Comment at: clang/test/AST/Interp/arrays.cpp:135
+ static_assert(u32[1] == U'b', "");
+};
+
---
tbaeder added inline comments.
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:446
+ if (Optional T = classify(LitType)) {
+APInt Val(getIntWidth(LitType), E->getValue());
+return this->emitConst(*T, 0, Val, E);
aaron.ballman wrote:
> Should this be
tbaeder updated this revision to Diff 466262.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135366/new/
https://reviews.llvm.org/D135366
Files:
clang/lib/AST/Interp/ByteCodeExprGen.cpp
clang/lib/AST/Interp/ByteCodeExprGen.h
clang/test/AST/Interp/arrays.cpp
Index: clang/test/AST/I
aaron.ballman added inline comments.
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:446
+ if (Optional T = classify(LitType)) {
+APInt Val(getIntWidth(LitType), E->getValue());
+return this->emitConst(*T, 0, Val, E);
Should this be concerned about
tbaeder added inline comments.
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:447
+APInt Val(getIntWidth(LitType), E->getValue());
+return this->emitConst(*T, 0, Val, E);
+ }
The `0` here is unused in `emitConst` anyway. I have a follow-up NFC patc
tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, erichkeane, tahonermann, shafik.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Repository:
rG LLVM Github Monorepo
https://revi
28 matches
Mail list logo
