efriedma 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<
----------------
`InGroup<C11>`


================
Comment at: clang/lib/Sema/SemaExpr.cpp:11571
+          Diag(Loc,
+               getLangOpts().C11
+                   ? diag::ext_typecheck_compare_complete_incomplete_pointers
----------------
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"`.


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

Reply via email to