[PATCH] D55868: [Fixed Point Arithmetic] Fixed Point Addition Constant Expression Evaluation

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 15 11:01:39 PST 2019


rjmccall added a comment.

Yeah, I would recommend splitting the `APFixedPoint` in `APValue` changes into a separate patch.



================
Comment at: clang/lib/AST/ExprConstant.cpp:9959
+  }
+  llvm_unreachable("unknown cast resulting in fixed point value");
+}
----------------
leonardchan wrote:
> rjmccall wrote:
> > This is obviously unreachable because of the `default` case in the `switch`.  Should this be moved into the `default` case, and then you can put your `Error` call in cases for the known-missing fixed-point conversions?
> > 
> > You should go ahead and add cases for `CK_NoOp` and `CK_LValueToRValue`, they should be obvious from the other evaluators.
> > 
> > You should add tests for constant-evaluating fixed-point conversions.
> I think it would be better to keep it as an error since elsewhere, we emit errors indicating a particular operation or conversion has not yet been implemented.
Okay.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55868





More information about the cfe-commits mailing list