aaron.ballman added inline comments.
================ Comment at: lib/Sema/SemaChecking.cpp:1122 + !PtrArgType->getPointeeType()->isRecordType()) { + this->Diag(PtrArg->getLocStart(), + diag::err_typecheck_convert_incompatible) ---------------- Drop all instances of `this->`. ================ Comment at: lib/Sema/SemaChecking.cpp:1156 + if (!FT->isVariadic() || FT->getReturnType() != Context.IntTy || + FT->getParamType(0) != firstArg) { + this->Diag(FnPtrArg->getLocStart(), ---------------- Rather than building up a type only for comparison purposes, why not check: ``` QualType PT = FT->getParamType(0); if (!PT->isPointerType() || !PT->getPointeeType()->isCharType() || !PT->getPointeeType().isConstQualified()) { Diag(...); } ``` ================ Comment at: test/CodeGen/dump-struct-builtin.c:254 + __builtin_dump_struct(&b, &printf); +} ---------------- I'd like to see some test cases using unions (perhaps mixing unions and inner struct types), tests using arrays, floating-point values, and type qualifiers on the data members. ================ Comment at: test/Sema/builtin-dump-struct.c:8 + void *b; + int (*goodfunc)(const char *, ...); + int (*badfunc1)(const char *); ---------------- Can you also add a test for: `int (*badfunc4)(char *, ...);` and `int (*badfunc5)();` ================ Comment at: test/Sema/builtin-dump-struct.c:15 + __builtin_dump_struct(1); // expected-error {{too few arguments to function call, expected 2, have 1}} + __builtin_dump_struct(1, 2); // expected-error {{passing 'int' to parameter of incompatible type 'structure pointer type': type mismatch at 1st parameter ('int' vs 'structure pointer type')}} + __builtin_dump_struct(&a, 2); // expected-error {{passing 'int' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int' vs 'int (*)(const char *, ...)')}} ---------------- Hrm, the `'structure pointer type'` in this diagnostic is unfortunate because it's being quoted as though it were a real type -- you could drop the single quotes. If you think the resulting diagnostic reads too strangely, perhaps we will have to go back to a custom diagnostic after all. ================ Comment at: test/Sema/builtin-dump-struct.c:17 + __builtin_dump_struct(&a, 2); // expected-error {{passing 'int' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int' vs 'int (*)(const char *, ...)')}} + __builtin_dump_struct(b, &goodfunc); // expected-error {{passing 'void *' to parameter of incompatible type 'structure pointer type': type mismatch at 1st parameter ('void *' vs 'structure pointer type')}} + __builtin_dump_struct(&a, badfunc1); // expected-error {{passing 'int (*)(const char *)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int (*)(const char *)' vs 'int (*)(const char *, ...)')}} ---------------- Why `&goodfunc`? Repository: rC Clang https://reviews.llvm.org/D44093 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits