erichkeane marked 13 inline comments as done. erichkeane added a comment. New patch coming as soon as my build finishes.
================ Comment at: clang/docs/LanguageExtensions.rst:480 = yes yes yes yes -:? yes -- -- -- +:?[*]_ yes -- yes -- sizeof yes yes yes yes ---------------- aaron.ballman wrote: > Do these columns line up in the actual source file? They don't appear to line > up in the Phab viewer, but I don't know if that's an artifact of Phab or not. They don't, I had misread the sphinx doc it seems and was doing footnotes incorrectly. A coworker shared the live-sphinx viewer so I corrected this. See next patch. ================ Comment at: clang/lib/CodeGen/CGExprScalar.cpp:4301 + llvm::Type *condType = ConvertType(condExpr->getType()); + llvm::VectorType *vecTy = cast<llvm::VectorType>(condType); + llvm::Value *zeroVec = llvm::Constant::getNullValue(vecTy); ---------------- aaron.ballman wrote: > `auto *` > > Also, should these three variables have identifiers starting with an > uppercase like `CondV` et al? Probably, I just C&P'ed this code from the above group :/ ================ Comment at: clang/lib/Sema/SemaExprCXX.cpp:5757 + const QualType EltTy = + cast<VectorType>(CondTy.getCanonicalType().getTypePtr()) + ->getElementType(); ---------------- aaron.ballman wrote: > Do you actually need the `getTypePtr()` bit, or will the `cast<>` suffice to > trigger the cast through `Type*`? TIL! ================ Comment at: clang/lib/Sema/SemaExprCXX.cpp:5793 + Diag(QuestionLoc, diag::err_conditional_vector_operand_type) + << /*isExtVector*/ true << RHSType; + return {}; ---------------- aaron.ballman wrote: > Shouldn't this pass in the `LHSType` instead because that's what's being > tested? Yep! ================ Comment at: clang/lib/Sema/SemaExprCXX.cpp:5805 + // If both are vector types, they must be the same type. + if (!Context.hasSameType(LHSType, RHSType)) { + Diag(QuestionLoc, diag::err_conditional_vector_mismatched_vectors) ---------------- aaron.ballman wrote: > Do you need to canonicalize these types before comparing them? No, both versions of this function canonicalize for me: https://clang.llvm.org/doxygen/ASTContext_8h_source.html#l02305 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71463/new/ https://reviews.llvm.org/D71463 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits