This is an archive of the discontinued LLVM Phabricator instance.

Differential D146866

[clang][ExtractAPI] Remove extra pointer indirection from declaration fragments for Obj-C lightweight generics on id
ClosedPublic

Authored by chaitanyav on Mar 25 2023, 12:12 AM.

Details

Reviewers
dang
ributzka
Commits
rGa590d8634308: [clang][ExtractAPI] Remove extra pointer indirection from declaration fragments…
Summary

Fixes https://github.com/llvm/llvm-project/issues/61479

Diff Detail

Event Timeline

chaitanyav created this revision.Mar 25 2023, 12:12 AM
Herald added a reviewer: ributzka. · View Herald TranscriptMar 25 2023, 12:12 AM
Herald added a project: Restricted Project. · View Herald Transcript
chaitanyav requested review of this revision.Mar 25 2023, 12:12 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 25 2023, 12:12 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
chaitanyav added a comment.Mar 27 2023, 12:43 AM

@dang @ributzka Please review.

Harbormaster completed remote builds in B221731: Diff 508284.Mar 27 2023, 12:44 AM
chaitanyav added a comment.Mar 27 2023, 2:09 PM

looks like its waiting on libcxx build(https://buildkite.com/llvm-project/premerge-checks/builds/143382#job-01872209-2788-44ef-930c-11e741ff906e) and it is actively being debugged as per comments on other differential.

dang added inline comments.Mar 29 2023, 8:24 AM
clang/lib/ExtractAPI/DeclarationFragments.cpp
176
176

IIUC the distinction here is that id<Protocol> is a id qualified type, but id<Protocol> * is not? If so this is a fairly subtle point, and worth mentioning in this comment.

chaitanyav updated this revision to Diff 509429.Mar 29 2023, 11:26 AM

Add comment to indicate that id<protocol>* is not an qualified id type

Harbormaster completed remote builds in B222565: Diff 509429.Mar 29 2023, 1:22 PM
chaitanyav added a comment.Mar 29 2023, 1:43 PM

@dang Please review.

dang accepted this revision.Mar 30 2023, 7:17 AM
This revision is now accepted and ready to land.Mar 30 2023, 7:17 AM
Closed by commit rGa590d8634308: [clang][ExtractAPI] Remove extra pointer indirection from declaration fragments… (authored by chaitanyav). · Explain WhyMar 30 2023, 11:36 AM
This revision was automatically updated to reflect the committed changes.
chaitanyav added a commit: rGa590d8634308: [clang][ExtractAPI] Remove extra pointer indirection from declaration fragments….

Revision Contents

PathSize
clang/
lib/
ExtractAPI/
22 lines
test/
ExtractAPI/
341 lines

Diff 509754

clang/lib/ExtractAPI/DeclarationFragments.cpp

Show First 20 Lines • Show All 154 Lines • ▼ Show 20 Lines
// qualifiers removed. // qualifiers removed.
DeclarationFragments DeclarationFragmentsBuilder::getFragmentsForType( DeclarationFragments DeclarationFragmentsBuilder::getFragmentsForType(
const Type *T, ASTContext &Context, DeclarationFragments &After) { const Type *T, ASTContext &Context, DeclarationFragments &After) {
assert(T && "invalid type"); assert(T && "invalid type");
DeclarationFragments Fragments; DeclarationFragments Fragments;
// Declaration fragments of a pointer type is the declaration fragments of // Declaration fragments of a pointer type is the declaration fragments of
// the pointee type followed by a `*`, except for Objective-C `id` and `Class` // the pointee type followed by a `*`,
// pointers, where we do not spell out the `*`. if (T->isPointerType())
if (T->isPointerType() ||
(T->isObjCObjectPointerType() &&
!T->getAs<ObjCObjectPointerType>()->isObjCIdOrClassType())) {
return Fragments return Fragments
.append(getFragmentsForType(T->getPointeeType(), Context, After)) .append(getFragmentsForType(T->getPointeeType(), Context, After))
.append(" *", DeclarationFragments::FragmentKind::Text); .append(" *", DeclarationFragments::FragmentKind::Text);
// For Objective-C `id` and `Class` pointers
// we do not spell out the `*`.
if (T->isObjCObjectPointerType() &&
!T->getAs<ObjCObjectPointerType>()->isObjCIdOrClassType()) {
Fragments.append(getFragmentsForType(T->getPointeeType(), Context, After));
// id<protocol> is an qualified id type
dangUnsubmitted
Not Done
ReplyInline Actions
Fragments.append(getFragmentsForType(T->getPointeeType(), Context, After));
- // id<protocol> is an qualified id type
+ // id<protocol> is a qualified id type
if (!T->getAs<ObjCObjectPointerType>()->isObjCQualifiedIdType()) {
dang:
dangUnsubmitted
Not Done
ReplyInline Actions

IIUC the distinction here is that id<Protocol> is a id qualified type, but id<Protocol> * is not? If so this is a fairly subtle point, and worth mentioning in this comment.

dang: IIUC the distinction here is that `id<Protocol>` is a `id` qualified type, but `id<Protocol> *`…
// id<protocol>* is not an qualified id type
if (!T->getAs<ObjCObjectPointerType>()->isObjCQualifiedIdType()) {
Fragments.append(" *", DeclarationFragments::FragmentKind::Text);
}
return Fragments;
} }
// Declaration fragments of a lvalue reference type is the declaration // Declaration fragments of a lvalue reference type is the declaration
// fragments of the underlying type followed by a `&`. // fragments of the underlying type followed by a `&`.
if (const LValueReferenceType *LRT = dyn_cast<LValueReferenceType>(T)) if (const LValueReferenceType *LRT = dyn_cast<LValueReferenceType>(T))
return Fragments return Fragments
.append( .append(
getFragmentsForType(LRT->getPointeeTypeAsWritten(), Context, After)) getFragmentsForType(LRT->getPointeeTypeAsWritten(), Context, After))
▲ Show 20 Lines • Show All 632 Lines • Show Last 20 Lines

clang/test/ExtractAPI/objc_id_protocol.m

  • This file was added.
// RUN: rm -rf %t
// RUN: split-file %s %t
// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
// RUN: %t/reference.output.json.in >> %t/reference.output.json
// RUN: %clang -extract-api -x objective-c-header -target arm64-apple-macosx \
// RUN: %t/input.h -o %t/output.json | FileCheck -allow-empty %s
// Generator version is not consistent across test runs, normalize it.
// RUN: sed -e "s@\"generator\": \".*\"@\"generator\": \"?\"@g" \
// RUN: %t/output.json >> %t/output-normalized.json
// RUN: diff %t/reference.output.json %t/output-normalized.json
// CHECK-NOT: error:
// CHECK-NOT: warning:
//--- input.h
@protocol MyProtocol
@end
@interface MyInterface
@property(copy, readwrite) id<MyProtocol> obj1;
@property(readwrite) id<MyProtocol> *obj2;
@end
//--- reference.output.json.in
{
"metadata": {
"formatVersion": {
"major": 0,
"minor": 5,
"patch": 3
},
"generator": "?"
},
"module": {
"name": "",
"platform": {
"architecture": "arm64",
"operatingSystem": {
"minimumVersion": {
"major": 11,
"minor": 0,
"patch": 0
},
"name": "macosx"
},
"vendor": "apple"
}
},
"relationships": [
{
"kind": "memberOf",
"source": "c:objc(cs)MyInterface(py)obj1",
"target": "c:objc(cs)MyInterface",
"targetFallback": "MyInterface"
},
{
"kind": "memberOf",
"source": "c:objc(cs)MyInterface(py)obj2",
"target": "c:objc(cs)MyInterface",
"targetFallback": "MyInterface"
}
],
"symbols": [
{
"accessLevel": "public",
"declarationFragments": [
{
"kind": "keyword",
"spelling": "@interface"
},
{
"kind": "text",
"spelling": " "
},
{
"kind": "identifier",
"spelling": "MyInterface"
}
],
"identifier": {
"interfaceLanguage": "objective-c",
"precise": "c:objc(cs)MyInterface"
},
"kind": {
"displayName": "Class",
"identifier": "objective-c.class"
},
"location": {
"position": {
"character": 12,
"line": 4
},
"uri": "file://INPUT_DIR/input.h"
},
"names": {
"navigator": [
{
"kind": "identifier",
"spelling": "MyInterface"
}
],
"subHeading": [
{
"kind": "identifier",
"spelling": "MyInterface"
}
],
"title": "MyInterface"
},
"pathComponents": [
"MyInterface"
]
},
{
"accessLevel": "public",
"declarationFragments": [
{
"kind": "keyword",
"spelling": "@property"
},
{
"kind": "text",
"spelling": " ("
},
{
"kind": "keyword",
"spelling": "atomic"
},
{
"kind": "text",
"spelling": ", "
},
{
"kind": "keyword",
"spelling": "copy"
},
{
"kind": "text",
"spelling": ", "
},
{
"kind": "keyword",
"spelling": "readwrite"
},
{
"kind": "text",
"spelling": ") "
},
{
"kind": "typeIdentifier",
"preciseIdentifier": "c:Qoobjc(pl)MyProtocol",
"spelling": "id<MyProtocol>"
},
{
"kind": "text",
"spelling": " "
},
{
"kind": "identifier",
"spelling": "obj1"
}
],
"identifier": {
"interfaceLanguage": "objective-c",
"precise": "c:objc(cs)MyInterface(py)obj1"
},
"kind": {
"displayName": "Instance Property",
"identifier": "objective-c.property"
},
"location": {
"position": {
"character": 43,
"line": 5
},
"uri": "file://INPUT_DIR/input.h"
},
"names": {
"navigator": [
{
"kind": "identifier",
"spelling": "obj1"
}
],
"subHeading": [
{
"kind": "identifier",
"spelling": "obj1"
}
],
"title": "obj1"
},
"pathComponents": [
"MyInterface",
"obj1"
]
},
{
"accessLevel": "public",
"declarationFragments": [
{
"kind": "keyword",
"spelling": "@property"
},
{
"kind": "text",
"spelling": " ("
},
{
"kind": "keyword",
"spelling": "atomic"
},
{
"kind": "text",
"spelling": ", "
},
{
"kind": "keyword",
"spelling": "assign"
},
{
"kind": "text",
"spelling": ", "
},
{
"kind": "keyword",
"spelling": "unsafe_unretained"
},
{
"kind": "text",
"spelling": ", "
},
{
"kind": "keyword",
"spelling": "readwrite"
},
{
"kind": "text",
"spelling": ") "
},
{
"kind": "typeIdentifier",
"preciseIdentifier": "c:Qoobjc(pl)MyProtocol",
"spelling": "id<MyProtocol>"
},
{
"kind": "text",
"spelling": " * "
},
{
"kind": "identifier",
"spelling": "obj2"
}
],
"identifier": {
"interfaceLanguage": "objective-c",
"precise": "c:objc(cs)MyInterface(py)obj2"
},
"kind": {
"displayName": "Instance Property",
"identifier": "objective-c.property"
},
"location": {
"position": {
"character": 38,
"line": 6
},
"uri": "file://INPUT_DIR/input.h"
},
"names": {
"navigator": [
{
"kind": "identifier",
"spelling": "obj2"
}
],
"subHeading": [
{
"kind": "identifier",
"spelling": "obj2"
}
],
"title": "obj2"
},
"pathComponents": [
"MyInterface",
"obj2"
]
},
{
"accessLevel": "public",
"declarationFragments": [
{
"kind": "keyword",
"spelling": "@protocol"
},
{
"kind": "text",
"spelling": " "
},
{
"kind": "identifier",
"spelling": "MyProtocol"
}
],
"identifier": {
"interfaceLanguage": "objective-c",
"precise": "c:objc(pl)MyProtocol"
},
"kind": {
"displayName": "Protocol",
"identifier": "objective-c.protocol"
},
"location": {
"position": {
"character": 11,
"line": 1
},
"uri": "file://INPUT_DIR/input.h"
},
"names": {
"navigator": [
{
"kind": "identifier",
"spelling": "MyProtocol"
}
],
"subHeading": [
{
"kind": "identifier",
"spelling": "MyProtocol"
}
],
"title": "MyProtocol"
},
"pathComponents": [
"MyProtocol"
]
}
]
}