owenpan marked an inline comment as done. owenpan added a comment. In D143546#4116196 <https://reviews.llvm.org/D143546#4116196>, @HazardyKnusperkeks wrote:
> In D143546#4114341 <https://reviews.llvm.org/D143546#4114341>, @owenpan wrote: > >> In D143546#4113721 <https://reviews.llvm.org/D143546#4113721>, @rymiel wrote: >> >>> In D143546#4112077 <https://reviews.llvm.org/D143546#4112077>, @owenpan >>> wrote: >>> >>>> As this one is an invalid-code-generation bug, I wanted it fixed ASAP. >>> >>> Do you intend to backport it to the 16 release branch then? >> >> I don't know but will go with whatever you, @MyDeveloperDay and >> @HazardyKnusperkeks prefer. > > I'd vote in favor, letting a code breaking bug knowingly in the most current > (or to come? I don't follow that.) version would be let's say not nice. +1. ================ Comment at: clang/lib/Format/TokenAnnotator.cpp:3884 if (Style.isCpp()) { + if (Right.is(tok::period) && Left.is(tok::numeric_constant)) + return true; ---------------- HazardyKnusperkeks wrote: > owenpan wrote: > > rsmith wrote: > > > owenpan wrote: > > > > HazardyKnusperkeks wrote: > > > > > Add a comment what that is? Without the bug report I'd not know what > > > > > that sequence would be. > > > > I could do that, but the github issue is linked in the summary above > > > > and will be in the commit message. In general, I don't like unnecessary > > > > comments littered in the source. They can become outdated, out of > > > > place, misleading, and even wrong. How about giving an example as shown > > > > above? > > > Does `clang-format` have any formatting modes where it would leave out > > > spaces around `+` or `-`? The same issue arises with things like `0xe + > > > n`, where removing the space between the `0xe` and the `+` results in a > > > token splice. > > No. I was aware of them and had made sure clang-format already handled them > > correctly. > > I could do that, but the github issue is linked in the summary above and > > will be in the commit message. In general, I don't like unnecessary > > comments littered in the source. They can become outdated, out of place, > > misleading, and even wrong. How about giving an example as shown above? > > If you do a `git blame` you can come to the commit and thus to the bug, but > if you are just reading, at least I don't blame all the lines. There I think > a comment is nice, and your proposal is a really nice one. > But I've accepted the patch as is, and so the decision is yours. :) +1. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143546/new/ https://reviews.llvm.org/D143546 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
