lebedev.ri resigned from this revision. lebedev.ri added a comment. In D59712#1472318 <https://reviews.llvm.org/D59712#1472318>, @jdenny wrote:
> In D59712#1469760 <https://reviews.llvm.org/D59712#1469760>, @lebedev.ri > wrote: > > > In D59712#1469693 <https://reviews.llvm.org/D59712#1469693>, @jdenny wrote: > > > > > In D59712#1469392 <https://reviews.llvm.org/D59712#1469392>, @lebedev.ri > > > wrote: > > > > > > > In D59712#1469358 <https://reviews.llvm.org/D59712#1469358>, @jdenny > > > > wrote: > > > > > > > > > In D59712#1469301 <https://reviews.llvm.org/D59712#1469301>, > > > > > @lebedev.ri wrote: > > > > > > > > > > > In D59712#1469295 <https://reviews.llvm.org/D59712#1469295>, > > > > > > @craig.topper wrote: > > > > > > > > > > > > > Wondering if it would be better to assert for asking for the sign > > > > > > > of an unsigned APSInt. I could see a caller just wanting to get > > > > > > > the msb for some reason and not knowing that isNegative won’t > > > > > > > work. > > > > > > > > > > > > > > > > > > Yes, i, too, would think an assert is much better solution (since i > > > > > > literally just tripped over this in this review.) > > > > > > > > > > > > > > > > > Does this pass `check-all`? `check-all` of stage-2? test-suite? > > > > > > > > > No. The assert breaks cases in at least ExprConstant.cpp and > > > SemaExpr.cpp. > > > > > > Err, i was talking about the current code in the patch :) > > > For me, check-all's success is not affected by the current patch. I built > all subprojects except llgo, which gave me a build problem independently of > this patch. (Built natively, or with clang with this patch?) > I've never tried running the other tests you mention, for any patch. I > thought people normally left those to the bots. Should this patch be handled > differently? Sounds good. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59712/new/ https://reviews.llvm.org/D59712 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
