rsmith added inline comments.
================ Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8368 +def warn_cast_function_type : Warning< + "cast between incompatible function types from %0 to %1">, + InGroup<CastFunctionType>, DefaultIgnore; ---------------- I'd find something like "cast from function type %0 to incompatible function type %1" to be easier to read. But I suppose %0 and %1 are the pointer types, not the function types, so perhaps "cast from %0 to %1 converts to incompatible function type" or something like that? ================ Comment at: clang/lib/Sema/SemaCast.cpp:1044-1048 + // Allow integral type mismatch if their size are equal. + if (SrcType->isIntegralType(Context) && DestType->isIntegralType(Context)) + if (Context.getTypeInfoInChars(SrcType).Width == + Context.getTypeInfoInChars(DestType).Width) + return true; ---------------- In addition to allowing the cases where the sizes are the same width, should we also allow cases where the promoted types are the same width (eg, `int` versus `short`)? What does GCC do? ================ Comment at: clang/lib/Sema/SemaCast.cpp:1050-1055 + llvm::DenseSet<std::pair<Decl *, Decl *>> NonEquivalentDecls; + StructuralEquivalenceContext Ctx( + Context, Context, NonEquivalentDecls, StructuralEquivalenceKind::Default, + false /*StrictTypeSpelling*/, false /*Complain*/, + false /*ErrorOnTagTypeMismatch*/); + return Ctx.IsEquivalent(SrcType, DestType); ---------------- I think a "same type" check (`Context.hasSameUnqualifiedType(SrcType, DestType)`) would be more appropriate here than a structural equivalence check. ================ Comment at: clang/lib/Sema/SemaCast.cpp:1067-1081 + if (SrcType->isFunctionPointerType() && DestType->isFunctionPointerType()) { + SrcFTy = SrcType->castAs<PointerType>() + ->getPointeeType() + ->castAs<FunctionType>(); + DstFTy = DestType->castAs<PointerType>() + ->getPointeeType() + ->castAs<FunctionType>(); ---------------- This can be simplified a bit. ================ Comment at: clang/lib/Sema/SemaCast.cpp:1082-1083 + ->castAs<FunctionType>(); + } else { + return true; + } ---------------- We should also handle the case where the source is of function type and the destination is a reference-to-function type. Maybe also block pointer types? ================ Comment at: clang/lib/Sema/SemaCast.cpp:1090 + return false; + if (auto PT = T->getAs<FunctionProtoType>()) + return !PT->isVariadic() && PT->getNumParams() == 0; ---------------- Please use `auto *` here (or `const auto *` as the linter suggests). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97831/new/ https://reviews.llvm.org/D97831 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits