ZijunZhao marked an inline comment as not done. ZijunZhao added inline comments.
================ Comment at: clang/lib/Sema/SemaChecking.cpp:388 + } + // First two arguments should be integers. ---------------- aaron.ballman wrote: > I keeps my original code because it just checks once but this checks many times and in case `BuiltinType::WChar_U` checking is missing(I know isWideCharType() can be added but I still think check the type is between Short and Int128, and UShort and UInt128 will be quicker and a bit safer?). ================ Comment at: clang/lib/Sema/SemaChecking.cpp:414-431 if (!PtrTy || !PtrTy->getPointeeType()->isIntegerType() || + (!PtrTy->getPointeeType()->isPureIntegerType() && CkdOperation) || PtrTy->getPointeeType().isConstQualified()) { S.Diag(Arg.get()->getBeginLoc(), diag::err_overflow_builtin_must_be_ptr_int) << Ty << Arg.get()->getSourceRange(); ---------------- aaron.ballman wrote: > I don't think the `else if` part should be removed. We should make sure the result type is not short type. In `ValidCkdIntType()` checking, short type is a valid type. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157331/new/ https://reviews.llvm.org/D157331 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits