tbaeder added inline comments.
================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:620
+ case UO_Deref: // *x
+ case UO_Not: // ~x
+ case UO_Real: // __real x
----------------
aaron.ballman wrote:
> tbaeder wrote:
> > aaron.ballman wrote:
> > > This is reachable and will always return true thanks to the magic of
> > > integer promotions:
> > > ```
> > > void func() {
> > > bool b = true;
> > > b = ~b;
> > > }
> > > ```
> > This code path is not just for boolean values though.
> All the more reason to be worried about `~` being marked as unreachable (I
> mostly worry because people often try to make bit fiddling operations
> constant expressions when possible, so this seems pretty likely to be hit).
> Perhaps make it into an `assert(false && "operation not supported yet");` or
> something more loud?
How is that different? It's just a marker for me so I know later in development
that this is not implemented yet. And now that I'm writing this, I guess I see
what you mean. Yes, it's not unreachable, but it shouldn't be reached when
testing what is currently implemented.
================
Comment at: clang/lib/AST/Interp/Integral.h:173
+ }
+
template <bool SrcSign> static Integral from(Integral<0, SrcSign> Value) {
----------------
aaron.ballman wrote:
> tbaeder wrote:
> > I'm a bit out of my element with the template magic here.
> It mostly looks correct to me, but the part I'm struggling with is
> `static_cast<Integral::T>`; can you explain what you're trying to do there?
I was trying to implement creating an Integral from a Boolean; the
`static_cast` casts to `Integral::T` (which in my case is `int`). It might not
be needed.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D132098/new/
https://reviews.llvm.org/D132098
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
