Document the Clang policies on claiming support for a feature

[aaron.ballman]

Summary

We do not currently have this written down anywhere, and as a result, we're sometimes inconsistent with how we handle feature test macros and the feature status pages. This is an attempt to document what I understand our existing policies to be instead of defining a new policy.

Note, this is being added to the Clang internals manual because we don't have a separate document for Clang developer policies. At some point, I think we may want a standalone document for that, and this content can be moved there at that time.

[erichkeane]

clang/docs/InternalsManual.rst

3276

Phab won't let me suggest it, but instead of __cpp_decltype (which to my mind, suggested it was a function-style FTM?), I might suggest something like __cpp_lambdas.

3289

you -> we? Rest of the doc is in 3rd person, this is in 2nd?

[cor3ntin]

clang/docs/InternalsManual.rst

3285

any way to rephrase that, "that cause clamg to reject" maybe?

3292

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.

3294–3299

+1 :)

3300–3306

The status reported by a feature test macro should always be reflected in the
language support page for the coresponding feature

such as claiming partial

(i can't suggest edits, for some reason?)

[aaron.ballman]

clang/docs/InternalsManual.rst

3285

Sure, we can go with "accept invalid code" instead of "fail to reject invalid".

3289

Reworded to remove the pronoun entirely.

3292

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.

[aaron.ballman]

Updating based on review feedback.

(Note: Phab seems to be confused about the changes being at the end of the file -- it claims there's no trailing context for the patch and that's what seems to disable the ability to make suggested edits.)

[cor3ntin]

clang/docs/InternalsManual.rst

3285

is the issue the thing that accepts the code? that's what bugs me

[aaron.ballman]

clang/docs/InternalsManual.rst

3285

I'm trying to list out two problems separately: the first is when we reject code that we should be accepting, and the second is when we accept code that we should be rejecting. I think I see where your confusion is coming from, how about:

* Are there known issues where we reject valid code that should be accepted?
* Are there known issues where we accept invalid code that should be rejected?

[cor3ntin]

clang/docs/InternalsManual.rst

3285

that sounds good to me!

[aaron.ballman]

Update based on review feedback.

[aaron.ballman]

Ping

[aaron.ballman]

Fixing a typo pointed out off-list (missed an "as").

[cor3ntin]

LGTM. I have some minor reservations about defining feature macros on a per-target basis but at the same time, given the current coroutines on 32 bits windows situation - which has been lingering - I'm not sure i can come up with a more sensible alternative

clang/docs/InternalsManual.rst

#. There are some "extras" that make other features work better.  It's worth
 
    * Add code completion support for your expression in
      ``SemaCodeComplete.cpp``.
    * If your expression has types in it, or has any "interesting" features
      other than subexpressions, extend libclang's ``CursorVisitor`` to provide
      proper visitation for your expression, enabling various IDE features such
      as syntax highlighting, cross-referencing, and so on.  The
      ``c-index-test`` helper program can be used to test these features.
+
+Feature Test Macros
+===================
+Clang implements several ways to test whether a feature is supported or not.
+Some of these feature tests are standardized, like ``__has_cpp_attribute`` or
+``__cpp_lambdas``, while others are Clang extensions, like ``__has_builtin``.
+The common theme among all the various feature tests is that they are a utility
+to tell users that we think a particular feature is complete. However,
+completeness is a difficult property to define because features may still have
+lingering bugs, may only work on some targets, etc. We use the following
+criteria when deciding whether to expose a feature test macro (or particular
+result value for the feature test):
+
+  * Are there known issues where we reject valid code that should be accepted?
+  * Are there known issues where we accept invalid code that should be rejected?
+  * Are there known crashes, failed assertions, or miscompilations?
+  * Are there known issues on a particular relevant target?
+
+If the answer to any of these is "yes", the feature test macro should either
+not be defined or there should be very strong rationale for why the issues
+should not prevent defining it. Note, it is acceptable to define the feature
+test macro on a per-target basis if needed.
+
+When in doubt, being conservative is better than being aggressive. If we don't
+claim support for the feature but it does useful things, users can still use it
+and provide us with useful feedback on what is missing. But if we claim support
+for a feature that has significant bugs, we've eliminated most of the utility
+of having a feature testing macro at all because users are then forced to test
+what compiler version is in use to get a more accurate answer.
+
+The status reported by the feature test macro should always be reflected in the
+language support page for the corresponding feature (`C++
+<https://clang.llvm.org/cxx_status.html>`_, `C
+<https://clang.llvm.org/c_status.html>`_) if applicable. This page can give
+more nuanced information to the user as well, such as claiming partial support
+for a feature and specifying details as to what remains to be done.