rjmccall added inline comments.
================ Comment at: clang/include/clang/AST/ASTContext.h:2638 + // type. + QualType getCorrespondingSignedFixedPointType(QualType Ty) const; + ---------------- Please include in the comment here that, unlike `getCorrespondingUnsignedType`, this has to be specific to fixed-point types because there are unsigned integer types like `bool` and `char8_t` that don't have signed equivalents. ================ Comment at: clang/include/clang/Basic/FixedPoint.h:41 + assert(!HasUnsignedPadding && + "Cannot have unsigned padding on a signed type."); } ---------------- `assert(!(IsSigned && HasUnsignedPadding) && ...);`, please. ================ Comment at: clang/include/clang/Basic/FixedPoint.h:67 + FixedPointSemantics + getCommonSemantics(const FixedPointSemantics &Other) const; + ---------------- Actually representing the fully-precise value is operation-specific; you'd need a different calculation for at least addition and multiplication, and maybe also subtraction and (maybe?) division. Is this what you actually want? ================ Comment at: clang/include/clang/Basic/FixedPoint.h:70 + /// Return the FixedPointSemantics for an integer type. + static FixedPointSemantics GetIntegerSemantics(unsigned Width, bool IsSigned); + ---------------- This is trivial enough that it should probably be implemented inline. ================ Comment at: clang/lib/AST/ASTContext.cpp:10489 + return FixedPointSemantics::GetIntegerSemantics(getIntWidth(Ty), + Ty->isSignedIntegerType()); + ---------------- Does `getIntWidth` do the thing you want for `bool`? ================ Comment at: clang/lib/Basic/FixedPoint.cpp:141 + if (ResultIsSigned || ResultHasUnsignedPadding) + CommonWidth++; + ---------------- Is this right? If I have `a+b+c+d+e+f+g+h+i`, where those are all the same signed type, won't the width of the common semantics keep growing as I add more operands? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53738/new/ https://reviews.llvm.org/D53738 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits