aaron.ballman 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
----------------
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?


================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:129-130
+    return this->emitCast(*FromT, *ToT, CE);
+  }
+    return false;
+
----------------



================
Comment at: clang/lib/AST/Interp/Integral.h:173
+  }
+
   template <bool SrcSign> static Integral from(Integral<0, SrcSign> Value) {
----------------
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?


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

Reply via email to