Re: [PATCH][RFC] c-family: Implement __has_feature and __has_extension [PR60512]

[Iain Sandoe <iain@sandoe.co.uk>]

Hi Alex,

thanks for working on this.

I’ve applied this patch and evaluated on a few Darwin versions (which is the
target currently most affected, I believe):

> On 9 May 2023, at 13:07, Alex Coplan <alex.coplan@arm.com> wrote:

> This patch implements clang's __has_feature and __has_extension in GCC.

Thanks, this blocks consuming Darwin SDK headers “properly” (PR 90709 as
linked to  60512) (which is why I had a WIP patch too).

So I am very keen to see this land in GCC-14, but have some  issues to deal 
with and would be looking for ideas about how to handle them by extending or
amending the patch.

The main concern I have at the moment is that it seems to me that we need
more flexible and general predicates for declaring feature/ext support:

  a) on target (see below for examples)
  b) on potentially multiple flags and language version at the same time (see below)
  c) what about features that exist for a closed range of language versions?

As mentioned by Jakub in a conversation about this on irc (months ago!) the
current identifiers potentially clash with use symbols.

IFF we add feature designations (which IMO we should, since this approach does
help simplify testcases and configurations) we should add them into the
implementation namespace:

e.g. ( for C) 
_GNU_nested_functions or __nested_functions

> Currently the patch aims to implement all documented features (and some
> undocumented ones) following the documentation at
> https://clang.llvm.org/docs/LanguageExtensions.html

TL;DR 
without guards or target-specific opt out this breaks bootstrap on Darwin.

I believe the patch is operating correctly .. but it is now exposing problems in
the system Frameworks that (in this first example) is union of three bugs
in the headers and clang versions.

Darwin’s SDK headers are a pretty complex amalgamation of macro and
conditional code spanning both regular (i.e. /usr/include) style headers and
those found in frameworks.

This took a bit of tracking down.

The bootstrap fail occurs because (with the patch) we now invoke a macro
CF_ENUM in the CoreFoundation framework with extended args.

BUG1;  This macro expands to wrong code (AFAICT) for a typedef enum. 
BUG2;  However, Apple clang accepts the wrong code and treats it as typedef.
           (GCC and upstream clang both reject, although MSVC accepts)
BUG3: Upstream clang is disabling that error when it appears in system headers.

.. so that combination means that we have a broken core framework header that
went undetected by both Apple and upstream clang.

Now it is possible that the header could be fixed in some future edition of the
SDK.  However that does not help existing and previous SDKs (I do not recall
a fix ever being made of any SDK other than the current).  So I have to figure
out how to approach this.

1. We could ‘fixincludes’ the issues… except because of PR105719, we can’t
    because we do not have  fixincludes for frameworks yet ...
   [this is not probably impossible to arrange but it’s quite some work to make
    sure we interpose the fixes correctly]

2. We could allow specific targets to opt out of declaring that they have
  support for some specific feature (see also below).

(one reason to allow target opt-in/out of specific features)

> with the following omissions:

> - Objective-C-specific features.

I can clearly append the objective-c(++) cases to the end of the respective
lists, but then we need to make them conditional on language, version and
dialect (some will not be appropriate to GNU runtime).

this is why I think we need more flexible predicates on declaring features
and extensions.

----

index 2b4c82facf7..5b8429244b2 100644
--- a/gcc/c-family/c-common.cc
+++ b/gcc/c-family/c-common.cc

+struct hf_feature_info

+  { "enumerator_attributes",		  0, 0 },
+  { "tls", 0, 0 },

Do all GCC targets support tls?

What about things like this:

 attribute_availability_tvos, 
 attribute_availability_watchos, 
 attribute_availability_driverkit, 
?

Even if they are implemented centrally, it is unlikely that all targets would want
to claim support (although note that the availability stuff does seem now to be
used on other platforms - like android, zos and hlsl.)

(non-bug-related reasons to consider target-specific predicates)

===

Once again, thanks for working on this - and I am keen to see it land,
Iain