aaron.ballman added inline comments.
================
Comment at: clang/test/Sema/prototype-redecls.c:32
// clear that the previous declaration was a builtin.
-float rintf() { // expected-error {{conflicting types for 'rintf'}} \
+float rintf() { // expected-warning {{incompatible redeclaration of library
function 'rintf'}} \
expected-note {{'rintf' is a builtin with type 'float
(float)'}}
----------------
erichkeane wrote:
> aaron.ballman wrote:
> > erichkeane wrote:
> > > Hmm... this is a definition of a builtin with a completely incompatible
> > > prototype. Do we REALLY want this to not be an error?
> > >
> > > I guess I could see it being OK with declarations, but it is odd with a
> > > definition that is incompatible.
> > I think we do want it to not be an error. The user has no choice in whether
> > these are predeclared or not, which steals identifiers from users. By
> > giving the user a warning, they're alerted to the fact they're doing
> > something that may be highly confusing, so I agree we definitely want a
> > diagnostic. If we give the user an error, they're stuck. Also, it's even
> > weirder that:
> > ```
> > float rintf(void) {} // Always a warning, is fine
> > float rintf() {} // Error pre C2x, but fine in C2x
> > ```
> Aren't all those identifiers already stolen by the standard? They are
> reserved already, right?
>
> But that behavior you post there is unfortunate... I guess leaving these as
> warnings is important so they get suppressed in headers (for cases where a
> C-library 'implements' it anyway, despite it being a builtin).
>
> I think I can live with this, and I think backporting it is the least
> breaking way to do this.
> Aren't all those identifiers already stolen by the standard? They are
> reserved already, right?
Yes, they are. However, the situation we should have sympathy for is a user who
is compiling in C17 mode and using an identifier like `ckd_add` suddenly having
that identifier stolen out from under them because C2x added that as a library
function and we decided we wanted it to be a builtin for some reason.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131499/new/
https://reviews.llvm.org/D131499
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
