[PATCH] D44093: [BUILTINS] structure pretty printer

[Paul Semel via Phabricator via cfe-commits]

paulsemel marked an inline comment as done.
paulsemel added a comment.

No problem, thanks for getting back on this ! I was busy because of my midterms anyway :)



================
Comment at: test/Sema/builtin-dump-struct.c:8
+  void *b;
+  int (*goodfunc)(const char *, ...);
+  int (*badfunc1)(const char *);
----------------
aaron.ballman wrote:
> paulsemel wrote:
> > aaron.ballman wrote:
> > > Can you also add a test for: `int (*badfunc4)(char *, ...);` and `int (*badfunc5)();`
> > Isn't `int (*func)()` is a valid prototype for a printf like function in C ?
> > I instead added `int (*func)(void)` to the test cases.
> > Isn't int (*func)() is a valid prototype for a printf like function in C ?
> 
> No, because it's missing the `const char *` as the mandatory first parameter. Do you want that to be allowed and hope the callee has it correct on their side, or do you want it to diagnose as not being a valid function?
Actually, from a kernel developer perspective, I would say it's better to let the user do its stuff on his side, because kernel is full of trick !
But if you think I'd rather check whether we have `int (*)(const char *, ...)` at any time, we can go for it !


================
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 *, ...)')}}
----------------
aaron.ballman wrote:
> paulsemel wrote:
> > aaron.ballman wrote:
> > > Why `&goodfunc`?
> > Yes, we already have a test like this anyway :)
> It was more a question of why the ampersand on the second argument -- I think that can be dropped and it'll be consistent with the rest of the tests.
Sure, sorry, I missed this. Totally agree with you, I am going to make the changes.


Repository:
  rC Clang

https://reviews.llvm.org/D44093