aaron.ballman added inline comments.
================
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
----------------
Mordante wrote:
> 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.
I don't spend a whole lot of time in CodeGen so I was surprised there were
CodeGen-specific diagnostics, but I see now that we do have some and they live
with the other frontend diagnostics. I think doing the work from CodeGen is
reasonable, but I'm also not certain if third-parties will find it difficult.
Given that we already support diagnostics in CodeGen, I'd say it's fine to add
another one.
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
