void accepted this revision.
void added a comment.
This revision is now accepted and ready to land.

One small comment about the documentation but L:GTM.



================
Comment at: clang/docs/LanguageExtensions.rst:1584
+Outputs may be used along any branches from the ``asm goto`` whether the
+branches are taken or not.
 
----------------
efriedma wrote:
> Maybe put a note about earlier releases, so people don't get confused if they 
> use newer documentation than their version of clang.
> 
> Is there any chance we want a dedicated __has_extension flag?
> Is there any chance we want a dedicated __has_extension flag?

Might not be a bad idea just in case they have more than one compiler version 
they're using. Then again, if they add code to use this feature, they're fairly 
committed to the assumption that it'll "just work" even with a __has_extension 
flag...(Remember that we currently don't actually warn if they're using the 
outputs on the indirect branch. We just specify that such behavior is 
undefined.)

I think a strongly worded note here that it's available with Clang 16+ (or 
whatever) is going to be slightly better. It'll force them to use macros to 
support multiple compiler versions, making their code wicked ugly, and them 
possibly coming up with better ways to ensure that the correct compiler is used 
for this feature.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136497

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

Reply via email to