Mordante marked 10 inline comments as done.
Mordante added a comment.
Thanks for the feedback. I'll update the patch after making the requested
changes.
================
Comment at: clang/include/clang/Basic/AttrDocs.td:1866
+ while(true) [[unlikely]] {
+ ... // The attribute has no effect
+ } // Clang elides comparison and generates an infinite loop
----------------
aaron.ballman wrote:
> Mordante wrote:
> > aaron.ballman wrote:
> > > This is a bit of an edge case where I can't tell whether we should or
> > > should not diagnose. On the one hand, the user wrote something and we
> > > think it's meaningless, which we would usually diagnose as being an
> > > ignored attribute so the user can maybe react to it. On the other hand,
> > > "has no effect" is perhaps not the same as "ignored", as with `i + 0`
> > > (it's not really ignored but you would expect it to have no effect
> > > either).
> > In this case it's ignored since the CodeGen doesn't create a branch
> > instruction. GCC does the same at -O0, MSVC creates the branch at -O0, but
> > removes it at higher optimization levels. So since the other two main
> > compilers also remove the branch I think we could issue a diagnostic in
> > this case we can do that when the CodeGen determines the branch isn't
> > needed. In that case I don't expect false positives. Agreed?
> >
> >
> I think that's reasonable if it's easy enough for you to do, but I don't
> insist on a diagnostic if it turns out to be tricky to support for some
> reason.
Emitting it from the CodeGen is trivial, there it's known whether the branch is
emitted. Emitting it from the Sema would be harder, since there it isn't known
whether the branch will be omitted. This would mean a `-fsyntax-only` run won't
emit the diagnostic. I'm not sure whether doing the check in the CodeGen
affects third-party tools that show diagnostics to the user. For users
compiling code it makes no difference.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D89899/new/
https://reviews.llvm.org/D89899
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
