jfb requested changes to this revision.
jfb added a comment.
Herald added a subscriber: dexonsmith.

In D64666#1597193 <https://reviews.llvm.org/D64666#1597193>, @aaron.ballman 
wrote:

> In D64666#1596660 <https://reviews.llvm.org/D64666#1596660>, @xbolva00 wrote:
>
> > I think we should warn in that case even if GCC does not warn.
>
>
> Strong +1.


Sorry if the phrasing was misleading: if we know for a fact that there's a 
problem, we should warn unconditionally. If we don't know for a fact then the 
warning should *not* be enabled by `-Wall` nor `-Wextra`. I don't really care 
what GCC does by default, LLVM doesn't have to match every single thing. That 
being said, if LLVM behaves differently then maybe the flag name should be 
different.

> In D64666#1596853 <https://reviews.llvm.org/D64666#1596853>, @ziangwan wrote:
> 
>> Final review ping.
> 
> 
> Please be sure to give reviewers enough time to respond to comments before 
> pinging a review.

Indeed. You haven't answered my first comment, I'd expect you to do so and not 
"final ping" anything. I'm not saying you must do what I say, just that you 
must answer comments, not ignore them.


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

https://reviews.llvm.org/D64666



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to