aaron.ballman marked 5 inline comments as done. aaron.ballman added inline comments.
================ Comment at: clang/docs/InternalsManual.rst:3285 + * Are there known issues with the feature that reject valid code? + * Are there known issues that fail to reject invalid code? + * Are there known crashes, failed assertions, or miscompilations? ---------------- cor3ntin wrote: > any way to rephrase that, "that cause clamg to reject" maybe? Sure, we can go with "accept invalid code" instead of "fail to reject invalid". ================ Comment at: clang/docs/InternalsManual.rst:3289 + +If the answer to any of these is "yes", you should either not define the +feature test macro or have very strong rationale for why the issues should not ---------------- erichkeane wrote: > you -> we? Rest of the doc is in 3rd person, this is in 2nd? Reworded to remove the pronoun entirely. ================ Comment at: clang/docs/InternalsManual.rst:3292 +prevent defining it. Note, it is acceptable to define the feature test macro on +a per-target basis if needed. + ---------------- cor3ntin wrote: > Is that desirable? > If i have a build system checking for support through feature macros, I'm not > sure they do that on a target per target basis. Maybe it's not desirable, I'm happy to hear arguments either way. The reason why I think this is reasonable is because we'll sometimes have a feature that works just fine everywhere EXCEPT a target (like coroutines work everywhere but have known miscompilation issues on 32-bit Windows). It seems somewhat unfortunate to not claim support anywhere because it's not supported on all targets -- some users likely never target 32-bit Windows in the first place. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146420/new/ https://reviews.llvm.org/D146420 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
