[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: clang/lib/Sema/SemaExpr.cpp:11571
+          Diag(Loc,
+               getLangOpts().C11
+                   ? diag::ext_typecheck_compare_complete_incomplete_pointers
----------------
efriedma wrote:
> pestctrl wrote:
> > efriedma wrote:
> > > rsmith wrote:
> > > > pestctrl wrote:
> > > > > efriedma wrote:
> > > > > > I think this condition is backwards?  Should be `!getLangOpts().C11`.  You want the warning with `-std=c99 -pedantic`, you don't want the warning with `std=c11 -pedantic`.
> > > > > I don't think it's backwards. If getLangOpts().C11, then it is an extension. Otherwise, it is the warning. I can switch the conditions if it is confusing though.
> > > > "Extension" means "this is invalid code that we're accepting anyway" -- that's what this is in C99. In C11, I think we shouldn't be diagnosing at all.
> > > > 
> > > > Has anyone checked whether WG14 removed this restriction in C11 as a DR resolution? If so, we shouldn't be diagnosing it at all, in any language mode.
> > > I tracked down the proposal for the change; it's http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1439.pdf .  Beyond the reference to http://www.open-std.org/jtc1/sc22/wg14/www/docs/dr_314.htm , I can't find any relevant defect report.
> > I have updated the diff to diagnose only for C99. Does the existence of the proposal mean we shouldn't be diagnosing in any language mode?
> > 
> > Also, how did you track down the proposal that quickly? Even after skimming through it, I still can't find it through searching. 
> > Also, how did you track down the proposal that quickly? 
> 
> It wasn't really that quick, but the thing that eventually worked was that I googled for `site:open-std.org "At various points within a translation unit"`.
I tried to get some clarity from WG14 as to whether they intended N1439 to be interpreted as applying retroactively, but it seems like their stance is that they do not do maintenance work on past standards, and have no mechanism for identifying whether papers should be encouraged for retroactive application or only for implementations intending to conform to later standards.

In the absence of guidance either way from WG14, I think our best bet is to follow GCC and the literal standards text as this patch does.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79945/new/

https://reviews.llvm.org/D79945