Skip to content

Add support for availability-style features #10098

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 28, 2025
Merged

Conversation

ahatanaka
Copy link

This patch doesn't include the codegen and serialization changes that are needed to support the availability-style features.

rdar://137999979

@ahatanaka
Copy link
Author

#9815 has the codegen and serialization changes.

@@ -3999,6 +4008,23 @@ std::optional<AvailabilitySpec> Parser::ParseAvailabilitySpec() {
Actions.CodeCompletion().CodeCompleteAvailabilityPlatformName();
return std::nullopt;
}

if (Tok.is(tok::identifier) && GetLookAheadToken(1).is(tok::equal)) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggest: tok::colon instead of tok:equal, so it matches the feat: <feature> style ones

ObjCShouldCallSuper(false), ObjCIsDesignatedInit(false),
ObjCWarnForNoDesignatedInitChain(false), ObjCIsSecondaryInit(false),
ObjCWarnForNoInitDelegation(false), NeedsCoroutineSuspends(true),
FoundImmediateEscalatingExpression(false), ErrorTrap(Diag) {}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggest to not clang-format this after adding the new line, so we reduce the downstream diff.

}

bool ASTContext::hasUnavailableFeature(const Decl *D) const {
if (D->hasAttrs())

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: using early exist instead of nesting the for loop inside the if check.

@@ -3539,6 +3539,10 @@ def fno_builtin : Flag<["-"], "fno-builtin">, Group<f_Group>,
def fno_builtin_ : Joined<["-"], "fno-builtin-">, Group<f_Group>,
Visibility<[ClangOption, CC1Option, CLOption, DXCOption]>,
HelpText<"Disable implicit builtin knowledge of a specific function">;
def ffeature_availability_EQ : Joined<["-"], "ffeature-availability=">, Group<f_Group>,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Silly question: why do we need this flag if all the features are defined in a header?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this flag for testing purposes before we decided on the availability domain definition spec. I can remove it as it's not needed anymore. I left it in as I thought it could be handy when testing or debugging the feature.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation! this matches with what I thought.

@ahatanaka
Copy link
Author

@tshortli I added an API for retrieving the information about availability domain on a decl. 81991a7

The function returns a pair of feature name and AvailabilityDomainInfo. The second member of AvailabilityDomainInfo is a CallExpr *, which calls the runtime query function, wrapped in an implicit cast to bool. Would that work? Or should the second member be a FunctionDecl * that points to the query function?

@ahatanaka
Copy link
Author

It's still possible to get the FunctionDecl for the query function by calling CallExpr::getCalleeDecl() or CallExpr::getDirectCallee() after removing the implicit cast.

@tshortli
Copy link

@tshortli I added an API for retrieving the information about availability domain on a decl. 81991a7

The function returns a pair of feature name and AvailabilityDomainInfo. The second member of AvailabilityDomainInfo is a CallExpr *, which calls the runtime query function, wrapped in an implicit cast to bool. Would that work? Or should the second member be a FunctionDecl * that points to the query function?

Yes, that looks reasonable. Thanks!

@ahatanaka ahatanaka force-pushed the feature-availability-sema branch from 95fa011 to 6d736cf Compare February 28, 2025 23:28
This commit extends the `__availability` attribute to accept a feature
name and a boolean-like integer argument (0 or 1) to indicate
availability based on feature presence or absence.

`__builtin_available` and `@available` take a feature name as their
arguments. References to annotated declarations in unguarded contexts
are rejected.

For example:

```
__attribute__((availability(domain:feature1, 0)))
void available_func(void);

__attribute__((availability(domain:feature1, 1)))
void unavailable_func(void);

if (__builtin_available(feature1)) {
  available_func();
  unavailable_func(); // error
} else {
  available_func(); // error
  unavailable_func();
}
```

rdar://137999979
@ahatanaka ahatanaka force-pushed the feature-availability-sema branch from 6d736cf to b53735b Compare February 28, 2025 23:36
@ahatanaka ahatanaka merged commit bc7b2cd into next Feb 28, 2025
@ahatanaka ahatanaka deleted the feature-availability-sema branch February 28, 2025 23:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants