[PATCH] D115670: Implement some constexpr vector unary operators, fix boolean-ops

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.


================
Comment at: clang/lib/AST/ExprConstant.cpp:10188
+    bool VisitUnaryOperator(const UnaryOperator *E);
+    // FIXME: Missing: unary ~, conditional operator (for GNU
     //                 conditional select), shufflevector, ExtVectorElementExpr
----------------
aaron.ballman wrote:
> Comment can be updated as we're not missing `~` any longer.
Drop "unary ~" since you handled it?


================
Comment at: clang/lib/AST/ExprConstant.cpp:10373
 
+static llvm::Optional<APValue> VectorHandleUnaryOperator(ASTContext &Ctx,
+                                                         QualType ResultTy,
----------------
Lower case per coding standards? Put "handle" first since it is the verb. We seem to be inconcistent about where we put "Vector", we have `handleVectorVectorBinOp` and `handleCompareOpForVector`.


================
Comment at: clang/lib/AST/ExprConstant.cpp:10395
+           "Vector operator ~ can only be int");
+    return APValue{~Elt.getInt()};
+  case UO_LNot: {
----------------
Would `APInt.getInt().flipAllbits()` be better than re-creating an APValue?


================
Comment at: clang/lib/Sema/SemaExpr.cpp:12264
       return Context.getExtVectorType(Context.CharTy, VTy->getNumElements());
-    else if (TypeSize == Context.getTypeSize(Context.ShortTy))
+    if (TypeSize == Context.getTypeSize(Context.ShortTy))
       return Context.getExtVectorType(Context.ShortTy, VTy->getNumElements());
----------------
Thanks for fixing the else after returns here. I almost commented on that earlier.


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

https://reviews.llvm.org/D115670