rsmith added inline comments.
================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6451
+ "%1 is %select{|in}3complete">,
+ InGroup<C99Compat>;
def ext_typecheck_ordered_comparison_of_function_pointers : ExtWarn<
----------------
pestctrl wrote:
> efriedma wrote:
> > `InGroup<C11>`
> Sorry, I'm not sure I understand. Isn't this a C99 warning? Why is it being
> put in the C11 group?
Because `C11` really means `C11Extensions`, and this is a C11 extension (ie,
it's code that's valid in C11 but not valid in C99):
```
// A warning group for warnings about using C11 features as extensions.
def C11 : DiagGroup<"c11-extensions">;
```
================
Comment at: clang/lib/Sema/SemaExpr.cpp:11571
+ Diag(Loc,
+ getLangOpts().C11
+ ? diag::ext_typecheck_compare_complete_incomplete_pointers
----------------
pestctrl wrote:
> rsmith wrote:
> > 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.
> GCC has left this warning on by default in any language mode, FWIW. Should we
> still restrict this warning to only C99 mode?
Yes, I think so. I assume the GCC folks haven't noticed that this rule was
relaxed in C11. I've filed a bug against GCC for this:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95630
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79945/new/
https://reviews.llvm.org/D79945
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
