[PATCH] D152770: [clang][ExtractAPI] Add support for Objective-C categories

[Zixu Wang via Phabricator via cfe-commits]

zixuw added a comment.

Thanks for the patch!

Like I mentioned in the inline comments, I'm not sure I understand the need of 
the "abstract" (in the sense that it's not really a concrete API) subtype of 
`APIRecord`. The design doesn't feel right within the existing structure.
If I recall the original issue correctly the problem is that a category might 
extend an interface that is not declared within the current compilation unit 
and the SymbolGraph output has references to an USR it cannot resolve. But 
that's why we have the `SymbolReference Interface` for the category record, and 
all the information we could possibly have from extract-api for that interface 
is already in the `SymbolReference`. Couldn't we use that to design an output 
format to indicate the external reference?

Also a side note that the word "module" has been overloaded, especially within 
LLVM/clang where "module" is used for clang modules or C++ modules. I would 
avoid creating a `*ModuleRecord` without qualifying it like `*ExtractAPIModule` 
and providing documentation to clarify the meaning.




Comment at: clang/include/clang/ExtractAPI/API.h:148-150
+  APIRecord(RecordKind Kind, StringRef USR, StringRef Name)
+  : USR(USR), Name(Name), Kind(Kind) {}
+

I would really try to avoid patching the base `APIRecord` to get ways of 
creating "fake" records. The API records in the API set should really be all 
concrete symbols mapping back to an entity that is an API.
This doesn't feel right and might open up to future bad designs and bugs.



Comment at: clang/include/clang/ExtractAPI/API.h:497-510
+struct ObjCCategoryModuleRecord : APIRecord {
+  // ObjCCategoryRecord%s are stored in and owned by APISet.
+  SmallVector Categories;
+
+  ObjCCategoryModuleRecord(StringRef USR, StringRef Name)
+  : APIRecord(RK_ObjCCategoryModule, USR, Name) {}
+

I'm still not convinced by this concept and design. Why do we need to have a 
subtype of `APIRecord` to represent relationships with external types from 
another "module"? We have `SymbolReference` for that purpose with the name and 
USR information.

Even if this is necessary, the name is extremely confusing. By the look of the 
rest of the change and the test cases, I believe this is meant to represent the 
external *interface* that a category is extending, not a category.



Comment at: clang/include/clang/ExtractAPI/ExtractAPIVisitor.h:128-135
+template 
+static bool isExternalCategory(const T , const StringRef ) {
+  for (const auto  : Records) {
+if (Name == Record.second.get()->Name)
+  return false;
+  }
+  return true;

This looks very concerning..
It's way too generic for its name and it's not doing what it's describing.
By the look of the call site below, this seems to be supposed to check if an 
*interface*, not category, is recorded in this API set. There's no reason to 
have a separate template function for that. You can just do the check inline 
and also probably covered by one of the existing llvm STLExtra utilities.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152770/new/

https://reviews.llvm.org/D152770

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146759: [ExtractAPI] Remove extra attributes in property declaration fragments

[Zixu Wang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG32b53cf9d0c8: [ExtractAPI] Remove extra attributes in 
property declaration fragments (authored by Unique_Usman, committed by zixuw).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146759/new/

https://reviews.llvm.org/D146759

Files:
  clang/lib/ExtractAPI/DeclarationFragments.cpp
  clang/test/ExtractAPI/objc_category.m
  clang/test/ExtractAPI/objc_id_protocol.m
  clang/test/ExtractAPI/objc_interface.m
  clang/test/ExtractAPI/objc_property.m

Index: clang/test/ExtractAPI/objc_property.m
===
--- clang/test/ExtractAPI/objc_property.m
+++ clang/test/ExtractAPI/objc_property.m
@@ -161,38 +161,6 @@
   "kind": "keyword",
   "spelling": "class"
 },
-{
-  "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": ") "
@@ -255,39 +223,7 @@
 },
 {
   "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": ") "
+  "spelling": " "
 },
 {
   "kind": "typeIdentifier",
@@ -353,38 +289,6 @@
   "kind": "keyword",
   "spelling": "class"
 },
-{
-  "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": ") "
@@ -447,39 +351,7 @@
 },
 {
   "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": ") "
+  "spelling": " "
 },
 {
   "kind": "typeIdentifier",
@@ -595,38 +467,6 @@
   "kind": "keyword",
   "spelling": "class"
 },
-{
-  "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"
-},
 {
   

[PATCH] D146656: [clang][ExtractAPI] Refactor ExtractAPIVisitor to make it more extensible

[Zixu Wang via Phabricator via cfe-commits]

zixuw added a comment.

LGTM for the `ExtractAPIVisitor` part.
Remaining:

- update test with `@LINE`
- the libclang side


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146656/new/

https://reviews.llvm.org/D146656

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146656: [clang][ExtractAPI] Refactor ExtractAPIVisitor to make it more extensible

[Zixu Wang via Phabricator via cfe-commits]

zixuw added inline comments.



Comment at: clang/include/clang/ExtractAPI/ExtractAPIVisitor.h:628
+public:
+  ExtractAPIVisitor(ASTContext , APISet ) : Base(Context, API) {}
+

Same as above, should the constructor be `protected`? I guess it depends if we 
want people to just instantiate and use a `ExtractAPIVisitor`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146656/new/

https://reviews.llvm.org/D146656

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146656: [clang][ExtractAPI] Refactor ExtractAPIVisitor to make it more extensible

[Zixu Wang via Phabricator via cfe-commits]

zixuw added a comment.

Good to see refactoring to make ExtractAPI easier to extend and use. Thanks 
Daniel!
Some comments on the ExtractAPIVisitor part. I'll leave the libclang part to 
the experts.




Comment at: clang/include/clang/ExtractAPI/ExtractAPIVisitor.h:32
+template 
+class ExtractAPIVisitorBase : public RecursiveASTVisitor {
 public:

Would it be better to call this `ExtractAPIVisitorImpl` because it provides the 
visitor implementation, and the `ExtractAPIVisitor` is actually the base for 
the second level CRTP for actual visitors?



Comment at: clang/include/clang/ExtractAPI/ExtractAPIVisitor.h:32
+template 
+class ExtractAPIVisitorBase : public RecursiveASTVisitor {
 public:

zixuw wrote:
> Would it be better to call this `ExtractAPIVisitorImpl` because it provides 
> the visitor implementation, and the `ExtractAPIVisitor` is actually the base 
> for the second level CRTP for actual visitors?
Should this be `: public RecursiveASTVisitor>` 
instead?



Comment at: clang/include/clang/ExtractAPI/ExtractAPIVisitor.h:34
 public:
-  ExtractAPIVisitor(ASTContext ,
-llvm::unique_function 
LocationChecker,
-APISet )
-  : Context(Context), API(API),
-LocationChecker(std::move(LocationChecker)) {}
+  ExtractAPIVisitorBase(ASTContext , APISet )
+  : Context(Context), API(API) {}

Should the constructor be made `protected` for a CRTP base?



Comment at: clang/include/clang/ExtractAPI/ExtractAPIVisitor.h:57
+
+  const RawComment *fetchRawCommentForDecl(const Decl *Decl) const;
+

Why does comment-fetching need to be dispatched?



Comment at: clang/include/clang/ExtractAPI/ExtractAPIVisitor.h:102
+private:
+  Derived () { return *static_cast(this); }
+};

I see that the `RecursiveASTVisitor` already provides a `getDerived()` for the 
top-level CRTP.
My head is still bending trying to get a clear view of the multi-level CRTP 
here  , but I'm guessing that this is to avoid the conflict with that method.
In that case could we be more verbose to make clear the purpose of this one? 
I'm thinking something like `getDerivedExtractAPIVisitor()`, to communicate 
that this is for getting the derived/concrete ExtractAPIVisitor subclass.



Comment at: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:170
   bool operator()(SourceLocation Loc) {
-// If the loc refers to a macro expansion we need to first get the file
+// If the loc refersSourceLocationxpansion we need to first get the file
 // location of the expansion.

bnbarham wrote:
> Accidental?
Missed change?



Comment at: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:228-234
+// Check that we have the definition for redeclarable types.
+if (auto *TD = llvm::dyn_cast(D))
+  ShouldBeIncluded = TD->isThisDeclarationADefinition();
+else if (auto *Interface = llvm::dyn_cast(D))
+  ShouldBeIncluded = Interface->isThisDeclarationADefinition();
+else if (auto *Protocol = llvm::dyn_cast(D))
+  ShouldBeIncluded = Protocol->isThisDeclarationADefinition();

Couldn't find the original logic in this patch. Could you remind me what are 
these for? Also good to have more comments here to explain things.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146656/new/

https://reviews.llvm.org/D146656

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144672: [Sanitizers] Error when attempting to use `static-lsan` with `TSan` or `Asan` on darwin

[Zixu Wang via Phabricator via cfe-commits]

zixuw added inline comments.



Comment at: 
compiler-rt/test/asan/TestCases/replaceable_new_delete_static.cpp:10-11
+
+// darwin only supports shared-libsan, so this should fail.
+// XFAIL: darwin
+ 

dmaclach wrote:
> yln wrote:
> > This should work, right?
> No.. darwin should fail with the `-static-libsan` flag. This is the test that 
> was failing and caused the rollback.
I think @yln is suggesting using `REQUIRES: asan-static-runtime` instead of 
`XFAIL: darwin`. I wasn't aware of that conditional but yeah that should be 
better if it works.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144672/new/

https://reviews.llvm.org/D144672

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144672: [Sanitizers] Error when attempting to use `static-lsan` with `TSan` or `Asan` on darwin

[Zixu Wang via Phabricator via cfe-commits]

zixuw added a comment.

Hi! This broke the asan replaceable_new_delete.cpp test on Darwin because it 
has a run line using `-static-libsan`. Could you take a look? Probably need to 
separate that check out and mark as unsupported on Darwin


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144672/new/

https://reviews.llvm.org/D144672

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D140010: [clang][ExtractAPI] Fix naming of typedef'd anonymous enums

[Zixu Wang via Phabricator via cfe-commits]

zixuw accepted this revision.
zixuw added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/ExtractAPI/ExtractAPIVisitor.cpp:174-175
+  StringRef Name = Decl->getName();
   if (Name.empty())
 Name = getTypedefName(Decl);
+  if (Name.empty()) {

dang wrote:
> zixuw wrote:
> > dang wrote:
> > > zixuw wrote:
> > > > Aren't these two lines supposed to do this?
> > > Qualified name is never empty, just contains some version of anonymous, 
> > > whereas getName() is actually empty for anonymous types. The flow is now, 
> > > try to populate with `getName` (which is unqualified and puts us in a 
> > > better spot for the eventual support of c++ and nested types). and if 
> > > that doesn't work fallback to the qualified name.
> > Sorry I meant `getTypedefName(Decl)`, which also tries to get the typedef 
> > name for an anonymous tag.
> > The patch summary says "Anonymous enums that are typedef'd should take on 
> > the name of the typedef." and I was a bit confused of the change.
> > 
> > Now we have three (two fallbacks) for Enum:
> > 1. straightforward `Decl->getName()`, and if that's empty
> > 2. `getTypedefName(Decl)`, which calls `Decl->getTypedefNameForAnonDecl()`, 
> > and if that fails,
> > 3. `Decl->printQualifiedName(OS)`
> > 
> > My questions:
> > 1. Do we need all three? We should be able to get rid of at least one 
> > right? The logic seems to be: if enum is named, use it. Otherwise get 
> > typedef name if it's part of a typedef. And finally leave something like 
> > `(anonymous)` if it's just an unattached anonymous enum.
> > 2. For the `printQualifiedName` path, isn't `Name` referencing a local 
> > string inside the method?
> > 3. structs are also tags that can be anonymous and inside a typedef, do we 
> > need similar changes there?
> > 
> > 
> 1. Yup that is exactly what I am trying to do here. If you can think of a way 
> of removing a chunk then let's do it. The reason for the third fallback is to 
> have a name for the container of an anonymous (without a typedef) top level 
> enum (there is an example of that in the enum.c test case)
> 2. Not entirely sure what you mean. If you are referring to the fact that the 
> backing storage for `Name` in this case is a local variable, this is fine 
> because we copy the string into the `APISet` when constructing the record 
> within this method a bit further down.
> 3. The struct case is a little different, It's not possible to have to have 
> an anonymous struct without a typedef at the top level, whereas it is 
> possible to do so with an enum (e.g. to introduce some constants). Currently 
> for structs we only do `Decl->getName()` and falls back to 
> `getTypedefName(Decl)`. If that fails we ignore the struct because it must be 
> a nested anonymous struct (that logic is flawed and we should aim to support 
> anonymous structs properly, but that would be a follow up patch to this one).
Ah I see. So before this patch we had `getQualifiedNameAsString` which would 
always bypass the following `getTypedefName` even if it's an anonymous within a 
typedef. This patch cleared out the logic and ordering.

> we copy the string into the APISet when constructing the record within this 
> method a bit further down.
 right, that part was folded in the diff view and I totally forgot we had that 
there...

LGTM. Thanks Daniel!





Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140010/new/

https://reviews.llvm.org/D140010

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D140010: [clang][ExtractAPI] Fix naming of typedef'd anonymous enums

[Zixu Wang via Phabricator via cfe-commits]

zixuw added inline comments.



Comment at: clang/lib/ExtractAPI/ExtractAPIVisitor.cpp:174-175
+  StringRef Name = Decl->getName();
   if (Name.empty())
 Name = getTypedefName(Decl);
+  if (Name.empty()) {

dang wrote:
> zixuw wrote:
> > Aren't these two lines supposed to do this?
> Qualified name is never empty, just contains some version of anonymous, 
> whereas getName() is actually empty for anonymous types. The flow is now, try 
> to populate with `getName` (which is unqualified and puts us in a better spot 
> for the eventual support of c++ and nested types). and if that doesn't work 
> fallback to the qualified name.
Sorry I meant `getTypedefName(Decl)`, which also tries to get the typedef name 
for an anonymous tag.
The patch summary says "Anonymous enums that are typedef'd should take on the 
name of the typedef." and I was a bit confused of the change.

Now we have three (two fallbacks) for Enum:
1. straightforward `Decl->getName()`, and if that's empty
2. `getTypedefName(Decl)`, which calls `Decl->getTypedefNameForAnonDecl()`, and 
if that fails,
3. `Decl->printQualifiedName(OS)`

My questions:
1. Do we need all three? We should be able to get rid of at least one right? 
The logic seems to be: if enum is named, use it. Otherwise get typedef name if 
it's part of a typedef. And finally leave something like `(anonymous)` if it's 
just an unattached anonymous enum.
2. For the `printQualifiedName` path, isn't `Name` referencing a local string 
inside the method?
3. structs are also tags that can be anonymous and inside a typedef, do we need 
similar changes there?




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140010/new/

https://reviews.llvm.org/D140010

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D140010: [clang][ExtractAPI] Fix naming of typedef'd anonymous enums

[Zixu Wang via Phabricator via cfe-commits]

zixuw added inline comments.



Comment at: clang/lib/ExtractAPI/ExtractAPIVisitor.cpp:174-175
+  StringRef Name = Decl->getName();
   if (Name.empty())
 Name = getTypedefName(Decl);
+  if (Name.empty()) {

Aren't these two lines supposed to do this?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140010/new/

https://reviews.llvm.org/D140010

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D139115: [clang][ExtractAPI] Add support for single symbol SGF

[Zixu Wang via Phabricator via cfe-commits]

zixuw added inline comments.



Comment at: clang/include/clang/ExtractAPI/API.h:128-130
+  /// Store hierarchy information for a given record.
+  ///
+  /// This is roughly analogous to the DeclContext hierarchy for an AST Node.

Misplaced comment?



Comment at: clang/include/clang/ExtractAPI/API.h:234
 ReadOnly = 1,
-Class = 1 << 1,
 Dynamic = 1 << 2,

What's the reason for refactoring out instance vs. class property? Looks like 
this should be a separated patch



Comment at: clang/include/clang/ExtractAPI/API.h:301
 
   static bool classof(const APIRecord *Record) {
+return Record->getKind() == RK_ObjCInstanceProperty ||

No need for `classof` in an abstract node



Comment at: clang/include/clang/ExtractAPI/API.h:393
 
   static bool classof(const APIRecord *Record) {
+return Record->getKind() == RK_ObjCClassMethod ||

No need for `classof` in an abstract node



Comment at: clang/include/clang/ExtractAPI/API.h:401
+
+struct ObjCInstanceMethodRecord : ObjCMethodRecord {
+  ObjCInstanceMethodRecord(StringRef USR, StringRef Name, PresumedLoc Loc,

Same question as instance/class property



Comment at: clang/include/clang/ExtractAPI/API.h:478
   virtual ~ObjCContainerRecord() = 0;
+
+  static bool classof(const APIRecord *Record) {

No need for `classof` in an abstract node



Comment at: clang/include/clang/ExtractAPI/DeclarationFragments.h:91
+/// The associated declaration, if applicable.
+const Decl *Declaration;
+

Is the decl guaranteed to be available when `Fragment` is used?
In our existing use case this might be fine as the serializer will be used 
while the AST is still alive. But this is actually the first thing that adds 
the dependency to the AST IIRC. So this makes the whole APISet dependent on the 
AST



Comment at: clang/lib/ExtractAPI/API.cpp:145
+APISet::addObjCInterface(StringRef Name, StringRef USR, PresumedLoc Loc,
+ AvailabilitySet Availabilities, LinkageInfo Linkage,
+ const DocComment ,

nit: tab



Comment at: clang/lib/ExtractAPI/API.cpp:169
+Record = std::make_unique(
+USR, Name, Loc, std::move(Availabilities), Comment, Declaration,
+SubHeading, Signature, IsFromSystemHeader);

nit: tab



Comment at: clang/lib/ExtractAPI/API.cpp:193
+Record = std::make_unique(
+USR, Name, Loc, std::move(Availabilities), Comment, Declaration,
+SubHeading, Attributes, GetterName, SetterName, IsOptional,

nit: tab



Comment at: clang/tools/c-index-test/c-index-test.c:4927-4928
+  fprintf(stderr,
+  "   c-index-test -write-pch  \n"
+  "   c-index-test -compilation-db [lookup ] 
database\n");
   fprintf(stderr,

nit: tab


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139115/new/

https://reviews.llvm.org/D139115

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136455: [clang][ExtractAPI] Add targetFallback to relationships in symbol graph

[Zixu Wang via Phabricator via cfe-commits]

zixuw added a comment.

Is it easy to/worth checking if the target is actually outside of the current 
module to keep the output smaller and concise?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136455/new/

https://reviews.llvm.org/D136455

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135295: [clang][ExtractAPI] Don't print locations for anonymous tags

[Zixu Wang via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5301826fa86a: [clang][ExtractAPI] Dont print locations 
for anonymous tags (authored by zixuw).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135295/new/

https://reviews.llvm.org/D135295

Files:
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp


Index: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
===
--- clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
+++ clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
@@ -850,6 +850,11 @@
   CI.getPreprocessor().addPPCallbacks(std::make_unique(
   CI.getSourceManager(), *LCF, *API, CI.getPreprocessor()));
 
+  // Do not include location in anonymous decls.
+  PrintingPolicy Policy = CI.getASTContext().getPrintingPolicy();
+  Policy.AnonymousTagLocations = false;
+  CI.getASTContext().setPrintingPolicy(Policy);
+
   return std::make_unique(CI.getASTContext(),
   std::move(LCF), *API);
 }


Index: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
===
--- clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
+++ clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
@@ -850,6 +850,11 @@
   CI.getPreprocessor().addPPCallbacks(std::make_unique(
   CI.getSourceManager(), *LCF, *API, CI.getPreprocessor()));
 
+  // Do not include location in anonymous decls.
+  PrintingPolicy Policy = CI.getASTContext().getPrintingPolicy();
+  Policy.AnonymousTagLocations = false;
+  CI.getASTContext().setPrintingPolicy(Policy);
+
   return std::make_unique(CI.getASTContext(),
   std::move(LCF), *API);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135295: [clang][ExtractAPI] Don't print locations for anonymous tags

[Zixu Wang via Phabricator via cfe-commits]

zixuw added a comment.

In D135295#3837895 , @zixuw wrote:

> In D135295#3837855 , @zixuw wrote:
>
>> In D135295#3837734 , @ributzka 
>> wrote:
>>
>>> This doesn't affect any tests?
>>
>> Just finished building locally, running tests to verify now
>
> Doesn't affect any test for this patch as expected. Still need to see how 
> does it interacts with https://reviews.llvm.org/D134813.

Interaction with D134813  looks good. Landing 
this


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135295/new/

https://reviews.llvm.org/D135295

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134813: Properly print unnamed TagDecl objects in diagnostics

[Zixu Wang via Phabricator via cfe-commits]

zixuw added a comment.

With the PrintingPolicy fix in https://reviews.llvm.org/D135295 and landed USR 
fix, the diff within ExtractAPI tests is only the wording with anonymous enums, 
and we can drop the lit change:

  658c658
  < "spelling": "(anonymous)"
  ---
  > "spelling": "enum (unnamed)"
  661c661
  < "title": "(anonymous)"
  ---
  > "title": "enum (unnamed)"
  664c664
  < "(anonymous)"
  ---
  > "enum (unnamed)"
  706c706
  < "(anonymous)",
  ---
  > "enum (unnamed)",
  746c746
  < "spelling": "(anonymous)"
  ---
  > "spelling": "enum (unnamed)"
  749c749
  < "title": "(anonymous)"
  ---
  > "title": "enum (unnamed)"
  752c752
  < "(anonymous)"
  ---
  > "enum (unnamed)"
  794c794
  < "(anonymous)",
  ---
  > "enum (unnamed)",

@dang @QuietMisdreavus for this change.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134813/new/

https://reviews.llvm.org/D134813

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134813: Properly print unnamed TagDecl objects in diagnostics

[Zixu Wang via Phabricator via cfe-commits]

zixuw added a comment.

I'm pulling this on top of https://reviews.llvm.org/D135295 to try locally now.




Comment at: clang/include/clang/AST/Decl.h:3647
 
+  void printName(raw_ostream , const PrintingPolicy ) const;
+

nit: missing an `override` here. (`warning: 'printName' overrides a member 
function but is not marked 'override' [-Winconsistent-missing-override]`)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134813/new/

https://reviews.llvm.org/D134813

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135295: [clang][ExtractAPI] Don't print locations for anonymous tags

[Zixu Wang via Phabricator via cfe-commits]

zixuw marked an inline comment as done.
zixuw added a comment.

In D135295#3837855 , @zixuw wrote:

> In D135295#3837734 , @ributzka 
> wrote:
>
>> This doesn't affect any tests?
>
> Just finished building locally, running tests to verify now

Doesn't affect any test for this patch as expected. Still need to see how does 
it interacts with https://reviews.llvm.org/D134813.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135295/new/

https://reviews.llvm.org/D135295

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135295: [clang][ExtractAPI] Don't print locations for anonymous tags

[Zixu Wang via Phabricator via cfe-commits]

zixuw updated this revision to Diff 465505.
zixuw added a comment.

Update on top of the existing PrintingPolicy in the ASTContext instead of 
creating a new default one.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135295/new/

https://reviews.llvm.org/D135295

Files:
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp


Index: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
===
--- clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
+++ clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
@@ -850,6 +850,11 @@
   CI.getPreprocessor().addPPCallbacks(std::make_unique(
   CI.getSourceManager(), *LCF, *API, CI.getPreprocessor()));
 
+  // Do not include location in anonymous decls.
+  PrintingPolicy Policy = CI.getASTContext().getPrintingPolicy();
+  Policy.AnonymousTagLocations = false;
+  CI.getASTContext().setPrintingPolicy(Policy);
+
   return std::make_unique(CI.getASTContext(),
   std::move(LCF), *API);
 }


Index: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
===
--- clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
+++ clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
@@ -850,6 +850,11 @@
   CI.getPreprocessor().addPPCallbacks(std::make_unique(
   CI.getSourceManager(), *LCF, *API, CI.getPreprocessor()));
 
+  // Do not include location in anonymous decls.
+  PrintingPolicy Policy = CI.getASTContext().getPrintingPolicy();
+  Policy.AnonymousTagLocations = false;
+  CI.getASTContext().setPrintingPolicy(Policy);
+
   return std::make_unique(CI.getASTContext(),
   std::move(LCF), *API);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135295: [clang][ExtractAPI] Don't print locations for anonymous tags

[Zixu Wang via Phabricator via cfe-commits]

zixuw added inline comments.



Comment at: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:854
+  // Do not include location in anonymous decls.
+  PrintingPolicy Policy(CI.getASTContext().getLangOpts());
+  Policy.AnonymousTagLocations = false;

sammccall wrote:
> nit: this isn't *just* clearing AnonymousTagLocations, but also resetting any 
> other PP flags to defaults based on LangOpts. You could getPrintingPolicy() 
> instead if you didn't intend to do this.
Makes sense. Probably doesn't matter because I'm pretty sure `PrintingPolicy` 
for ExtractAPI is just the default now. Still, good to use `getPrintingPolicy` 
to be sure


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135295/new/

https://reviews.llvm.org/D135295

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135295: [clang][ExtractAPI] Don't print locations for anonymous tags

[Zixu Wang via Phabricator via cfe-commits]

zixuw added a comment.

In D135295#3837734 , @ributzka wrote:

> This doesn't affect any tests?

Just finished building locally, running tests to verify now


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135295/new/

https://reviews.llvm.org/D135295

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135295: [clang][ExtractAPI] Don't print locations for anonymous tags

[Zixu Wang via Phabricator via cfe-commits]

zixuw updated this revision to Diff 465486.
zixuw added a comment.

Set PrintingPolicy properly


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135295/new/

https://reviews.llvm.org/D135295

Files:
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp


Index: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
===
--- clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
+++ clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
@@ -850,6 +850,11 @@
   CI.getPreprocessor().addPPCallbacks(std::make_unique(
   CI.getSourceManager(), *LCF, *API, CI.getPreprocessor()));
 
+  // Do not include location in anonymous decls.
+  PrintingPolicy Policy(CI.getASTContext().getLangOpts());
+  Policy.AnonymousTagLocations = false;
+  CI.getASTContext().setPrintingPolicy(Policy);
+
   return std::make_unique(CI.getASTContext(),
   std::move(LCF), *API);
 }


Index: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
===
--- clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
+++ clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
@@ -850,6 +850,11 @@
   CI.getPreprocessor().addPPCallbacks(std::make_unique(
   CI.getSourceManager(), *LCF, *API, CI.getPreprocessor()));
 
+  // Do not include location in anonymous decls.
+  PrintingPolicy Policy(CI.getASTContext().getLangOpts());
+  Policy.AnonymousTagLocations = false;
+  CI.getASTContext().setPrintingPolicy(Policy);
+
   return std::make_unique(CI.getASTContext(),
   std::move(LCF), *API);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134813: Properly print unnamed TagDecl objects in diagnostics

[Zixu Wang via Phabricator via cfe-commits]

zixuw added a comment.

Setting `PrintingPolicy::AnonymousTagLocations` to `false` in 
https://reviews.llvm.org/D135295


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134813/new/

https://reviews.llvm.org/D134813

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135295: [clang][ExtractAPI] Don't print locations for anonymous tags

[Zixu Wang via Phabricator via cfe-commits]

zixuw created this revision.
Herald added a subscriber: ributzka.
Herald added a reviewer: dang.
Herald added a reviewer: ributzka.
Herald added a project: All.
zixuw requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

ExtractAPI doesn't care about locations of anonymous TagDecls. Set the
printing policy to exclude that from anonymous decl names.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135295

Files:
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp


Index: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
===
--- clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
+++ clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
@@ -850,6 +850,9 @@
   CI.getPreprocessor().addPPCallbacks(std::make_unique(
   CI.getSourceManager(), *LCF, *API, CI.getPreprocessor()));
 
+  // Do not include location in anonymous decls.
+  CI.getASTContext().getPrintingPolicy().AnonymousTagLocations = false;
+
   return std::make_unique(CI.getASTContext(),
   std::move(LCF), *API);
 }


Index: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
===
--- clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
+++ clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
@@ -850,6 +850,9 @@
   CI.getPreprocessor().addPPCallbacks(std::make_unique(
   CI.getSourceManager(), *LCF, *API, CI.getPreprocessor()));
 
+  // Do not include location in anonymous decls.
+  CI.getASTContext().getPrintingPolicy().AnonymousTagLocations = false;
+
   return std::make_unique(CI.getASTContext(),
   std::move(LCF), *API);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134813: Properly print unnamed TagDecl objects in diagnostics

[Zixu Wang via Phabricator via cfe-commits]

zixuw added a comment.

Ah sorry I just finished reading the discussions. IIUC 
20c9ac29250493f5e0a3791dc1e5e9114ff0dc6e 
 should 
have already fixed the USR generation part, and all of the USR updates in the 
test cases should be gone now?

For the name part (`title`) I believe it's less likely to cause any problem, 
but would still be good to get rid of the location info just for the sake of 
keeping diff working and not having to change lit  (yeah probably using diff 
was not a good idea in the first place.. )


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134813/new/

https://reviews.llvm.org/D134813

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134813: Properly print unnamed TagDecl objects in diagnostics

[Zixu Wang via Phabricator via cfe-commits]

zixuw added inline comments.



Comment at: clang/test/ExtractAPI/typedef_anonymous_record.c:74
 "interfaceLanguage": "objective-c",
-"precise": "c:@SA@MyStruct"
+"precise": "c:@S@MyStruct"
   },

Why is the `A` dropped here? Isn't this USR still referring to an anonymous 
struct (`typedef struct { } MyStruct;`)?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134813/new/

https://reviews.llvm.org/D134813

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134813: Properly print unnamed TagDecl objects in diagnostics

[Zixu Wang via Phabricator via cfe-commits]

zixuw added a subscriber: QuietMisdreavus.
zixuw added a comment.

In D134813#3836836 , @aaron.ballman 
wrote:

>> I'd guess we need some kind of change to CommentXML and ExtractAPI, but I 
>> don't know enough to be sure what it should be.
>
> Ping @dang @zixuw and @dexonsmith for questions about how to handle 
> ExtractAPI changes, and @gribozavr for questions about CommentXML

Thanks for the heads up! Looking.

At a glance it seems that ExtractAPI would like to fall back to the original 
formats to not break downstream tooling. But not sure if it's fine because it's 
still consistent. As far as I can tell this only affects anonymous `TagDecl`s. 
@QuietMisdreavus thoughts?

IMO we would still want to keep the ExtractAPI outputs unchanged if it's easy 
enough to do. We're using `getName()/getQualifiedNameAsString()` for names and 
`index::generateUSRForDecl()` for USRs. Is there a 'policy' or some 
alternatives to use? Or is the location info now part of the 'correct' format 
for USRs (seems a bit weird to see that as part of the USR)?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134813/new/

https://reviews.llvm.org/D134813

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130918: [clang][ExtractAPI] Record availability information on all platforms

[Zixu Wang via Phabricator via cfe-commits]

zixuw accepted this revision.
zixuw added a comment.
This revision is now accepted and ready to land.

LGTM!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130918/new/

https://reviews.llvm.org/D130918

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130918: [clang][ExtractAPI] Record availability information on all platforms

[Zixu Wang via Phabricator via cfe-commits]

zixuw added a comment.

Thanks Daniel!




Comment at: clang/test/ExtractAPI/availability.c:16
+//--- input.h
+/// Documentation for a.
+void a(void);

Do we care about doc comments for this particular test?
The testing convention for all the ExtractAPI tests is probably not the most 
ideal one because every test is a complete matching of the full Symbol Graph 
output.
So if we try to limit the scope of this test to just availability attributes it 
would be easier to read, and also avoid potential irrelevant failures in this 
test if something should broke the doc comment part.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130918/new/

https://reviews.llvm.org/D130918

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130583: [clang][ExtractAPI] Add a space between type and name in property declaration fragments

[Zixu Wang via Phabricator via cfe-commits]

zixuw accepted this revision.
zixuw added a comment.
This revision is now accepted and ready to land.

Awesome! Thanks Daniel!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130583/new/

https://reviews.llvm.org/D130583

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130581: [clang][ExtractAPI] Ensure that class properties have a kind of "Type Property"

[Zixu Wang via Phabricator via cfe-commits]

zixuw accepted this revision.
zixuw added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks Daniel!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130581/new/

https://reviews.llvm.org/D130581

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125678: [clang][extract-api] Don't emit symbols prefixed with an underscore

[Zixu Wang via Phabricator via cfe-commits]

zixuw accepted this revision.
zixuw added a comment.

In D125678#3517168 , @QuietMisdreavus 
wrote:

> clang-format failed:
>
>   ---  clang-format
>   
>   changed files:
>   
>   clang/test/ExtractAPI/underscored.c

I think it's fine. clang-format always get tripped on split-file tests because 
of the mixed format and what not.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125678/new/

https://reviews.llvm.org/D125678

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125061: [clang] A more robust way to attach comments

[Zixu Wang via Phabricator via cfe-commits]

zixuw added a comment.

Well, apparently there are still some corner-case bugs in the logic. ExtractAPI 
and a few other tests failed. Will look into it tomorrow.

And if folks have concerns or opinions of the performance/space cost of adding 
the extra pointer to `DeclBase`, I can try to keep the original logic and just 
special-case enum constants to find the previous enum constant decl in the 
parent enum. Not sure if there are existing ways to get preceding enum 
constants efficiently, otherwise it'd be an O(n^2) operation for an enum.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125061/new/

https://reviews.llvm.org/D125061

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125061: [clang] A more robust way to attach comments

[Zixu Wang via Phabricator via cfe-commits]

zixuw created this revision.
Herald added a subscriber: arphaman.
Herald added a reviewer: dang.
Herald added a project: All.
zixuw requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The current implementation in `getRawCommentForDeclNoCacheImpl` to check
if a leading comment is attached to a given `Decl *D` is to see if there
is any of `";{}#@"` between the comment and the source location of `D`,
which indicates that there's other declarations or preprocessor
directives that the comment is meant for. However, this does not work
for enum constants, which are separated by commas.
Comma was originally included in the filter characters list, but removed
in b534d3a0ef69 because macros, attributes and other decorators might
also introduce commas before a declaration. And there's no good way to
reliably and efficiently check if all the commas are from other
declarations.
This patch added a forward pointer in `DeclBase` pointing to the
previous declaration inside the lexical `DeclContext`, similar to the
existing `NextInContextAndBits`. This constructs a doubly linked list so
we can easily query the lexically preceding declaration of the current
decl `D`, to check if any decl falls between the comment and `D`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125061

Files:
  clang/include/clang/AST/DeclBase.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/DeclBase.cpp
  clang/test/Index/annotate-comments-enum-constant.c
  clang/test/Index/annotate-comments.cpp

Index: clang/test/Index/annotate-comments.cpp
===
--- clang/test/Index/annotate-comments.cpp
+++ clang/test/Index/annotate-comments.cpp
@@ -254,6 +254,14 @@
 MYMAC(0,0)
 void isdoxy54(int);
 
+/// IS_DOXYGEN_SINGLE
+struct isdoxy55 {
+  int notdoxy48;
+  /// IS_DOXYGEN_SINGLE
+  int isdoxy56;
+};
+bool notdoxy49;
+
 #endif
 
 // RUN: rm -rf %t
@@ -337,3 +345,6 @@
 // CHECK: annotate-comments.cpp:241:6: FunctionDecl=isdoxy52:{{.*}} BriefComment=[Aaa. IS_DOXYGEN_START Bbb.]
 // CHECK: annotate-comments.cpp:248:6: FunctionDecl=isdoxy53:{{.*}} BriefComment=[Aaa. IS_DOXYGEN_START IS_DOXYGEN_END]
 // CHECK: annotate-comments.cpp:255:6: FunctionDecl=isdoxy54:{{.*}} BriefComment=[Aaa. IS_DOXYGEN_START IS_DOXYGEN_END]
+
+// CHECK: annotate-comments.cpp:258:8: StructDecl=isdoxy55:{{.*}} IS_DOXYGEN_SINGLE
+// CHECK: annotate-comments.cpp:261:7: FieldDecl=isdoxy56:{{.*}} IS_DOXYGEN_SINGLE
Index: clang/test/Index/annotate-comments-enum-constant.c
===
--- /dev/null
+++ clang/test/Index/annotate-comments-enum-constant.c
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: c-index-test -test-load-source all -comments-xml-schema=%S/../../bindings/xml/comment-xml-schema.rng %s > %t/out
+// RUN: FileCheck %s --dump-input always < %t/out
+
+enum {
+  /// Documentation for Foo
+  Foo,
+  Bar, // No documentation for Bar
+  /// Documentation for Baz
+  Baz,
+};
+// CHECK: EnumConstantDecl=Foo:[[@LINE-5]]:3 (Definition) {{.*}} BriefComment=[Documentation for Foo] FullCommentAsHTML=[ Documentation for Foo] FullCommentAsXML=[Fooc:@Ea@Foo@FooFoo Documentation for Foo]
+// CHECK: EnumConstantDecl=Bar:[[@LINE-5]]:3 (Definition)
+// CHECK-NOT: BriefComment=[Documentation for Foo]
+// CHECK: EnumConstantDecl=Baz:[[@LINE-5]]:3 (Definition) {{.*}} BriefComment=[Documentation for Baz] FullCommentAsHTML=[ Documentation for Baz] FullCommentAsXML=[Bazc:@Ea@Foo@BazBaz Documentation for Baz]
Index: clang/lib/AST/DeclBase.cpp
===
--- clang/lib/AST/DeclBase.cpp
+++ clang/lib/AST/DeclBase.cpp
@@ -1344,6 +1344,7 @@
 else
   FirstNewDecl = D;
 
+D->PreviousInContext = PrevDecl;
 PrevDecl = D;
   }
 
@@ -1392,6 +1393,8 @@
   std::tie(ExternalFirst, ExternalLast) =
   BuildDeclChain(Decls, FieldsAlreadyLoaded);
   ExternalLast->NextInContextAndBits.setPointer(FirstDecl);
+  if (FirstDecl)
+FirstDecl->PreviousInContext = ExternalLast;
   FirstDecl = ExternalFirst;
   if (!LastDecl)
 LastDecl = ExternalLast;
@@ -1498,25 +1501,26 @@
   assert((D->NextInContextAndBits.getPointer() || D == LastDecl) &&
  "decl is not in decls list");
 
-  // Remove D from the decl chain.  This is O(n) but hopefully rare.
+  // Remove D from the decl chain.
   if (D == FirstDecl) {
 if (D == LastDecl)
   FirstDecl = LastDecl = nullptr;
 else
   FirstDecl = D->NextInContextAndBits.getPointer();
-  } else {
-for (Decl *I = FirstDecl; true; I = I->NextInContextAndBits.getPointer()) {
-  assert(I && "decl not found in linked list");
-  if (I->NextInContextAndBits.getPointer() == D) {
-I->NextInContextAndBits.setPointer(D->NextInContextAndBits.getPointer());
-if (D == LastDecl) LastDecl = I;
-break;
-  }
-}
+  }
+
+  if (auto *Next = 

[PATCH] D124964: Revert "Revert "[clang][extract-api] Use relative includes""

[Zixu Wang via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGcb5bb28511f2: Revert Revert [clang][extract-api] 
Use relative includes (authored by zixuw).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124964/new/

https://reviews.llvm.org/D124964

Files:
  clang/include/clang/ExtractAPI/FrontendActions.h
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/test/ExtractAPI/known_files_only_hmap.c
  clang/test/ExtractAPI/relative_include.m

Index: clang/test/ExtractAPI/relative_include.m
===
--- /dev/null
+++ clang/test/ExtractAPI/relative_include.m
@@ -0,0 +1,193 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// Setup framework root
+// RUN: mkdir -p %t/Frameworks/MyFramework.framework/Headers
+// RUN: cp %t/MyFramework.h %t/Frameworks/MyFramework.framework/Headers/
+// RUN: cp %t/MyHeader.h %t/Frameworks/MyFramework.framework/Headers/
+
+// RUN: sed -e "s@SRCROOT@%{/t:regex_replacement}@g" \
+// RUN: %t/reference.output.json.in >> %t/reference.output.json
+
+// Headermap maps headers to the source root SRCROOT
+// RUN: sed -e "s@SRCROOT@%{/t:regex_replacement}@g" \
+// RUN: %t/headermap.hmap.json.in >> %t/headermap.hmap.json
+// RUN: %hmaptool write %t/headermap.hmap.json %t/headermap.hmap
+
+// Input headers use paths to the framework root/DSTROOT
+// RUN: %clang_cc1 -extract-api -v --product-name=MyFramework \
+// RUN: -triple arm64-apple-macosx \
+// RUN: -iquote%t -I%t/headermap.hmap -F%t/Frameworks \
+// RUN: -x objective-c-header \
+// RUN: %t/Frameworks/MyFramework.framework/Headers/MyFramework.h \
+// RUN: %t/Frameworks/MyFramework.framework/Headers/MyHeader.h \
+// RUN: %t/QuotedHeader.h \
+// RUN: -o %t/output.json 2>&1 -verify | 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:  :
+// CHECK-NEXT: #import 
+// CHECK-NEXT: #import 
+// CHECK-NEXT: #import "QuotedHeader.h"
+
+//--- headermap.hmap.json.in
+{
+  "mappings" :
+{
+ "MyFramework/MyHeader.h" : "SRCROOT/MyHeader.h"
+}
+}
+
+//--- MyFramework.h
+// Umbrella for MyFramework
+#import 
+// expected-no-diagnostics
+
+//--- MyHeader.h
+#import 
+int MyInt;
+// expected-no-diagnostics
+
+//--- QuotedHeader.h
+char MyChar;
+// expected-no-diagnostics
+
+//--- Frameworks/OtherFramework.framework/Headers/OtherHeader.h
+int OtherInt;
+// expected-no-diagnostics
+
+//--- reference.output.json.in
+{
+  "metadata": {
+"formatVersion": {
+  "major": 0,
+  "minor": 5,
+  "patch": 3
+},
+"generator": "?"
+  },
+  "module": {
+"name": "MyFramework",
+"platform": {
+  "architecture": "arm64",
+  "operatingSystem": {
+"minimumVersion": {
+  "major": 11,
+  "minor": 0,
+  "patch": 0
+},
+"name": "macosx"
+  },
+  "vendor": "apple"
+}
+  },
+  "relationships": [],
+  "symbols": [
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:I",
+  "spelling": "int"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "MyInt"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "objective-c",
+"precise": "c:@MyInt"
+  },
+  "kind": {
+"displayName": "Global Variable",
+"identifier": "objective-c.var"
+  },
+  "location": {
+"position": {
+  "character": 5,
+  "line": 2
+},
+"uri": "file://SRCROOT/MyHeader.h"
+  },
+  "names": {
+"navigator": [
+  {
+"kind": "identifier",
+"spelling": "MyInt"
+  }
+],
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "MyInt"
+  }
+],
+"title": "MyInt"
+  },
+  "pathComponents": [
+"MyInt"
+  ]
+},
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:C",
+  "spelling": "char"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "MyChar"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "objective-c",
+"precise": "c:@MyChar"
+  },
+  "kind": {
+"displayName": "Global Variable",
+"identifier": "objective-c.var"
+  },
+  "location": {
+

[PATCH] D124964: Revert "Revert "[clang][extract-api] Use relative includes""

[Zixu Wang via Phabricator via cfe-commits]

zixuw added a comment.

In D124964#3492423 , @dang wrote:

> LGTM! If I understand the issue correctly we gave `llvm::Regex::match` a 
> string temporary to match against before and now we store it for long enough 
> to process the match results?

Yup, exactly.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124964/new/

https://reviews.llvm.org/D124964

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124964: Revert "Revert "[clang][extract-api] Use relative includes""

[Zixu Wang via Phabricator via cfe-commits]

zixuw updated this revision to Diff 427140.
zixuw added a comment.

Whitespace change: clang-format removed an empty line


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124964/new/

https://reviews.llvm.org/D124964

Files:
  clang/include/clang/ExtractAPI/FrontendActions.h
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/test/ExtractAPI/known_files_only_hmap.c
  clang/test/ExtractAPI/relative_include.m

Index: clang/test/ExtractAPI/relative_include.m
===
--- /dev/null
+++ clang/test/ExtractAPI/relative_include.m
@@ -0,0 +1,193 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// Setup framework root
+// RUN: mkdir -p %t/Frameworks/MyFramework.framework/Headers
+// RUN: cp %t/MyFramework.h %t/Frameworks/MyFramework.framework/Headers/
+// RUN: cp %t/MyHeader.h %t/Frameworks/MyFramework.framework/Headers/
+
+// RUN: sed -e "s@SRCROOT@%{/t:regex_replacement}@g" \
+// RUN: %t/reference.output.json.in >> %t/reference.output.json
+
+// Headermap maps headers to the source root SRCROOT
+// RUN: sed -e "s@SRCROOT@%{/t:regex_replacement}@g" \
+// RUN: %t/headermap.hmap.json.in >> %t/headermap.hmap.json
+// RUN: %hmaptool write %t/headermap.hmap.json %t/headermap.hmap
+
+// Input headers use paths to the framework root/DSTROOT
+// RUN: %clang_cc1 -extract-api -v --product-name=MyFramework \
+// RUN: -triple arm64-apple-macosx \
+// RUN: -iquote%t -I%t/headermap.hmap -F%t/Frameworks \
+// RUN: -x objective-c-header \
+// RUN: %t/Frameworks/MyFramework.framework/Headers/MyFramework.h \
+// RUN: %t/Frameworks/MyFramework.framework/Headers/MyHeader.h \
+// RUN: %t/QuotedHeader.h \
+// RUN: -o %t/output.json 2>&1 -verify | 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:  :
+// CHECK-NEXT: #import 
+// CHECK-NEXT: #import 
+// CHECK-NEXT: #import "QuotedHeader.h"
+
+//--- headermap.hmap.json.in
+{
+  "mappings" :
+{
+ "MyFramework/MyHeader.h" : "SRCROOT/MyHeader.h"
+}
+}
+
+//--- MyFramework.h
+// Umbrella for MyFramework
+#import 
+// expected-no-diagnostics
+
+//--- MyHeader.h
+#import 
+int MyInt;
+// expected-no-diagnostics
+
+//--- QuotedHeader.h
+char MyChar;
+// expected-no-diagnostics
+
+//--- Frameworks/OtherFramework.framework/Headers/OtherHeader.h
+int OtherInt;
+// expected-no-diagnostics
+
+//--- reference.output.json.in
+{
+  "metadata": {
+"formatVersion": {
+  "major": 0,
+  "minor": 5,
+  "patch": 3
+},
+"generator": "?"
+  },
+  "module": {
+"name": "MyFramework",
+"platform": {
+  "architecture": "arm64",
+  "operatingSystem": {
+"minimumVersion": {
+  "major": 11,
+  "minor": 0,
+  "patch": 0
+},
+"name": "macosx"
+  },
+  "vendor": "apple"
+}
+  },
+  "relationships": [],
+  "symbols": [
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:I",
+  "spelling": "int"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "MyInt"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "objective-c",
+"precise": "c:@MyInt"
+  },
+  "kind": {
+"displayName": "Global Variable",
+"identifier": "objective-c.var"
+  },
+  "location": {
+"position": {
+  "character": 5,
+  "line": 2
+},
+"uri": "file://SRCROOT/MyHeader.h"
+  },
+  "names": {
+"navigator": [
+  {
+"kind": "identifier",
+"spelling": "MyInt"
+  }
+],
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "MyInt"
+  }
+],
+"title": "MyInt"
+  },
+  "pathComponents": [
+"MyInt"
+  ]
+},
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:C",
+  "spelling": "char"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "MyChar"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "objective-c",
+"precise": "c:@MyChar"
+  },
+  "kind": {
+"displayName": "Global Variable",
+"identifier": "objective-c.var"
+  },
+  "location": {
+"position": {
+  "character": 6,
+  "line": 1
+},
+"uri": "file://SRCROOT/QuotedHeader.h"

[PATCH] D124964: Revert "Revert "[clang][extract-api] Use relative includes""

[Zixu Wang via Phabricator via cfe-commits]

zixuw created this revision.
zixuw added reviewers: ributzka, dang, cishida.
Herald added a project: All.
zixuw requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Reapply the change after fixing sanitizer errors.
The original problem was that `StringRef`s in `Matches` are pointing to
temporary local `std::string`s created by `path::convert_to_slash` in
the regex match call. This patch does the conversion up front in
container `FilePath`.

This reverts commit 2966f0fa505266735dbc8324b8821b7f0aa901ff 
.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124964

Files:
  clang/include/clang/ExtractAPI/FrontendActions.h
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/test/ExtractAPI/known_files_only_hmap.c
  clang/test/ExtractAPI/relative_include.m

Index: clang/test/ExtractAPI/relative_include.m
===
--- /dev/null
+++ clang/test/ExtractAPI/relative_include.m
@@ -0,0 +1,193 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// Setup framework root
+// RUN: mkdir -p %t/Frameworks/MyFramework.framework/Headers
+// RUN: cp %t/MyFramework.h %t/Frameworks/MyFramework.framework/Headers/
+// RUN: cp %t/MyHeader.h %t/Frameworks/MyFramework.framework/Headers/
+
+// RUN: sed -e "s@SRCROOT@%{/t:regex_replacement}@g" \
+// RUN: %t/reference.output.json.in >> %t/reference.output.json
+
+// Headermap maps headers to the source root SRCROOT
+// RUN: sed -e "s@SRCROOT@%{/t:regex_replacement}@g" \
+// RUN: %t/headermap.hmap.json.in >> %t/headermap.hmap.json
+// RUN: %hmaptool write %t/headermap.hmap.json %t/headermap.hmap
+
+// Input headers use paths to the framework root/DSTROOT
+// RUN: %clang_cc1 -extract-api -v --product-name=MyFramework \
+// RUN: -triple arm64-apple-macosx \
+// RUN: -iquote%t -I%t/headermap.hmap -F%t/Frameworks \
+// RUN: -x objective-c-header \
+// RUN: %t/Frameworks/MyFramework.framework/Headers/MyFramework.h \
+// RUN: %t/Frameworks/MyFramework.framework/Headers/MyHeader.h \
+// RUN: %t/QuotedHeader.h \
+// RUN: -o %t/output.json 2>&1 -verify | 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:  :
+// CHECK-NEXT: #import 
+// CHECK-NEXT: #import 
+// CHECK-NEXT: #import "QuotedHeader.h"
+
+//--- headermap.hmap.json.in
+{
+  "mappings" :
+{
+ "MyFramework/MyHeader.h" : "SRCROOT/MyHeader.h"
+}
+}
+
+//--- MyFramework.h
+// Umbrella for MyFramework
+#import 
+// expected-no-diagnostics
+
+//--- MyHeader.h
+#import 
+int MyInt;
+// expected-no-diagnostics
+
+//--- QuotedHeader.h
+char MyChar;
+// expected-no-diagnostics
+
+//--- Frameworks/OtherFramework.framework/Headers/OtherHeader.h
+int OtherInt;
+// expected-no-diagnostics
+
+//--- reference.output.json.in
+{
+  "metadata": {
+"formatVersion": {
+  "major": 0,
+  "minor": 5,
+  "patch": 3
+},
+"generator": "?"
+  },
+  "module": {
+"name": "MyFramework",
+"platform": {
+  "architecture": "arm64",
+  "operatingSystem": {
+"minimumVersion": {
+  "major": 11,
+  "minor": 0,
+  "patch": 0
+},
+"name": "macosx"
+  },
+  "vendor": "apple"
+}
+  },
+  "relationships": [],
+  "symbols": [
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:I",
+  "spelling": "int"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "MyInt"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "objective-c",
+"precise": "c:@MyInt"
+  },
+  "kind": {
+"displayName": "Global Variable",
+"identifier": "objective-c.var"
+  },
+  "location": {
+"position": {
+  "character": 5,
+  "line": 2
+},
+"uri": "file://SRCROOT/MyHeader.h"
+  },
+  "names": {
+"navigator": [
+  {
+"kind": "identifier",
+"spelling": "MyInt"
+  }
+],
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "MyInt"
+  }
+],
+"title": "MyInt"
+  },
+  "pathComponents": [
+"MyInt"
+  ]
+},
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:C",
+  "spelling": "char"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  

[PATCH] D123831: [clang][extract-api] Use relative includes

[Zixu Wang via Phabricator via cfe-commits]

zixuw added a comment.

Accidentally added another test case in my local workspace. Removed in 
5f841c71fc2cc77c92f526791cd7a938bcac69aa 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123831/new/

https://reviews.llvm.org/D123831

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123831: [clang][extract-api] Use relative includes

[Zixu Wang via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4c262fee08b5: [clang][extract-api] Use relative includes 
(authored by zixuw).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123831/new/

https://reviews.llvm.org/D123831

Files:
  clang/include/clang/ExtractAPI/FrontendActions.h
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/test/ExtractAPI/known_files_only_hmap.c
  clang/test/ExtractAPI/relative_include.m
  clang/test/Index/annotate-comments-enum-constant.c

Index: clang/test/Index/annotate-comments-enum-constant.c
===
--- /dev/null
+++ clang/test/Index/annotate-comments-enum-constant.c
@@ -0,0 +1,17 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: c-index-test -test-load-source all -comments-xml-schema=%S/../../bindings/xml/comment-xml-schema.rng %s > %t/out
+// RUN: FileCheck %s --dump-input always < %t/out
+
+enum {
+  /// Documentation for Foo
+  Foo,
+  Bar, // No documentation for Bar
+  /// Documentation for Baz
+  Baz,
+};
+// CHECK: EnumConstantDecl=Foo:[[@LINE-5]]:3 (Definition) {{.*}} BriefComment=[Documentation for Foo] FullCommentAsHTML=[ Documentation for Foo] FullCommentAsXML=[Fooc:@Ea@Foo@FooFoo Documentation for Foo]
+// CHECK: EnumConstantDecl=Bar:[[@LINE-5]]:3 (Definition)
+// CHECK-NOT: BriefComment=[Documentation for Foo]
+// CHECK: EnumConstantDecl=Baz:[[@LINE-5]]:3 (Definition) {{.*}} BriefComment=[Documentation for Baz] FullCommentAsHTML=[ Documentation for Baz] FullCommentAsXML=[Bazc:@Ea@Foo@BazBaz Documentation for Baz]
+
Index: clang/test/ExtractAPI/relative_include.m
===
--- /dev/null
+++ clang/test/ExtractAPI/relative_include.m
@@ -0,0 +1,193 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// Setup framework root
+// RUN: mkdir -p %t/Frameworks/MyFramework.framework/Headers
+// RUN: cp %t/MyFramework.h %t/Frameworks/MyFramework.framework/Headers/
+// RUN: cp %t/MyHeader.h %t/Frameworks/MyFramework.framework/Headers/
+
+// RUN: sed -e "s@SRCROOT@%{/t:regex_replacement}@g" \
+// RUN: %t/reference.output.json.in >> %t/reference.output.json
+
+// Headermap maps headers to the source root SRCROOT
+// RUN: sed -e "s@SRCROOT@%{/t:regex_replacement}@g" \
+// RUN: %t/headermap.hmap.json.in >> %t/headermap.hmap.json
+// RUN: %hmaptool write %t/headermap.hmap.json %t/headermap.hmap
+
+// Input headers use paths to the framework root/DSTROOT
+// RUN: %clang_cc1 -extract-api -v --product-name=MyFramework \
+// RUN: -triple arm64-apple-macosx \
+// RUN: -iquote%t -I%t/headermap.hmap -F%t/Frameworks \
+// RUN: -x objective-c-header \
+// RUN: %t/Frameworks/MyFramework.framework/Headers/MyFramework.h \
+// RUN: %t/Frameworks/MyFramework.framework/Headers/MyHeader.h \
+// RUN: %t/QuotedHeader.h \
+// RUN: -o %t/output.json 2>&1 -verify | 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:  :
+// CHECK-NEXT: #import 
+// CHECK-NEXT: #import 
+// CHECK-NEXT: #import "QuotedHeader.h"
+
+//--- headermap.hmap.json.in
+{
+  "mappings" :
+{
+ "MyFramework/MyHeader.h" : "SRCROOT/MyHeader.h"
+}
+}
+
+//--- MyFramework.h
+// Umbrella for MyFramework
+#import 
+// expected-no-diagnostics
+
+//--- MyHeader.h
+#import 
+int MyInt;
+// expected-no-diagnostics
+
+//--- QuotedHeader.h
+char MyChar;
+// expected-no-diagnostics
+
+//--- Frameworks/OtherFramework.framework/Headers/OtherHeader.h
+int OtherInt;
+// expected-no-diagnostics
+
+//--- reference.output.json.in
+{
+  "metadata": {
+"formatVersion": {
+  "major": 0,
+  "minor": 5,
+  "patch": 3
+},
+"generator": "?"
+  },
+  "module": {
+"name": "MyFramework",
+"platform": {
+  "architecture": "arm64",
+  "operatingSystem": {
+"minimumVersion": {
+  "major": 11,
+  "minor": 0,
+  "patch": 0
+},
+"name": "macosx"
+  },
+  "vendor": "apple"
+}
+  },
+  "relationships": [],
+  "symbols": [
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:I",
+  "spelling": "int"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "MyInt"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "objective-c",
+"precise": "c:@MyInt"
+  },
+  "kind": {
+"displayName": "Global Variable",
+"identifier": "objective-c.var"
+  },
+  "location": {
+"position": {
+   

[PATCH] D123831: [clang][extract-api] Use relative includes

[Zixu Wang via Phabricator via cfe-commits]

zixuw updated this revision to Diff 427061.
zixuw added a comment.
Herald added a subscriber: arphaman.

Update test case to use cc1 instead of driver


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123831/new/

https://reviews.llvm.org/D123831

Files:
  clang/include/clang/ExtractAPI/FrontendActions.h
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/test/ExtractAPI/known_files_only_hmap.c
  clang/test/ExtractAPI/relative_include.m
  clang/test/Index/annotate-comments-enum-constant.c

Index: clang/test/Index/annotate-comments-enum-constant.c
===
--- /dev/null
+++ clang/test/Index/annotate-comments-enum-constant.c
@@ -0,0 +1,17 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: c-index-test -test-load-source all -comments-xml-schema=%S/../../bindings/xml/comment-xml-schema.rng %s > %t/out
+// RUN: FileCheck %s --dump-input always < %t/out
+
+enum {
+  /// Documentation for Foo
+  Foo,
+  Bar, // No documentation for Bar
+  /// Documentation for Baz
+  Baz,
+};
+// CHECK: EnumConstantDecl=Foo:[[@LINE-5]]:3 (Definition) {{.*}} BriefComment=[Documentation for Foo] FullCommentAsHTML=[ Documentation for Foo] FullCommentAsXML=[Fooc:@Ea@Foo@FooFoo Documentation for Foo]
+// CHECK: EnumConstantDecl=Bar:[[@LINE-5]]:3 (Definition)
+// CHECK-NOT: BriefComment=[Documentation for Foo]
+// CHECK: EnumConstantDecl=Baz:[[@LINE-5]]:3 (Definition) {{.*}} BriefComment=[Documentation for Baz] FullCommentAsHTML=[ Documentation for Baz] FullCommentAsXML=[Bazc:@Ea@Foo@BazBaz Documentation for Baz]
+
Index: clang/test/ExtractAPI/relative_include.m
===
--- /dev/null
+++ clang/test/ExtractAPI/relative_include.m
@@ -0,0 +1,193 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// Setup framework root
+// RUN: mkdir -p %t/Frameworks/MyFramework.framework/Headers
+// RUN: cp %t/MyFramework.h %t/Frameworks/MyFramework.framework/Headers/
+// RUN: cp %t/MyHeader.h %t/Frameworks/MyFramework.framework/Headers/
+
+// RUN: sed -e "s@SRCROOT@%{/t:regex_replacement}@g" \
+// RUN: %t/reference.output.json.in >> %t/reference.output.json
+
+// Headermap maps headers to the source root SRCROOT
+// RUN: sed -e "s@SRCROOT@%{/t:regex_replacement}@g" \
+// RUN: %t/headermap.hmap.json.in >> %t/headermap.hmap.json
+// RUN: %hmaptool write %t/headermap.hmap.json %t/headermap.hmap
+
+// Input headers use paths to the framework root/DSTROOT
+// RUN: %clang_cc1 -extract-api -v --product-name=MyFramework \
+// RUN: -triple arm64-apple-macosx \
+// RUN: -iquote%t -I%t/headermap.hmap -F%t/Frameworks \
+// RUN: -x objective-c-header \
+// RUN: %t/Frameworks/MyFramework.framework/Headers/MyFramework.h \
+// RUN: %t/Frameworks/MyFramework.framework/Headers/MyHeader.h \
+// RUN: %t/QuotedHeader.h \
+// RUN: -o %t/output.json 2>&1 -verify | 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:  :
+// CHECK-NEXT: #import 
+// CHECK-NEXT: #import 
+// CHECK-NEXT: #import "QuotedHeader.h"
+
+//--- headermap.hmap.json.in
+{
+  "mappings" :
+{
+ "MyFramework/MyHeader.h" : "SRCROOT/MyHeader.h"
+}
+}
+
+//--- MyFramework.h
+// Umbrella for MyFramework
+#import 
+// expected-no-diagnostics
+
+//--- MyHeader.h
+#import 
+int MyInt;
+// expected-no-diagnostics
+
+//--- QuotedHeader.h
+char MyChar;
+// expected-no-diagnostics
+
+//--- Frameworks/OtherFramework.framework/Headers/OtherHeader.h
+int OtherInt;
+// expected-no-diagnostics
+
+//--- reference.output.json.in
+{
+  "metadata": {
+"formatVersion": {
+  "major": 0,
+  "minor": 5,
+  "patch": 3
+},
+"generator": "?"
+  },
+  "module": {
+"name": "MyFramework",
+"platform": {
+  "architecture": "arm64",
+  "operatingSystem": {
+"minimumVersion": {
+  "major": 11,
+  "minor": 0,
+  "patch": 0
+},
+"name": "macosx"
+  },
+  "vendor": "apple"
+}
+  },
+  "relationships": [],
+  "symbols": [
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:I",
+  "spelling": "int"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "MyInt"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "objective-c",
+"precise": "c:@MyInt"
+  },
+  "kind": {
+"displayName": "Global Variable",
+"identifier": "objective-c.var"
+  },
+  "location": {
+"position": {
+  "character": 5,
+  "line": 2
+},
+"uri": 

[PATCH] D123831: [clang][extract-api] Use relative includes

[Zixu Wang via Phabricator via cfe-commits]

zixuw added a comment.

In D123831#3490684 , @dang wrote:

> Since this is a new test can we use the approach in 
> https://reviews.llvm.org/D124634 to check for diagnostics output.

I used FileCheck to check the input buffer dump in this test, but I'll try to 
move to cc1 and fix the diagnostics check.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123831/new/

https://reviews.llvm.org/D123831

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123831: [clang][extract-api] Use relative includes

[Zixu Wang via Phabricator via cfe-commits]

zixuw updated this revision to Diff 426803.
zixuw added a comment.

- Capture whether the include is quoted in KnownFiles
- Misc: use `getInputBufferName()` instead of string literal


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123831/new/

https://reviews.llvm.org/D123831

Files:
  clang/include/clang/ExtractAPI/FrontendActions.h
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/test/ExtractAPI/known_files_only_hmap.c
  clang/test/ExtractAPI/relative_include.m

Index: clang/test/ExtractAPI/relative_include.m
===
--- clang/test/ExtractAPI/relative_include.m
+++ clang/test/ExtractAPI/relative_include.m
@@ -1,13 +1,27 @@
 // RUN: rm -rf %t
 // RUN: split-file %s %t
-// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
+
+// Setup framework root
+// RUN: mkdir -p %t/Frameworks/MyFramework.framework/Headers
+// RUN: cp %t/MyFramework.h %t/Frameworks/MyFramework.framework/Headers/
+// RUN: cp %t/MyHeader.h %t/Frameworks/MyFramework.framework/Headers/
+
+// RUN: sed -e "s@SRCROOT@%{/t:regex_replacement}@g" \
 // RUN: %t/reference.output.json.in >> %t/reference.output.json
-// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
-// RUN: %t/known_files_only.hmap.json.in >> %t/known_files_only.hmap.json
-// RUN: %hmaptool write %t/known_files_only.hmap.json %t/known_files_only.hmap
-// RUN: %clang -extract-api --product-name=KnownFilesOnlyHmap -target arm64-apple-macosx \
-// RUN: -I%t/known_files_only.hmap -I%t/subdir %t/subdir/subdir1/input.h \
-// RUN: %t/subdir/subdir2/known_file.h -o %t/output.json | FileCheck -allow-empty %s
+
+// Headermap maps headers to the source root SRCROOT
+// RUN: sed -e "s@SRCROOT@%{/t:regex_replacement}@g" \
+// RUN: %t/headermap.hmap.json.in >> %t/headermap.hmap.json
+// RUN: %hmaptool write %t/headermap.hmap.json %t/headermap.hmap
+
+// Input headers use paths to the framework root/DSTROOT
+// RUN: %clang -extract-api -v --product-name=MyFramework -target arm64-apple-macosx \
+// RUN: -iquote%t -I%t/headermap.hmap -F%t/Frameworks \
+// RUN: -x objective-c-header \
+// RUN: %t/Frameworks/MyFramework.framework/Headers/MyFramework.h \
+// RUN: %t/Frameworks/MyFramework.framework/Headers/MyHeader.h \
+// RUN: %t/QuotedHeader.h \
+// RUN: -o %t/output.json 2>&1 | FileCheck -allow-empty %s
 
 // Generator version is not consistent across test runs, normalize it.
 // RUN: sed -e "s@\"generator\": \".*\"@\"generator\": \"?\"@g" \
@@ -16,32 +30,33 @@
 
 // CHECK-NOT: error:
 // CHECK-NOT: warning:
-//--- known_files_only.hmap.json.in
+
+// CHECK:  :
+// CHECK-NEXT: #import 
+// CHECK-NEXT: #import 
+// CHECK-NEXT: #import "QuotedHeader.h"
+
+//--- headermap.hmap.json.in
 {
   "mappings" :
 {
- "subdir2/known_file.h" : "INPUT_DIR/subdir/subdir3/unknown.h"
+ "MyFramework/MyHeader.h" : "SRCROOT/MyHeader.h"
 }
 }
 
-//--- subdir/subdir1/input.h
-int num;
-#include "subdir2/known_file.h"
+//--- MyFramework.h
+// Umbrella for MyFramework
+#import 
 
-//--- subdir/subdir2/known_file.h
-int known_num;
+//--- MyHeader.h
+#import 
+int MyInt;
 
-//--- subdir/subdir3/unknown.h
-// Ensure that these symbols are not emitted in the Symbol Graph.
-#ifndef INPUT4_H
-#define INPUT4_H
+//--- QuotedHeader.h
+char MyChar;
 
-#define HELLO 1
-char not_emitted;
-void foo(int);
-struct Foo { int a; };
-
-#endif
+//--- Frameworks/OtherFramework.framework/Headers/OtherHeader.h
+int OtherInt;
 
 //--- reference.output.json.in
 {
@@ -54,7 +69,7 @@
 "generator": "?"
   },
   "module": {
-"name": "KnownFilesOnlyHmap",
+"name": "MyFramework",
 "platform": {
   "architecture": "arm64",
   "operatingSystem": {
@@ -84,41 +99,41 @@
 },
 {
   "kind": "identifier",
-  "spelling": "num"
+  "spelling": "MyInt"
 }
   ],
   "identifier": {
-"interfaceLanguage": "c",
-"precise": "c:@num"
+"interfaceLanguage": "objective-c",
+"precise": "c:@MyInt"
   },
   "kind": {
 "displayName": "Global Variable",
-"identifier": "c.var"
+"identifier": "objective-c.var"
   },
   "location": {
 "position": {
   "character": 5,
-  "line": 1
+  "line": 2
 },
-"uri": "file://INPUT_DIR/subdir/subdir1/input.h"
+"uri": "file://SRCROOT/MyHeader.h"
   },
   "names": {
 "navigator": [
   {
 "kind": "identifier",
-"spelling": "num"
+"spelling": "MyInt"
   }
 ],
 "subHeading": [
   {
 "kind": "identifier",
-"spelling": "num"
+"spelling": "MyInt"
   }
 ],
-"title": "num"
+"title": "MyInt"
   },
   "pathComponents": [
-"num"
+"MyInt"
   ]
 },
 {
@@ -126,8 +141,8 @@
   "declarationFragments": [
 {
   

[PATCH] D123831: [clang][extract-api] Use relative includes

[Zixu Wang via Phabricator via cfe-commits]

zixuw updated this revision to Diff 426547.
zixuw added a comment.

Convert file path to use slashes for headermap reverse lookup


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123831/new/

https://reviews.llvm.org/D123831

Files:
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/test/ExtractAPI/known_files_only_hmap.c
  clang/test/ExtractAPI/relative_include.m

Index: clang/test/ExtractAPI/relative_include.m
===
--- clang/test/ExtractAPI/relative_include.m
+++ clang/test/ExtractAPI/relative_include.m
@@ -1,13 +1,27 @@
 // RUN: rm -rf %t
 // RUN: split-file %s %t
-// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
+
+// Setup framework root
+// RUN: mkdir -p %t/Frameworks/MyFramework.framework/Headers
+// RUN: cp %t/MyFramework.h %t/Frameworks/MyFramework.framework/Headers/
+// RUN: cp %t/MyHeader.h %t/Frameworks/MyFramework.framework/Headers/
+
+// RUN: sed -e "s@SRCROOT@%{/t:regex_replacement}@g" \
 // RUN: %t/reference.output.json.in >> %t/reference.output.json
-// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
-// RUN: %t/known_files_only.hmap.json.in >> %t/known_files_only.hmap.json
-// RUN: %hmaptool write %t/known_files_only.hmap.json %t/known_files_only.hmap
-// RUN: %clang -extract-api --product-name=KnownFilesOnlyHmap -target arm64-apple-macosx \
-// RUN: -I%t/known_files_only.hmap -I%t/subdir %t/subdir/subdir1/input.h \
-// RUN: %t/subdir/subdir2/known_file.h -o %t/output.json | FileCheck -allow-empty %s
+
+// Headermap maps headers to the source root SRCROOT
+// RUN: sed -e "s@SRCROOT@%{/t:regex_replacement}@g" \
+// RUN: %t/headermap.hmap.json.in >> %t/headermap.hmap.json
+// RUN: %hmaptool write %t/headermap.hmap.json %t/headermap.hmap
+
+// Input headers use paths to the framework root/DSTROOT
+// RUN: %clang -extract-api -v --product-name=MyFramework -target arm64-apple-macosx \
+// RUN: -iquote%t -I%t/headermap.hmap -F%t/Frameworks \
+// RUN: -x objective-c-header \
+// RUN: %t/Frameworks/MyFramework.framework/Headers/MyFramework.h \
+// RUN: %t/Frameworks/MyFramework.framework/Headers/MyHeader.h \
+// RUN: %t/QuotedHeader.h \
+// RUN: -o %t/output.json 2>&1 | FileCheck -allow-empty %s
 
 // Generator version is not consistent across test runs, normalize it.
 // RUN: sed -e "s@\"generator\": \".*\"@\"generator\": \"?\"@g" \
@@ -16,32 +30,33 @@
 
 // CHECK-NOT: error:
 // CHECK-NOT: warning:
-//--- known_files_only.hmap.json.in
+
+// CHECK:  :
+// CHECK-NEXT: #import 
+// CHECK-NEXT: #import 
+// CHECK-NEXT: #import "QuotedHeader.h"
+
+//--- headermap.hmap.json.in
 {
   "mappings" :
 {
- "subdir2/known_file.h" : "INPUT_DIR/subdir/subdir3/unknown.h"
+ "MyFramework/MyHeader.h" : "SRCROOT/MyHeader.h"
 }
 }
 
-//--- subdir/subdir1/input.h
-int num;
-#include "subdir2/known_file.h"
+//--- MyFramework.h
+// Umbrella for MyFramework
+#import 
 
-//--- subdir/subdir2/known_file.h
-int known_num;
+//--- MyHeader.h
+#import 
+int MyInt;
 
-//--- subdir/subdir3/unknown.h
-// Ensure that these symbols are not emitted in the Symbol Graph.
-#ifndef INPUT4_H
-#define INPUT4_H
+//--- QuotedHeader.h
+char MyChar;
 
-#define HELLO 1
-char not_emitted;
-void foo(int);
-struct Foo { int a; };
-
-#endif
+//--- Frameworks/OtherFramework.framework/Headers/OtherHeader.h
+int OtherInt;
 
 //--- reference.output.json.in
 {
@@ -54,7 +69,7 @@
 "generator": "?"
   },
   "module": {
-"name": "KnownFilesOnlyHmap",
+"name": "MyFramework",
 "platform": {
   "architecture": "arm64",
   "operatingSystem": {
@@ -84,41 +99,41 @@
 },
 {
   "kind": "identifier",
-  "spelling": "num"
+  "spelling": "MyInt"
 }
   ],
   "identifier": {
-"interfaceLanguage": "c",
-"precise": "c:@num"
+"interfaceLanguage": "objective-c",
+"precise": "c:@MyInt"
   },
   "kind": {
 "displayName": "Global Variable",
-"identifier": "c.var"
+"identifier": "objective-c.var"
   },
   "location": {
 "position": {
   "character": 5,
-  "line": 1
+  "line": 2
 },
-"uri": "file://INPUT_DIR/subdir/subdir1/input.h"
+"uri": "file://SRCROOT/MyHeader.h"
   },
   "names": {
 "navigator": [
   {
 "kind": "identifier",
-"spelling": "num"
+"spelling": "MyInt"
   }
 ],
 "subHeading": [
   {
 "kind": "identifier",
-"spelling": "num"
+"spelling": "MyInt"
   }
 ],
-"title": "num"
+"title": "MyInt"
   },
   "pathComponents": [
-"num"
+"MyInt"
   ]
 },
 {
@@ -126,8 +141,8 @@
   "declarationFragments": [
 {
   "kind": "typeIdentifier",
-  "preciseIdentifier": "c:I",
-  "spelling": "int"
+  

[PATCH] D123831: [clang][extract-api] Use relative includes

[Zixu Wang via Phabricator via cfe-commits]

zixuw updated this revision to Diff 426540.
zixuw added a comment.

- Delete test `known_files_only_hmap`
- Handle quoted includes
- Attempt to fix Windows fails by converting backslashes before matching the 
framework regex
- Update test `relative_include`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123831/new/

https://reviews.llvm.org/D123831

Files:
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/test/ExtractAPI/known_files_only_hmap.c
  clang/test/ExtractAPI/relative_include.m

Index: clang/test/ExtractAPI/relative_include.m
===
--- clang/test/ExtractAPI/relative_include.m
+++ clang/test/ExtractAPI/relative_include.m
@@ -1,13 +1,27 @@
 // RUN: rm -rf %t
 // RUN: split-file %s %t
-// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
+
+// Setup framework root
+// RUN: mkdir -p %t/Frameworks/MyFramework.framework/Headers
+// RUN: cp %t/MyFramework.h %t/Frameworks/MyFramework.framework/Headers/
+// RUN: cp %t/MyHeader.h %t/Frameworks/MyFramework.framework/Headers/
+
+// RUN: sed -e "s@SRCROOT@%{/t:regex_replacement}@g" \
 // RUN: %t/reference.output.json.in >> %t/reference.output.json
-// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
-// RUN: %t/known_files_only.hmap.json.in >> %t/known_files_only.hmap.json
-// RUN: %hmaptool write %t/known_files_only.hmap.json %t/known_files_only.hmap
-// RUN: %clang -extract-api --product-name=KnownFilesOnlyHmap -target arm64-apple-macosx \
-// RUN: -I%t/known_files_only.hmap -I%t/subdir %t/subdir/subdir1/input.h \
-// RUN: %t/subdir/subdir2/known_file.h -o %t/output.json | FileCheck -allow-empty %s
+
+// Headermap maps headers to the source root SRCROOT
+// RUN: sed -e "s@SRCROOT@%{/t:regex_replacement}@g" \
+// RUN: %t/headermap.hmap.json.in >> %t/headermap.hmap.json
+// RUN: %hmaptool write %t/headermap.hmap.json %t/headermap.hmap
+
+// Input headers use paths to the framework root/DSTROOT
+// RUN: %clang -extract-api -v --product-name=MyFramework -target arm64-apple-macosx \
+// RUN: -iquote%t -I%t/headermap.hmap -F%t/Frameworks \
+// RUN: -x objective-c-header \
+// RUN: %t/Frameworks/MyFramework.framework/Headers/MyFramework.h \
+// RUN: %t/Frameworks/MyFramework.framework/Headers/MyHeader.h \
+// RUN: %t/QuotedHeader.h \
+// RUN: -o %t/output.json 2>&1 | FileCheck -allow-empty %s
 
 // Generator version is not consistent across test runs, normalize it.
 // RUN: sed -e "s@\"generator\": \".*\"@\"generator\": \"?\"@g" \
@@ -16,32 +30,33 @@
 
 // CHECK-NOT: error:
 // CHECK-NOT: warning:
-//--- known_files_only.hmap.json.in
+
+// CHECK:  :
+// CHECK-NEXT: #import 
+// CHECK-NEXT: #import 
+// CHECK-NEXT: #import "QuotedHeader.h"
+
+//--- headermap.hmap.json.in
 {
   "mappings" :
 {
- "subdir2/known_file.h" : "INPUT_DIR/subdir/subdir3/unknown.h"
+ "MyFramework/MyHeader.h" : "SRCROOT/MyHeader.h"
 }
 }
 
-//--- subdir/subdir1/input.h
-int num;
-#include "subdir2/known_file.h"
+//--- MyFramework.h
+// Umbrella for MyFramework
+#import 
 
-//--- subdir/subdir2/known_file.h
-int known_num;
+//--- MyHeader.h
+#import 
+int MyInt;
 
-//--- subdir/subdir3/unknown.h
-// Ensure that these symbols are not emitted in the Symbol Graph.
-#ifndef INPUT4_H
-#define INPUT4_H
+//--- QuotedHeader.h
+char MyChar;
 
-#define HELLO 1
-char not_emitted;
-void foo(int);
-struct Foo { int a; };
-
-#endif
+//--- Frameworks/OtherFramework.framework/Headers/OtherHeader.h
+int OtherInt;
 
 //--- reference.output.json.in
 {
@@ -54,7 +69,7 @@
 "generator": "?"
   },
   "module": {
-"name": "KnownFilesOnlyHmap",
+"name": "MyFramework",
 "platform": {
   "architecture": "arm64",
   "operatingSystem": {
@@ -84,41 +99,41 @@
 },
 {
   "kind": "identifier",
-  "spelling": "num"
+  "spelling": "MyInt"
 }
   ],
   "identifier": {
-"interfaceLanguage": "c",
-"precise": "c:@num"
+"interfaceLanguage": "objective-c",
+"precise": "c:@MyInt"
   },
   "kind": {
 "displayName": "Global Variable",
-"identifier": "c.var"
+"identifier": "objective-c.var"
   },
   "location": {
 "position": {
   "character": 5,
-  "line": 1
+  "line": 2
 },
-"uri": "file://INPUT_DIR/subdir/subdir1/input.h"
+"uri": "file://SRCROOT/MyHeader.h"
   },
   "names": {
 "navigator": [
   {
 "kind": "identifier",
-"spelling": "num"
+"spelling": "MyInt"
   }
 ],
 "subHeading": [
   {
 "kind": "identifier",
-"spelling": "num"
+"spelling": "MyInt"
   }
 ],
-"title": "num"
+"title": "MyInt"
   },
   "pathComponents": [
-"num"
+"MyInt"
   ]
 },
 {
@@ -126,8 +141,8 @@
   "declarationFragments": [

[PATCH] D124638: [clang] Track how headers get included generally during lookup time

[Zixu Wang via Phabricator via cfe-commits]

zixuw accepted this revision.
zixuw added a comment.
This revision is now accepted and ready to land.

LGTM!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124638/new/

https://reviews.llvm.org/D124638

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123831: [POC][WIP] Use relative include in extract-api

[Zixu Wang via Phabricator via cfe-commits]

zixuw updated this revision to Diff 425083.
zixuw added a comment.

- Rewrite commit message for preparation
- Remove shortening paths based on the current working directory: it does not 
work with angled includes, and unnecessary for our use
- XFAIL test known_files_only_hmap.c, as it is not a valid setup with a 
questionable headermap. Need to determine how to fix that test case or just 
discard it, as the new relative_include.m test also checks that symbols from 
external files are dropped


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123831/new/

https://reviews.llvm.org/D123831

Files:
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/test/ExtractAPI/known_files_only_hmap.c
  clang/test/ExtractAPI/relative_include.m

Index: clang/test/ExtractAPI/relative_include.m
===
--- /dev/null
+++ clang/test/ExtractAPI/relative_include.m
@@ -0,0 +1,130 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// Setup framework root
+// RUN: mkdir -p %t/Frameworks/MyFramework.framework/Headers
+// RUN: cp %t/MyFramework.h %t/Frameworks/MyFramework.framework/Headers/
+// RUN: cp %t/MyHeader.h %t/Frameworks/MyFramework.framework/Headers/
+
+// RUN: sed -e "s@SRCROOT@%{/t:regex_replacement}@g" \
+// RUN: %t/reference.output.json.in >> %t/reference.output.json
+
+// Headermap maps headers to the source root SRCROOT
+// RUN: sed -e "s@SRCROOT@%{/t:regex_replacement}@g" \
+// RUN: %t/headermap.hmap.json.in >> %t/headermap.hmap.json
+// RUN: %hmaptool write %t/headermap.hmap.json %t/headermap.hmap
+
+// Input headers use paths to the framework root/DSTROOT
+// RUN: %clang -extract-api --product-name=MyFramework -target arm64-apple-macosx \
+// RUN: -I%t/headermap.hmap -F%t/Frameworks \
+// RUN: %t/Frameworks/MyFramework.framework/Headers/MyFramework.h \
+// RUN: %t/Frameworks/MyFramework.framework/Headers/MyHeader.h \
+// RUN: -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:
+
+//--- headermap.hmap.json.in
+{
+  "mappings" :
+{
+ "MyFramework/MyHeader.h" : "SRCROOT/MyHeader.h"
+}
+}
+
+//--- MyFramework.h
+// Umbrella for MyFramework
+#import 
+
+//--- MyHeader.h
+#import 
+int MyInt;
+
+//--- Frameworks/OtherFramework.framework/Headers/OtherHeader.h
+int OtherInt;
+
+//--- reference.output.json.in
+{
+  "metadata": {
+"formatVersion": {
+  "major": 0,
+  "minor": 5,
+  "patch": 3
+},
+"generator": "?"
+  },
+  "module": {
+"name": "MyFramework",
+"platform": {
+  "architecture": "arm64",
+  "operatingSystem": {
+"minimumVersion": {
+  "major": 11,
+  "minor": 0,
+  "patch": 0
+},
+"name": "macosx"
+  },
+  "vendor": "apple"
+}
+  },
+  "relationships": [],
+  "symbols": [
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:I",
+  "spelling": "int"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "MyInt"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "c",
+"precise": "c:@MyInt"
+  },
+  "kind": {
+"displayName": "Global Variable",
+"identifier": "c.var"
+  },
+  "location": {
+"position": {
+  "character": 5,
+  "line": 2
+},
+"uri": "file://SRCROOT/MyHeader.h"
+  },
+  "names": {
+"navigator": [
+  {
+"kind": "identifier",
+"spelling": "MyInt"
+  }
+],
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "MyInt"
+  }
+],
+"title": "MyInt"
+  },
+  "pathComponents": [
+"MyInt"
+  ]
+}
+  ]
+}
Index: clang/test/ExtractAPI/known_files_only_hmap.c
===
--- clang/test/ExtractAPI/known_files_only_hmap.c
+++ clang/test/ExtractAPI/known_files_only_hmap.c
@@ -1,3 +1,4 @@
+// XFAIL: *
 // RUN: rm -rf %t
 // RUN: split-file %s %t
 // RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
Index: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
===
--- clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
+++ clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
@@ -38,7 +38,10 @@
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/Support/FileSystem.h"
 #include 

[PATCH] D123831: [POC][WIP] Use relative include in extract-api

[Zixu Wang via Phabricator via cfe-commits]

zixuw added inline comments.



Comment at: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:134
+if (!SpelledFilename.empty())
+  return SpelledFilename.str();
+

zixuw wrote:
> zixuw wrote:
> > One problem I can see in this right now is that there might be multiple 
> > headermaps that together construct a chain of mapping, for example
> > ```
> > // A.hmap
> > Header.h -> Product/Header.h
> > 
> > // B.hmap
> > Product/Header.h -> /Absolute/path/to/Header.h
> > ```
> > Then this iteration would conclude on using `Product/Header.h` and missed 
> > the first mapping (if the command line goes `clang -extract-api -I A.hmap 
> > -I B.hmap ...`).
> > 
> > It's also possible that a headermap and a search path together resolve the 
> > header. For example:
> > ```
> > //A.hmap
> > Header.h -> Product/Header.h
> > 
> > // Invocation:
> > clang -extract-api /Some/Path/to/Product/Header.h -I A.hmap -I 
> > /Some/path/to/
> > ```
> > 
> > I think we need to keep records and iterate backwards on the search paths 
> > again to get the full reverse lookup
> Or, iterate once in reverse and find the last match? 樂 
Another thing just came to me: with multiple headermaps chaining remaps, which 
is the "correct" way to include a header?
```
// A.hmap
Header.h -> Product/Header.h

// B.hmap
Product/Header.h -> /Absolute/path/to/Header.h
```
In the context of Darwin frameworks, we'd use `` so we 
don't want to follow the chain all the way backwards.
But without any context or build system rationales of why multiple headermaps 
with remap chains are generated in the first place, I'm feeling that we don't 
nearly have enough information for this if we're only talking about headermap 
as it is and refraining from adopting heuristics.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123831/new/

https://reviews.llvm.org/D123831

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123831: [POC][WIP] Use relative include in extract-api

[Zixu Wang via Phabricator via cfe-commits]

zixuw added inline comments.



Comment at: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:134
+if (!SpelledFilename.empty())
+  return SpelledFilename.str();
+

zixuw wrote:
> One problem I can see in this right now is that there might be multiple 
> headermaps that together construct a chain of mapping, for example
> ```
> // A.hmap
> Header.h -> Product/Header.h
> 
> // B.hmap
> Product/Header.h -> /Absolute/path/to/Header.h
> ```
> Then this iteration would conclude on using `Product/Header.h` and missed the 
> first mapping (if the command line goes `clang -extract-api -I A.hmap -I 
> B.hmap ...`).
> 
> It's also possible that a headermap and a search path together resolve the 
> header. For example:
> ```
> //A.hmap
> Header.h -> Product/Header.h
> 
> // Invocation:
> clang -extract-api /Some/Path/to/Product/Header.h -I A.hmap -I /Some/path/to/
> ```
> 
> I think we need to keep records and iterate backwards on the search paths 
> again to get the full reverse lookup
Or, iterate once in reverse and find the last match? 樂 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123831/new/

https://reviews.llvm.org/D123831

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123831: [POC][WIP] Use relative include in extract-api

[Zixu Wang via Phabricator via cfe-commits]

zixuw added inline comments.



Comment at: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:134
+if (!SpelledFilename.empty())
+  return SpelledFilename.str();
+

One problem I can see in this right now is that there might be multiple 
headermaps that together construct a chain of mapping, for example
```
// A.hmap
Header.h -> Product/Header.h

// B.hmap
Product/Header.h -> /Absolute/path/to/Header.h
```
Then this iteration would conclude on using `Product/Header.h` and missed the 
first mapping (if the command line goes `clang -extract-api -I A.hmap -I B.hmap 
...`).

It's also possible that a headermap and a search path together resolve the 
header. For example:
```
//A.hmap
Header.h -> Product/Header.h

// Invocation:
clang -extract-api /Some/Path/to/Product/Header.h -I A.hmap -I /Some/path/to/
```

I think we need to keep records and iterate backwards on the search paths again 
to get the full reverse lookup


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123831/new/

https://reviews.llvm.org/D123831

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123831: [POC][WIP] Use relative include in extract-api

[Zixu Wang via Phabricator via cfe-commits]

zixuw updated this revision to Diff 425017.
zixuw added a comment.

- Create FileManager in PrepareToExecuteAction
- Use FileManager to load headermaps and reverse lookup mappings
- Use FileSystem to correctly get working directory and make absolute paths


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123831/new/

https://reviews.llvm.org/D123831

Files:
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/test/ExtractAPI/relative_include.m

Index: clang/test/ExtractAPI/relative_include.m
===
--- /dev/null
+++ clang/test/ExtractAPI/relative_include.m
@@ -0,0 +1,130 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// Setup framework root
+// RUN: mkdir -p %t/Frameworks/MyFramework.framework/Headers
+// RUN: cp %t/MyFramework.h %t/Frameworks/MyFramework.framework/Headers/
+// RUN: cp %t/MyHeader.h %t/Frameworks/MyFramework.framework/Headers/
+
+// RUN: sed -e "s@SRCROOT@%{/t:regex_replacement}@g" \
+// RUN: %t/reference.output.json.in >> %t/reference.output.json
+
+// Headermap maps headers to the source root SRCROOT
+// RUN: sed -e "s@SRCROOT@%{/t:regex_replacement}@g" \
+// RUN: %t/headermap.hmap.json.in >> %t/headermap.hmap.json
+// RUN: %hmaptool write %t/headermap.hmap.json %t/headermap.hmap
+
+// Input headers use paths to the framework root/DSTROOT
+// RUN: %clang -extract-api --product-name=MyFramework -target arm64-apple-macosx \
+// RUN: -I%t/headermap.hmap -F%t/Frameworks \
+// RUN: %t/Frameworks/MyFramework.framework/Headers/MyFramework.h \
+// RUN: %t/Frameworks/MyFramework.framework/Headers/MyHeader.h \
+// RUN: -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:
+
+//--- headermap.hmap.json.in
+{
+  "mappings" :
+{
+ "MyFramework/MyHeader.h" : "SRCROOT/MyHeader.h"
+}
+}
+
+//--- MyFramework.h
+// Umbrella for MyFramework
+#import 
+
+//--- MyHeader.h
+#import 
+int MyInt;
+
+//--- Frameworks/OtherFramework.framework/Headers/OtherHeader.h
+int OtherInt;
+
+//--- reference.output.json.in
+{
+  "metadata": {
+"formatVersion": {
+  "major": 0,
+  "minor": 5,
+  "patch": 3
+},
+"generator": "?"
+  },
+  "module": {
+"name": "MyFramework",
+"platform": {
+  "architecture": "arm64",
+  "operatingSystem": {
+"minimumVersion": {
+  "major": 11,
+  "minor": 0,
+  "patch": 0
+},
+"name": "macosx"
+  },
+  "vendor": "apple"
+}
+  },
+  "relationships": [],
+  "symbols": [
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:I",
+  "spelling": "int"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "MyInt"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "c",
+"precise": "c:@MyInt"
+  },
+  "kind": {
+"displayName": "Global Variable",
+"identifier": "c.var"
+  },
+  "location": {
+"position": {
+  "character": 5,
+  "line": 2
+},
+"uri": "file://SRCROOT/MyHeader.h"
+  },
+  "names": {
+"navigator": [
+  {
+"kind": "identifier",
+"spelling": "MyInt"
+  }
+],
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "MyInt"
+  }
+],
+"title": "MyInt"
+  },
+  "pathComponents": [
+"MyInt"
+  ]
+}
+  ]
+}
Index: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
===
--- clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
+++ clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
@@ -38,7 +38,10 @@
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Regex.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 #include 
@@ -55,10 +58,128 @@
   return {};
 }
 
+Optional getRelativeIncludeName(const CompilerInstance ,
+ StringRef File) {
+  assert(CI.hasFileManager() &&
+ "CompilerInstance does not have a FileNamager!");
+
+  using namespace llvm::sys;
+  // Matches framework include patterns
+  const llvm::Regex Rule("/(.+)\\.framework/(.+)?Headers/(.+)");
+
+  const auto  = CI.getVirtualFileSystem();
+
+  SmallString<128> FilePath(File.begin(), File.end());
+  

[PATCH] D123831: [POC][WIP] Use relative include in extract-api

[Zixu Wang via Phabricator via cfe-commits]

zixuw added a comment.

The problem is that we are trying to shorten the input file paths in 
`PrepareToExecuteAction`, where the `CompilerInstance` is still primal and 
doesn't even have a `FileManager` that we could use. That makes it hard (if 
possible at all) to reverse lookup headermaps and use the spelled names if we 
failed to find a search path prefix.
The impact is that we won't be able to get an angled include name for cases 
where an input doesn't reside in any of the normal search paths, but a 
headermap entry maps some name to that path. Or we might get the wrong include 
name where there are both headermap entries and search paths that match the 
input path and the ordering matters.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123831/new/

https://reviews.llvm.org/D123831

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123831: [POC][WIP] Use relative include in extract-api

[Zixu Wang via Phabricator via cfe-commits]

zixuw updated this revision to Diff 424959.
zixuw added a comment.

- Use first match in `getRelativeIncludeName`
- Try to get Preprocessor and reverse lookup headermaps in 
`getRelativeIncludeName`
- Use `getRelativeIncludeName` to check for known files in `LocationFileChecker`
- Misc fixes & adjustments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123831/new/

https://reviews.llvm.org/D123831

Files:
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/test/ExtractAPI/relative_include.m

Index: clang/test/ExtractAPI/relative_include.m
===
--- /dev/null
+++ clang/test/ExtractAPI/relative_include.m
@@ -0,0 +1,130 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// Setup framework root
+// RUN: mkdir -p %t/Frameworks/MyFramework.framework/Headers
+// RUN: cp %t/MyFramework.h %t/Frameworks/MyFramework.framework/Headers/
+// RUN: cp %t/MyHeader.h %t/Frameworks/MyFramework.framework/Headers/
+
+// RUN: sed -e "s@SRCROOT@%{/t:regex_replacement}@g" \
+// RUN: %t/reference.output.json.in >> %t/reference.output.json
+
+// Headermap maps headers to the source root SRCROOT
+// RUN: sed -e "s@SRCROOT@%{/t:regex_replacement}@g" \
+// RUN: %t/headermap.hmap.json.in >> %t/headermap.hmap.json
+// RUN: %hmaptool write %t/headermap.hmap.json %t/headermap.hmap
+
+// Input headers use paths to the framework root/DSTROOT
+// RUN: %clang -extract-api --product-name=MyFramework -target arm64-apple-macosx \
+// RUN: -I%t/headermap.hmap -F%t/Frameworks \
+// RUN: %t/Frameworks/MyFramework.framework/Headers/MyFramework.h \
+// RUN: %t/Frameworks/MyFramework.framework/Headers/MyHeader.h \
+// RUN: -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:
+
+//--- headermap.hmap.json.in
+{
+  "mappings" :
+{
+ "MyFramework/MyHeader.h" : "SRCROOT/MyHeader.h"
+}
+}
+
+//--- MyFramework.h
+// Umbrella for MyFramework
+#import 
+
+//--- MyHeader.h
+#import 
+int MyInt;
+
+//--- Frameworks/OtherFramework.framework/Headers/OtherHeader.h
+int OtherInt;
+
+//--- reference.output.json.in
+{
+  "metadata": {
+"formatVersion": {
+  "major": 0,
+  "minor": 5,
+  "patch": 3
+},
+"generator": "?"
+  },
+  "module": {
+"name": "MyFramework",
+"platform": {
+  "architecture": "arm64",
+  "operatingSystem": {
+"minimumVersion": {
+  "major": 11,
+  "minor": 0,
+  "patch": 0
+},
+"name": "macosx"
+  },
+  "vendor": "apple"
+}
+  },
+  "relationships": [],
+  "symbols": [
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:I",
+  "spelling": "int"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "MyInt"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "c",
+"precise": "c:@MyInt"
+  },
+  "kind": {
+"displayName": "Global Variable",
+"identifier": "c.var"
+  },
+  "location": {
+"position": {
+  "character": 5,
+  "line": 2
+},
+"uri": "file://SRCROOT/MyHeader.h"
+  },
+  "names": {
+"navigator": [
+  {
+"kind": "identifier",
+"spelling": "MyInt"
+  }
+],
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "MyInt"
+  }
+],
+"title": "MyInt"
+  },
+  "pathComponents": [
+"MyInt"
+  ]
+}
+  ]
+}
Index: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
===
--- clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
+++ clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
@@ -38,7 +38,10 @@
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Regex.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 #include 
@@ -55,10 +58,128 @@
   return {};
 }
 
+Optional getRelativeIncludeName(const CompilerInstance ,
+ StringRef File) {
+  using namespace llvm::sys;
+  // Matches framework include patterns
+  const llvm::Regex Rule("/(.+)\\.framework/(.+)?Headers/(.+)");
+
+  // FIXME: WorkingDir might not be set at this point.
+  StringRef WorkingDir = CI.getFileSystemOpts().WorkingDir;
+
+  SmallString<128> 

[PATCH] D123831: [POC][WIP] Use relative include in extract-api

[Zixu Wang via Phabricator via cfe-commits]

zixuw updated this revision to Diff 423011.
zixuw added a comment.

Add test case to demonstrate the framework case.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123831/new/

https://reviews.llvm.org/D123831

Files:
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/test/ExtractAPI/relative_include.m

Index: clang/test/ExtractAPI/relative_include.m
===
--- /dev/null
+++ clang/test/ExtractAPI/relative_include.m
@@ -0,0 +1,130 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// Setup framework root
+// RUN: mkdir -p %t/Frameworks/MyFramework.framework/Headers
+// RUN: cp %t/MyFramework.h %t/Frameworks/MyFramework.framework/Headers/
+// RUN: cp %t/MyHeader.h %t/Frameworks/MyFramework.framework/Headers/
+
+// RUN: sed -e "s@SRCROOT@%{/t:regex_replacement}@g" \
+// RUN: %t/reference.output.json.in >> %t/reference.output.json
+
+// Headermap maps headers to the source root SRCROOT
+// RUN: sed -e "s@SRCROOT@%{/t:regex_replacement}@g" \
+// RUN: %t/headermap.hmap.json.in >> %t/headermap.hmap.json
+// RUN: %hmaptool write %t/headermap.hmap.json %t/headermap.hmap
+
+// Input headers use paths to the framework root/DSTROOT
+// RUN: %clang -extract-api --product-name=MyFramework -target arm64-apple-macosx \
+// RUN: -I%t/headermap.hmap -F%t/Frameworks \
+// RUN: %t/Frameworks/MyFramework.framework/Headers/MyFramework.h \
+// RUN: %t/Frameworks/MyFramework.framework/Headers/MyHeader.h \
+// RUN: -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:
+
+//--- headermap.hmap.json.in
+{
+  "mappings" :
+{
+ "MyFramework/MyHeader.h" : "SRCROOT/MyHeader.h"
+}
+}
+
+//--- MyFramework.h
+// Umbrella for MyFramework
+#import 
+
+//--- MyHeader.h
+#import 
+int MyInt;
+
+//--- Frameworks/OtherFramework.framework/Headers/OtherHeader.h
+int OtherInt;
+
+//--- reference.output.json.in
+{
+  "metadata": {
+"formatVersion": {
+  "major": 0,
+  "minor": 5,
+  "patch": 3
+},
+"generator": "?"
+  },
+  "module": {
+"name": "MyFramework",
+"platform": {
+  "architecture": "arm64",
+  "operatingSystem": {
+"minimumVersion": {
+  "major": 11,
+  "minor": 0,
+  "patch": 0
+},
+"name": "macosx"
+  },
+  "vendor": "apple"
+}
+  },
+  "relationships": [],
+  "symbols": [
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:I",
+  "spelling": "int"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "MyInt"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "c",
+"precise": "c:@MyInt"
+  },
+  "kind": {
+"displayName": "Global Variable",
+"identifier": "c.var"
+  },
+  "location": {
+"position": {
+  "character": 5,
+  "line": 2
+},
+"uri": "file://SRCROOT/MyHeader.h"
+  },
+  "names": {
+"navigator": [
+  {
+"kind": "identifier",
+"spelling": "MyInt"
+  }
+],
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "MyInt"
+  }
+],
+"title": "MyInt"
+  },
+  "pathComponents": [
+"MyInt"
+  ]
+}
+  ]
+}
Index: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
===
--- clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
+++ clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
@@ -38,7 +38,10 @@
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Regex.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 #include 
@@ -55,10 +58,113 @@
   return {};
 }
 
+Optional getRelativeIncludeName(const CompilerInstance ,
+ StringRef File) {
+  using namespace llvm::sys;
+  // Matches framework include patterns
+  const llvm::Regex Rule("/(.+)\\.framework/(.+)?Headers/(.+)");
+  StringRef WorkingDir = CI.getFileSystemOpts().WorkingDir;
+
+  SmallString<128> FilePath(File.begin(), File.end());
+  if (!WorkingDir.empty() && !path::is_absolute(FilePath))
+fs::make_absolute(WorkingDir, FilePath);
+  path::remove_dots(FilePath, true);
+  File = FilePath;
+
+  // FIXME: The following is adapted from
+  // 

[PATCH] D123831: [POC][WIP] Use relative include in extract-api

[Zixu Wang via Phabricator via cfe-commits]

zixuw added inline comments.



Comment at: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:87
+Dir = DirPath;
+for (auto NI = path::begin(File), NE = path::end(File),
+  DI = path::begin(Dir), DE = path::end(Dir);

I still need to dig into this logic and see what kinds of special cases it's 
trying to handle, and do we really need these instead of simply 
`File.startswith(Dir)`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123831/new/

https://reviews.llvm.org/D123831

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123831: [POC][WIP] Use relative include in extract-api

[Zixu Wang via Phabricator via cfe-commits]

zixuw created this revision.
zixuw added reviewers: ributzka, dang, cishida.
Herald added a project: All.
zixuw requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Proof-of-concept patch. Needs more work.
Transform input headers to relative (angle) includes.
`getRelativeIncludeName` walks `HeaderSearchOptions::UserEntries` to try
to find a relative include stem. Problems:

- do we want the exact logic of `HeaderSearch::suggestPathToFileForDiagnostics`?
- in `ExtractAPIAction::PrepareToExecuteAction` where the includes are 
transformed, we don't have a pre-processor yet, so we don't have access to a 
lot of useful information like `HeaderSearch`, headermaps, DirectoryLookup etc.
- we might not always want to transform an absolute path because the resulting 
relative include name might get remapped in a headermap, for example in test 
`known_files_only_hmap.c`. But how does it work with modules where we need 
relative includes? Is the setup in `known_files_only_hmap` even valid?
- reverse lookup in headermap when deciding if the symbol is coming from a 
knwon file: I think my current implementation is correct and necessary to solve 
the headermap problem, but might have problems due to the above point: an 
unknown file might be mapped to the transformed relative name of a known input.

I think the key problem is that, by passing in an input
`/absolute/path/header.h`, do we mean to process the actual content of
that particular header, or do we mean to use it just as a "lable" and
the thing we really want to process is whatever this lable points to
with the given search paths and headermaps?
In the example of frameworks, I think the latter is true: we want to
use DSTROOT to pass in the headers because DSTROOT has a defined
framework layout so that we can transform the paths to framework-style
relative includes by matching the layout pattern `*.framework/Header/`
and rely on headermaps/VFS/module/search paths to find the intended
headers.
In that sense, the `known_files_only_hmap` test is a really strange
setup with ambiguious intention and context.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123831

Files:
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp

Index: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
===
--- clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
+++ clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
@@ -38,7 +38,10 @@
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Regex.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 #include 
@@ -55,10 +58,113 @@
   return {};
 }
 
+Optional getRelativeIncludeName(const CompilerInstance ,
+ StringRef File) {
+  using namespace llvm::sys;
+  // Matches framework include patterns
+  const llvm::Regex Rule("/(.+)\\.framework/(.+)?Headers/(.+)");
+  StringRef WorkingDir = CI.getFileSystemOpts().WorkingDir;
+
+  SmallString<128> FilePath(File.begin(), File.end());
+  if (!WorkingDir.empty() && !path::is_absolute(FilePath))
+fs::make_absolute(WorkingDir, FilePath);
+  path::remove_dots(FilePath, true);
+  File = FilePath;
+
+  // FIXME: The following is adapted from
+  // HeaderSearch::suggestPathToFileForDiagnostics. Need to figure out if
+  // everything here is actually needed.
+  unsigned BestPrefixLength = 0;
+  // Checks whether `Dir` is a strict path prefix of `File`. If so and that's
+  // the longest prefix we've seen so for it, returns true and updates the
+  // `BestPrefixLength` accordingly.
+  auto CheckDir = [&](llvm::StringRef Dir) -> bool {
+llvm::SmallString<32> DirPath(Dir.begin(), Dir.end());
+if (!WorkingDir.empty() && !path::is_absolute(DirPath))
+  fs::make_absolute(WorkingDir, DirPath);
+path::remove_dots(DirPath, true);
+Dir = DirPath;
+for (auto NI = path::begin(File), NE = path::end(File),
+  DI = path::begin(Dir), DE = path::end(Dir);
+ /*termination condition in loop*/; ++NI, ++DI) {
+  // '.' components in File are ignored.
+  while (NI != NE && *NI == ".")
+++NI;
+  if (NI == NE)
+break;
+
+  // '.' components in Dir are ignored.
+  while (DI != DE && *DI == ".")
+++DI;
+  if (DI == DE) {
+// Dir is a prefix of File, up to '.' components and choice of path
+// separators.
+unsigned PrefixLength = NI - path::begin(File);
+if (PrefixLength > BestPrefixLength) {
+  BestPrefixLength = PrefixLength;
+  return true;
+}
+break;
+  }
+
+  // Consider all path separators equal.
+  if (NI->size() == 1 && DI->size() == 1 &&
+  path::is_separator(NI->front()) && path::is_separator(DI->front()))
+continue;

[PATCH] D123533: [clang][extract-api] Add support for true anonymous enums

[Zixu Wang via Phabricator via cfe-commits]

zixuw accepted this revision.
zixuw added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123533/new/

https://reviews.llvm.org/D123533

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123526: [clang][ExtractAPI][NFC] Fix sed delimiter in test

[Zixu Wang via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe08c435401bc: [clang][ExtractAPI][NFC] Fix sed delimiter in 
test (authored by zixuw).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123526/new/

https://reviews.llvm.org/D123526

Files:
  clang/test/ExtractAPI/enum.c
  clang/test/ExtractAPI/global_record.c
  clang/test/ExtractAPI/global_record_multifile.c
  clang/test/ExtractAPI/known_files_only.c
  clang/test/ExtractAPI/known_files_only_hmap.c
  clang/test/ExtractAPI/language.c
  clang/test/ExtractAPI/macro_undefined.c
  clang/test/ExtractAPI/macros.c
  clang/test/ExtractAPI/objc_category.m
  clang/test/ExtractAPI/objc_interface.m
  clang/test/ExtractAPI/objc_protocol.m
  clang/test/ExtractAPI/struct.c
  clang/test/ExtractAPI/typedef.c
  clang/test/ExtractAPI/typedef_anonymous_record.c
  clang/test/ExtractAPI/typedef_chain.c

Index: clang/test/ExtractAPI/typedef_chain.c
===
--- clang/test/ExtractAPI/typedef_chain.c
+++ clang/test/ExtractAPI/typedef_chain.c
@@ -1,7 +1,7 @@
 // RUN: rm -rf %t
 // RUN: split-file %s %t
-// RUN: sed -e "s@INPUT_DIR@%/t@g" %t/reference.output.json.in >> \
-// RUN: %t/reference.output.json
+// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
+// RUN: %t/reference.output.json.in >> %t/reference.output.json
 // RUN: %clang -extract-api --product-name=TypedefChain -target arm64-apple-macosx \
 // RUN: -x objective-c-header %t/input.h -o %t/output.json | FileCheck -allow-empty %s
 
Index: clang/test/ExtractAPI/typedef_anonymous_record.c
===
--- clang/test/ExtractAPI/typedef_anonymous_record.c
+++ clang/test/ExtractAPI/typedef_anonymous_record.c
@@ -1,7 +1,7 @@
 // RUN: rm -rf %t
 // RUN: split-file %s %t
-// RUN: sed -e "s@INPUT_DIR@%/t@g" %t/reference.output.json.in >> \
-// RUN: %t/reference.output.json
+// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
+// RUN: %t/reference.output.json.in >> %t/reference.output.json
 // RUN: %clang -extract-api --product-name=TypedefChain -target arm64-apple-macosx \
 // RUN: -x objective-c-header %t/input.h -o %t/output.json | FileCheck -allow-empty %s
 
Index: clang/test/ExtractAPI/typedef.c
===
--- clang/test/ExtractAPI/typedef.c
+++ clang/test/ExtractAPI/typedef.c
@@ -1,7 +1,7 @@
 // RUN: rm -rf %t
 // RUN: split-file %s %t
-// RUN: sed -e "s@INPUT_DIR@%/t@g" %t/reference.output.json.in >> \
-// RUN: %t/reference.output.json
+// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
+// RUN: %t/reference.output.json.in >> %t/reference.output.json
 // RUN: %clang -extract-api --product-name=Typedef -target arm64-apple-macosx \
 // RUN: -x objective-c-header %t/input.h -o %t/output.json | FileCheck -allow-empty %s
 
Index: clang/test/ExtractAPI/struct.c
===
--- clang/test/ExtractAPI/struct.c
+++ clang/test/ExtractAPI/struct.c
@@ -1,7 +1,7 @@
 // RUN: rm -rf %t
 // RUN: split-file %s %t
-// RUN: sed -e "s@INPUT_DIR@%/t@g" %t/reference.output.json.in >> \
-// RUN: %t/reference.output.json
+// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
+// RUN: %t/reference.output.json.in >> %t/reference.output.json
 // RUN: %clang -extract-api -target arm64-apple-macosx \
 // RUN: %t/input.h -o %t/output.json | FileCheck -allow-empty %s
 
Index: clang/test/ExtractAPI/objc_protocol.m
===
--- clang/test/ExtractAPI/objc_protocol.m
+++ clang/test/ExtractAPI/objc_protocol.m
@@ -1,7 +1,7 @@
 // RUN: rm -rf %t
 // RUN: split-file %s %t
-// RUN: sed -e "s@INPUT_DIR@%/t@g" %t/reference.output.json.in >> \
-// RUN: %t/reference.output.json
+// 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
 
Index: clang/test/ExtractAPI/objc_interface.m
===
--- clang/test/ExtractAPI/objc_interface.m
+++ clang/test/ExtractAPI/objc_interface.m
@@ -1,7 +1,7 @@
 // RUN: rm -rf %t
 // RUN: split-file %s %t
-// RUN: sed -e "s@INPUT_DIR@%/t@g" %t/reference.output.json.in >> \
-// RUN: %t/reference.output.json
+// 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
 
Index: clang/test/ExtractAPI/objc_category.m
===
--- clang/test/ExtractAPI/objc_category.m
+++ 

[PATCH] D123533: [clang][extract-api] Add support for true anonymous enums

[Zixu Wang via Phabricator via cfe-commits]

zixuw added inline comments.



Comment at: clang/test/ExtractAPI/enum.c:693
+],
+"title": "(anonymous)"
+  },

dang wrote:
> zixuw wrote:
> > So the `Name` of the record is literally `(anonymous)`? I think this might 
> > create problems (well I guess we always had this problem) that multiple 
> > anonymous records of the same kind getting the same `Name`, and `RecordMap` 
> > is indexed with it...
> > We should switch the key of `RecordMap`s to `USR` instead probably.
> Agreed, added this to this patch.
Nice! Thanks Daniel!
Could you please also update the test case to see if it now handles two 
anonymous records correctly?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123533/new/

https://reviews.llvm.org/D123533

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123533: [clang][extract-api] Add support for true anonymous enums

[Zixu Wang via Phabricator via cfe-commits]

zixuw added inline comments.



Comment at: clang/test/ExtractAPI/enum.c:684
+},
+"uri": 
"file:///Users/dgrumberg/VersionControlledDocuments/oss/llvm-project/build/tools/clang/test/ExtractAPI/Output/enum.c.tmp/input.h"
+  },

nit: normalize



Comment at: clang/test/ExtractAPI/enum.c:693
+],
+"title": "(anonymous)"
+  },

So the `Name` of the record is literally `(anonymous)`? I think this might 
create problems (well I guess we always had this problem) that multiple 
anonymous records of the same kind getting the same `Name`, and `RecordMap` is 
indexed with it...
We should switch the key of `RecordMap`s to `USR` instead probably.



Comment at: clang/test/ExtractAPI/enum.c:720
+},
+"uri": 
"file:///Users/dgrumberg/VersionControlledDocuments/oss/llvm-project/build/tools/clang/test/ExtractAPI/Output/enum.c.tmp/input.h"
+  },

nit: normalize


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123533/new/

https://reviews.llvm.org/D123533

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123526: [clang][ExtractAPI][NFC] Fix sed delimiter in test

[Zixu Wang via Phabricator via cfe-commits]

zixuw created this revision.
Herald added a reviewer: dang.
Herald added a project: All.
zixuw requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fix path replacement in sed (properly this time) using lit
regex_replacement.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123526

Files:
  clang/test/ExtractAPI/enum.c
  clang/test/ExtractAPI/global_record.c
  clang/test/ExtractAPI/global_record_multifile.c
  clang/test/ExtractAPI/known_files_only.c
  clang/test/ExtractAPI/known_files_only_hmap.c
  clang/test/ExtractAPI/language.c
  clang/test/ExtractAPI/macro_undefined.c
  clang/test/ExtractAPI/macros.c
  clang/test/ExtractAPI/objc_category.m
  clang/test/ExtractAPI/objc_interface.m
  clang/test/ExtractAPI/objc_protocol.m
  clang/test/ExtractAPI/struct.c
  clang/test/ExtractAPI/typedef.c
  clang/test/ExtractAPI/typedef_anonymous_record.c
  clang/test/ExtractAPI/typedef_chain.c

Index: clang/test/ExtractAPI/typedef_chain.c
===
--- clang/test/ExtractAPI/typedef_chain.c
+++ clang/test/ExtractAPI/typedef_chain.c
@@ -1,7 +1,7 @@
 // RUN: rm -rf %t
 // RUN: split-file %s %t
-// RUN: sed -e "s@INPUT_DIR@%/t@g" %t/reference.output.json.in >> \
-// RUN: %t/reference.output.json
+// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
+// RUN: %t/reference.output.json.in >> %t/reference.output.json
 // RUN: %clang -extract-api --product-name=TypedefChain -target arm64-apple-macosx \
 // RUN: -x objective-c-header %t/input.h -o %t/output.json | FileCheck -allow-empty %s
 
Index: clang/test/ExtractAPI/typedef_anonymous_record.c
===
--- clang/test/ExtractAPI/typedef_anonymous_record.c
+++ clang/test/ExtractAPI/typedef_anonymous_record.c
@@ -1,7 +1,7 @@
 // RUN: rm -rf %t
 // RUN: split-file %s %t
-// RUN: sed -e "s@INPUT_DIR@%/t@g" %t/reference.output.json.in >> \
-// RUN: %t/reference.output.json
+// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
+// RUN: %t/reference.output.json.in >> %t/reference.output.json
 // RUN: %clang -extract-api --product-name=TypedefChain -target arm64-apple-macosx \
 // RUN: -x objective-c-header %t/input.h -o %t/output.json | FileCheck -allow-empty %s
 
Index: clang/test/ExtractAPI/typedef.c
===
--- clang/test/ExtractAPI/typedef.c
+++ clang/test/ExtractAPI/typedef.c
@@ -1,7 +1,7 @@
 // RUN: rm -rf %t
 // RUN: split-file %s %t
-// RUN: sed -e "s@INPUT_DIR@%/t@g" %t/reference.output.json.in >> \
-// RUN: %t/reference.output.json
+// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
+// RUN: %t/reference.output.json.in >> %t/reference.output.json
 // RUN: %clang -extract-api --product-name=Typedef -target arm64-apple-macosx \
 // RUN: -x objective-c-header %t/input.h -o %t/output.json | FileCheck -allow-empty %s
 
Index: clang/test/ExtractAPI/struct.c
===
--- clang/test/ExtractAPI/struct.c
+++ clang/test/ExtractAPI/struct.c
@@ -1,7 +1,7 @@
 // RUN: rm -rf %t
 // RUN: split-file %s %t
-// RUN: sed -e "s@INPUT_DIR@%/t@g" %t/reference.output.json.in >> \
-// RUN: %t/reference.output.json
+// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
+// RUN: %t/reference.output.json.in >> %t/reference.output.json
 // RUN: %clang -extract-api -target arm64-apple-macosx \
 // RUN: %t/input.h -o %t/output.json | FileCheck -allow-empty %s
 
Index: clang/test/ExtractAPI/objc_protocol.m
===
--- clang/test/ExtractAPI/objc_protocol.m
+++ clang/test/ExtractAPI/objc_protocol.m
@@ -1,7 +1,7 @@
 // RUN: rm -rf %t
 // RUN: split-file %s %t
-// RUN: sed -e "s@INPUT_DIR@%/t@g" %t/reference.output.json.in >> \
-// RUN: %t/reference.output.json
+// 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
 
Index: clang/test/ExtractAPI/objc_interface.m
===
--- clang/test/ExtractAPI/objc_interface.m
+++ clang/test/ExtractAPI/objc_interface.m
@@ -1,7 +1,7 @@
 // RUN: rm -rf %t
 // RUN: split-file %s %t
-// RUN: sed -e "s@INPUT_DIR@%/t@g" %t/reference.output.json.in >> \
-// RUN: %t/reference.output.json
+// 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
 
Index: clang/test/ExtractAPI/objc_category.m
===
--- clang/test/ExtractAPI/objc_category.m
+++ 

[PATCH] D123304: [clang][extract-api] Emit "functionSignature" in SGF for ObjC methods.

[Zixu Wang via Phabricator via cfe-commits]

zixuw accepted this revision.
zixuw added a comment.
This revision is now accepted and ready to land.

LGTM!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123304/new/

https://reviews.llvm.org/D123304

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123304: [clang][extract-api] Emit "functionSignature" in SGF for ObjC methods.

[Zixu Wang via Phabricator via cfe-commits]

zixuw added inline comments.



Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:495
 
+if (const auto *Method = dyn_cast(Member.get()))
+  serializeObject(*MemberRecord, "functionSignature",

dang wrote:
> zixuw wrote:
> > I'd prefer not to use `dyn_cast` as `MemberTy` here is a concrete type. So 
> > maybe a type trait `has_function_signature` and a `if constexpr`?
> `if constexpr` is c++17 which we don't support yet? Anyway I added the type 
> trait and implemented this a bit lower down the "stack" (in 
> serializeAPIRecord) as there are multiple record types that have signatures. 
> This is done using tag dispatch based on the trait.
Hmm.. That's a bummer. Didn't realize that when I made the suggestion. I 
actually don't have that strong an opinion regarding the `dyn_cast`.
What do you think? Do you think it's still worth going through all these to get 
the type trait working?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123304/new/

https://reviews.llvm.org/D123304

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123304: [clang][extract-api] Emit "functionSignature" in SGF for ObjC methods.

[Zixu Wang via Phabricator via cfe-commits]

zixuw added inline comments.



Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:495
 
+if (const auto *Method = dyn_cast(Member.get()))
+  serializeObject(*MemberRecord, "functionSignature",

I'd prefer not to use `dyn_cast` as `MemberTy` here is a concrete type. So 
maybe a type trait `has_function_signature` and a `if constexpr`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123304/new/

https://reviews.llvm.org/D123304

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123261: [clang][ExtractAPI] Fix declaration fragments for ObjC methods

[Zixu Wang via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4048aad85a84: [clang][ExtractAPI] Fix declaration fragments 
for ObjC methods (authored by zixuw).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123261/new/

https://reviews.llvm.org/D123261

Files:
  clang/lib/ExtractAPI/DeclarationFragments.cpp
  clang/test/ExtractAPI/objc_category.m
  clang/test/ExtractAPI/objc_interface.m


Index: clang/test/ExtractAPI/objc_interface.m
===
--- clang/test/ExtractAPI/objc_interface.m
+++ clang/test/ExtractAPI/objc_interface.m
@@ -146,11 +146,7 @@
 },
 {
   "kind": "identifier",
-  "spelling": "getWithProperty"
-},
-{
-  "kind": "text",
-  "spelling": ":"
+  "spelling": "getWithProperty:"
 },
 {
   "kind": "text",
@@ -168,6 +164,10 @@
 {
   "kind": "internalParam",
   "spelling": "Property"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -403,6 +403,10 @@
 {
   "kind": "identifier",
   "spelling": "getIvar"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
Index: clang/test/ExtractAPI/objc_category.m
===
--- clang/test/ExtractAPI/objc_category.m
+++ clang/test/ExtractAPI/objc_category.m
@@ -136,6 +136,10 @@
 {
   "kind": "identifier",
   "spelling": "InstanceMethod"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -190,6 +194,10 @@
 {
   "kind": "identifier",
   "spelling": "ClassMethod"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
Index: clang/lib/ExtractAPI/DeclarationFragments.cpp
===
--- clang/lib/ExtractAPI/DeclarationFragments.cpp
+++ clang/lib/ExtractAPI/DeclarationFragments.cpp
@@ -593,20 +593,21 @@
 
   // For Objective-C methods that take arguments, build the selector slots.
   for (unsigned i = 0, end = Method->param_size(); i != end; ++i) {
-Fragments.appendSpace()
-.append(Selector.getNameForSlot(i),
-// The first slot is the name of the method, record as an
-// identifier, otherwise as exteranl parameters.
-i == 0 ? DeclarationFragments::FragmentKind::Identifier
-   : DeclarationFragments::FragmentKind::ExternalParam)
-.append(":", DeclarationFragments::FragmentKind::Text);
+// Objective-C method selector parts are considered as identifiers instead
+// of "external parameters" as in Swift. This is because Objective-C method
+// symbols are referenced with the entire selector, instead of just the
+// method name in Swift.
+SmallString<32> ParamID(Selector.getNameForSlot(i));
+ParamID.append(":");
+Fragments.appendSpace().append(
+ParamID, DeclarationFragments::FragmentKind::Identifier);
 
 // Build the internal parameter.
 const ParmVarDecl *Param = Method->getParamDecl(i);
 Fragments.append(getFragmentsForParam(Param));
   }
 
-  return Fragments;
+  return Fragments.append(";", DeclarationFragments::FragmentKind::Text);
 }
 
 DeclarationFragments DeclarationFragmentsBuilder::getFragmentsForObjCProperty(


Index: clang/test/ExtractAPI/objc_interface.m
===
--- clang/test/ExtractAPI/objc_interface.m
+++ clang/test/ExtractAPI/objc_interface.m
@@ -146,11 +146,7 @@
 },
 {
   "kind": "identifier",
-  "spelling": "getWithProperty"
-},
-{
-  "kind": "text",
-  "spelling": ":"
+  "spelling": "getWithProperty:"
 },
 {
   "kind": "text",
@@ -168,6 +164,10 @@
 {
   "kind": "internalParam",
   "spelling": "Property"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -403,6 +403,10 @@
 {
   "kind": "identifier",
   "spelling": "getIvar"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
Index: clang/test/ExtractAPI/objc_category.m
===
--- clang/test/ExtractAPI/objc_category.m
+++ clang/test/ExtractAPI/objc_category.m
@@ -136,6 +136,10 @@
 {
   "kind": "identifier",
   "spelling": "InstanceMethod"
+},
+{
+  "kind": "text",

[PATCH] D123295: [clang][extract-api] Use dedicated API to check for macro equality

[Zixu Wang via Phabricator via cfe-commits]

zixuw accepted this revision.
zixuw added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123295/new/

https://reviews.llvm.org/D123295

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123259: [clang][ExtractAPI] Fix appendSpace in DeclarationFragments

[Zixu Wang via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGfe2c77a0065c: [clang][ExtractAPI] Fix appendSpace in 
DeclarationFragments (authored by zixuw).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123259/new/

https://reviews.llvm.org/D123259

Files:
  clang/lib/ExtractAPI/DeclarationFragments.cpp
  clang/test/ExtractAPI/global_record.c
  clang/test/ExtractAPI/global_record_multifile.c
  clang/test/ExtractAPI/macro_undefined.c
  clang/test/ExtractAPI/objc_category.m
  clang/test/ExtractAPI/objc_interface.m

Index: clang/test/ExtractAPI/objc_interface.m
===
--- clang/test/ExtractAPI/objc_interface.m
+++ clang/test/ExtractAPI/objc_interface.m
@@ -142,7 +142,7 @@
 },
 {
   "kind": "text",
-  "spelling": ")"
+  "spelling": ") "
 },
 {
   "kind": "identifier",
@@ -163,7 +163,7 @@
 },
 {
   "kind": "text",
-  "spelling": ")"
+  "spelling": ") "
 },
 {
   "kind": "internalParam",
@@ -244,7 +244,7 @@
 },
 {
   "kind": "text",
-  "spelling": ")"
+  "spelling": ") "
 },
 {
   "kind": "typeIdentifier",
@@ -398,7 +398,7 @@
 },
 {
   "kind": "text",
-  "spelling": ")"
+  "spelling": ") "
 },
 {
   "kind": "identifier",
Index: clang/test/ExtractAPI/objc_category.m
===
--- clang/test/ExtractAPI/objc_category.m
+++ clang/test/ExtractAPI/objc_category.m
@@ -131,7 +131,7 @@
 },
 {
   "kind": "text",
-  "spelling": ")"
+  "spelling": ") "
 },
 {
   "kind": "identifier",
@@ -185,7 +185,7 @@
 },
 {
   "kind": "text",
-  "spelling": ")"
+  "spelling": ") "
 },
 {
   "kind": "identifier",
@@ -266,7 +266,7 @@
 },
 {
   "kind": "text",
-  "spelling": ")"
+  "spelling": ") "
 },
 {
   "kind": "typeIdentifier",
Index: clang/test/ExtractAPI/macro_undefined.c
===
--- clang/test/ExtractAPI/macro_undefined.c
+++ clang/test/ExtractAPI/macro_undefined.c
@@ -142,7 +142,7 @@
 },
 {
   "kind": "text",
-  "spelling": " *"
+  "spelling": " * "
 },
 {
   "kind": "internalParam",
@@ -189,7 +189,7 @@
   },
   {
 "kind": "text",
-"spelling": " *"
+"spelling": " * "
   },
   {
 "kind": "internalParam",
Index: clang/test/ExtractAPI/global_record_multifile.c
===
--- clang/test/ExtractAPI/global_record_multifile.c
+++ clang/test/ExtractAPI/global_record_multifile.c
@@ -177,7 +177,7 @@
 },
 {
   "kind": "text",
-  "spelling": " *"
+  "spelling": " * "
 },
 {
   "kind": "internalParam",
@@ -333,7 +333,7 @@
   },
   {
 "kind": "text",
-"spelling": " *"
+"spelling": " * "
   },
   {
 "kind": "internalParam",
Index: clang/test/ExtractAPI/global_record.c
===
--- clang/test/ExtractAPI/global_record.c
+++ clang/test/ExtractAPI/global_record.c
@@ -175,7 +175,7 @@
 },
 {
   "kind": "text",
-  "spelling": " *"
+  "spelling": " * "
 },
 {
   "kind": "internalParam",
@@ -331,7 +331,7 @@
   },
   {
 "kind": "text",
-"spelling": " *"
+"spelling": " * "
   },
   {
 "kind": "internalParam",
Index: clang/lib/ExtractAPI/DeclarationFragments.cpp
===
--- clang/lib/ExtractAPI/DeclarationFragments.cpp
+++ clang/lib/ExtractAPI/DeclarationFragments.cpp
@@ -21,7 +21,7 @@
 
 DeclarationFragments ::appendSpace() {
   if (!Fragments.empty()) {
-Fragment Last = Fragments.back();
+Fragment  = Fragments.back();
 if (Last.Kind == FragmentKind::Text) {
   // Merge the extra space into the last fragment if the last fragment is
   // also text.
@@ -390,7 +390,7 @@
   if (Param->isObjCMethodParameter())
 Fragments.append("(", DeclarationFragments::FragmentKind::Text)
 .append(std::move(TypeFragments))
-.append(")", DeclarationFragments::FragmentKind::Text);
+

[PATCH] D123261: [clang][ExtractAPI] Fix declaration fragments for ObjC methods

[Zixu Wang via Phabricator via cfe-commits]

zixuw created this revision.
Herald added a reviewer: dang.
Herald added a project: All.
zixuw requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Objective-C methods selector parts should be considered as identifiers.

Depends on D123259 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123261

Files:
  clang/lib/ExtractAPI/DeclarationFragments.cpp
  clang/test/ExtractAPI/objc_category.m
  clang/test/ExtractAPI/objc_interface.m


Index: clang/test/ExtractAPI/objc_interface.m
===
--- clang/test/ExtractAPI/objc_interface.m
+++ clang/test/ExtractAPI/objc_interface.m
@@ -146,11 +146,7 @@
 },
 {
   "kind": "identifier",
-  "spelling": "getWithProperty"
-},
-{
-  "kind": "text",
-  "spelling": ":"
+  "spelling": "getWithProperty:"
 },
 {
   "kind": "text",
@@ -168,6 +164,10 @@
 {
   "kind": "internalParam",
   "spelling": "Property"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -403,6 +403,10 @@
 {
   "kind": "identifier",
   "spelling": "getIvar"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
Index: clang/test/ExtractAPI/objc_category.m
===
--- clang/test/ExtractAPI/objc_category.m
+++ clang/test/ExtractAPI/objc_category.m
@@ -136,6 +136,10 @@
 {
   "kind": "identifier",
   "spelling": "InstanceMethod"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -190,6 +194,10 @@
 {
   "kind": "identifier",
   "spelling": "ClassMethod"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
Index: clang/lib/ExtractAPI/DeclarationFragments.cpp
===
--- clang/lib/ExtractAPI/DeclarationFragments.cpp
+++ clang/lib/ExtractAPI/DeclarationFragments.cpp
@@ -593,20 +593,21 @@
 
   // For Objective-C methods that take arguments, build the selector slots.
   for (unsigned i = 0, end = Method->param_size(); i != end; ++i) {
-Fragments.appendSpace()
-.append(Selector.getNameForSlot(i),
-// The first slot is the name of the method, record as an
-// identifier, otherwise as exteranl parameters.
-i == 0 ? DeclarationFragments::FragmentKind::Identifier
-   : DeclarationFragments::FragmentKind::ExternalParam)
-.append(":", DeclarationFragments::FragmentKind::Text);
+// Objective-C method selector parts are considered as identifiers instead
+// of "external parameters" as in Swift. This is because Objective-C method
+// symbols are referenced with the entire selector, instead of just the
+// method name in Swift.
+SmallString<32> ParamID(Selector.getNameForSlot(i));
+ParamID.append(":");
+Fragments.appendSpace().append(
+ParamID, DeclarationFragments::FragmentKind::Identifier);
 
 // Build the internal parameter.
 const ParmVarDecl *Param = Method->getParamDecl(i);
 Fragments.append(getFragmentsForParam(Param));
   }
 
-  return Fragments;
+  return Fragments.append(";", DeclarationFragments::FragmentKind::Text);
 }
 
 DeclarationFragments DeclarationFragmentsBuilder::getFragmentsForObjCProperty(


Index: clang/test/ExtractAPI/objc_interface.m
===
--- clang/test/ExtractAPI/objc_interface.m
+++ clang/test/ExtractAPI/objc_interface.m
@@ -146,11 +146,7 @@
 },
 {
   "kind": "identifier",
-  "spelling": "getWithProperty"
-},
-{
-  "kind": "text",
-  "spelling": ":"
+  "spelling": "getWithProperty:"
 },
 {
   "kind": "text",
@@ -168,6 +164,10 @@
 {
   "kind": "internalParam",
   "spelling": "Property"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -403,6 +403,10 @@
 {
   "kind": "identifier",
   "spelling": "getIvar"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
Index: clang/test/ExtractAPI/objc_category.m
===
--- clang/test/ExtractAPI/objc_category.m
+++ clang/test/ExtractAPI/objc_category.m
@@ -136,6 +136,10 @@
 {
   "kind": "identifier",
   "spelling": "InstanceMethod"
+},
+{
+  

[PATCH] D123259: [clang][ExtractAPI] Fix appendSpace in DeclarationFragments

[Zixu Wang via Phabricator via cfe-commits]

zixuw created this revision.
Herald added a reviewer: dang.
Herald added a project: All.
zixuw requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

There is a bug in `DeclarationFragments::appendSpace` where the space
character is added to a local copy of the last fragment.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123259

Files:
  clang/lib/ExtractAPI/DeclarationFragments.cpp
  clang/test/ExtractAPI/global_record.c
  clang/test/ExtractAPI/global_record_multifile.c
  clang/test/ExtractAPI/macro_undefined.c
  clang/test/ExtractAPI/objc_category.m
  clang/test/ExtractAPI/objc_interface.m

Index: clang/test/ExtractAPI/objc_interface.m
===
--- clang/test/ExtractAPI/objc_interface.m
+++ clang/test/ExtractAPI/objc_interface.m
@@ -142,7 +142,7 @@
 },
 {
   "kind": "text",
-  "spelling": ")"
+  "spelling": ") "
 },
 {
   "kind": "identifier",
@@ -163,7 +163,7 @@
 },
 {
   "kind": "text",
-  "spelling": ")"
+  "spelling": ") "
 },
 {
   "kind": "internalParam",
@@ -244,7 +244,7 @@
 },
 {
   "kind": "text",
-  "spelling": ")"
+  "spelling": ") "
 },
 {
   "kind": "typeIdentifier",
@@ -398,7 +398,7 @@
 },
 {
   "kind": "text",
-  "spelling": ")"
+  "spelling": ") "
 },
 {
   "kind": "identifier",
Index: clang/test/ExtractAPI/objc_category.m
===
--- clang/test/ExtractAPI/objc_category.m
+++ clang/test/ExtractAPI/objc_category.m
@@ -131,7 +131,7 @@
 },
 {
   "kind": "text",
-  "spelling": ")"
+  "spelling": ") "
 },
 {
   "kind": "identifier",
@@ -185,7 +185,7 @@
 },
 {
   "kind": "text",
-  "spelling": ")"
+  "spelling": ") "
 },
 {
   "kind": "identifier",
@@ -266,7 +266,7 @@
 },
 {
   "kind": "text",
-  "spelling": ")"
+  "spelling": ") "
 },
 {
   "kind": "typeIdentifier",
Index: clang/test/ExtractAPI/macro_undefined.c
===
--- clang/test/ExtractAPI/macro_undefined.c
+++ clang/test/ExtractAPI/macro_undefined.c
@@ -142,7 +142,7 @@
 },
 {
   "kind": "text",
-  "spelling": " *"
+  "spelling": " * "
 },
 {
   "kind": "internalParam",
@@ -189,7 +189,7 @@
   },
   {
 "kind": "text",
-"spelling": " *"
+"spelling": " * "
   },
   {
 "kind": "internalParam",
Index: clang/test/ExtractAPI/global_record_multifile.c
===
--- clang/test/ExtractAPI/global_record_multifile.c
+++ clang/test/ExtractAPI/global_record_multifile.c
@@ -177,7 +177,7 @@
 },
 {
   "kind": "text",
-  "spelling": " *"
+  "spelling": " * "
 },
 {
   "kind": "internalParam",
@@ -333,7 +333,7 @@
   },
   {
 "kind": "text",
-"spelling": " *"
+"spelling": " * "
   },
   {
 "kind": "internalParam",
Index: clang/test/ExtractAPI/global_record.c
===
--- clang/test/ExtractAPI/global_record.c
+++ clang/test/ExtractAPI/global_record.c
@@ -175,7 +175,7 @@
 },
 {
   "kind": "text",
-  "spelling": " *"
+  "spelling": " * "
 },
 {
   "kind": "internalParam",
@@ -331,7 +331,7 @@
   },
   {
 "kind": "text",
-"spelling": " *"
+"spelling": " * "
   },
   {
 "kind": "internalParam",
Index: clang/lib/ExtractAPI/DeclarationFragments.cpp
===
--- clang/lib/ExtractAPI/DeclarationFragments.cpp
+++ clang/lib/ExtractAPI/DeclarationFragments.cpp
@@ -21,7 +21,7 @@
 
 DeclarationFragments ::appendSpace() {
   if (!Fragments.empty()) {
-Fragment Last = Fragments.back();
+Fragment  = Fragments.back();
 if (Last.Kind == FragmentKind::Text) {
   // Merge the extra space into the last fragment if the last fragment is
   // also text.
@@ -390,7 +390,7 @@
   if (Param->isObjCMethodParameter())
 Fragments.append("(", DeclarationFragments::FragmentKind::Text)
 .append(std::move(TypeFragments))
-.append(")", DeclarationFragments::FragmentKind::Text);

[PATCH] D122774: [clang][extract-api] Add Objective-C Category support

[Zixu Wang via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG178aad9b946e: [clang][extract-api] Add Objective-C Category 
support (authored by zixuw).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122774/new/

https://reviews.llvm.org/D122774

Files:
  clang/include/clang/ExtractAPI/API.h
  clang/include/clang/ExtractAPI/DeclarationFragments.h
  clang/include/clang/ExtractAPI/Serialization/SymbolGraphSerializer.h
  clang/lib/ExtractAPI/API.cpp
  clang/lib/ExtractAPI/DeclarationFragments.cpp
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
  clang/test/ExtractAPI/objc_category.m

Index: clang/test/ExtractAPI/objc_category.m
===
--- /dev/null
+++ clang/test/ExtractAPI/objc_category.m
@@ -0,0 +1,311 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s@INPUT_DIR@%/t@g" %t/reference.output.json.in >> \
+// RUN: %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 Protocol;
+
+@interface Interface
+@end
+
+@interface Interface (Category) 
+@property int Property;
+- (void)InstanceMethod;
++ (void)ClassMethod;
+@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)Interface(im)InstanceMethod",
+  "target": "c:objc(cs)Interface"
+},
+{
+  "kind": "memberOf",
+  "source": "c:objc(cs)Interface(cm)ClassMethod",
+  "target": "c:objc(cs)Interface"
+},
+{
+  "kind": "memberOf",
+  "source": "c:objc(cs)Interface(py)Property",
+  "target": "c:objc(cs)Interface"
+},
+{
+  "kind": "conformsTo",
+  "source": "c:objc(cs)Interface",
+  "target": "c:objc(pl)Protocol"
+}
+  ],
+  "symbols": [
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "keyword",
+  "spelling": "@interface"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "Interface"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "objective-c",
+"precise": "c:objc(cs)Interface"
+  },
+  "kind": {
+"displayName": "Class",
+"identifier": "objective-c.class"
+  },
+  "location": {
+"position": {
+  "character": 12,
+  "line": 3
+},
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "Interface"
+  }
+],
+"title": "Interface"
+  },
+  "pathComponents": [
+"Interface"
+  ]
+},
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "text",
+  "spelling": "- ("
+},
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:v",
+  "spelling": "void"
+},
+{
+  "kind": "text",
+  "spelling": ")"
+},
+{
+  "kind": "identifier",
+  "spelling": "InstanceMethod"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "objective-c",
+"precise": "c:objc(cs)Interface(im)InstanceMethod"
+  },
+  "kind": {
+"displayName": "Instance Method",
+"identifier": "objective-c.method"
+  },
+  "location": {
+"position": {
+  "character": 1,
+  "line": 8
+},
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"subHeading": [
+  {
+"kind": "text",
+"spelling": "- "
+  },
+  {
+"kind": "identifier",
+"spelling": "InstanceMethod"
+  }
+],
+"title": "InstanceMethod"
+  },
+  "pathComponents": [
+"Interface",
+

[PATCH] D122774: [clang][extract-api] Add Objective-C Category support

[Zixu Wang via Phabricator via cfe-commits]

zixuw updated this revision to Diff 420964.
zixuw added a comment.

Remove extra commas in test case.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122774/new/

https://reviews.llvm.org/D122774

Files:
  clang/include/clang/ExtractAPI/API.h
  clang/include/clang/ExtractAPI/DeclarationFragments.h
  clang/include/clang/ExtractAPI/Serialization/SymbolGraphSerializer.h
  clang/lib/ExtractAPI/API.cpp
  clang/lib/ExtractAPI/DeclarationFragments.cpp
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
  clang/test/ExtractAPI/objc_category.m

Index: clang/test/ExtractAPI/objc_category.m
===
--- /dev/null
+++ clang/test/ExtractAPI/objc_category.m
@@ -0,0 +1,311 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s@INPUT_DIR@%/t@g" %t/reference.output.json.in >> \
+// RUN: %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 Protocol;
+
+@interface Interface
+@end
+
+@interface Interface (Category) 
+@property int Property;
+- (void)InstanceMethod;
++ (void)ClassMethod;
+@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)Interface(im)InstanceMethod",
+  "target": "c:objc(cs)Interface"
+},
+{
+  "kind": "memberOf",
+  "source": "c:objc(cs)Interface(cm)ClassMethod",
+  "target": "c:objc(cs)Interface"
+},
+{
+  "kind": "memberOf",
+  "source": "c:objc(cs)Interface(py)Property",
+  "target": "c:objc(cs)Interface"
+},
+{
+  "kind": "conformsTo",
+  "source": "c:objc(cs)Interface",
+  "target": "c:objc(pl)Protocol"
+}
+  ],
+  "symbols": [
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "keyword",
+  "spelling": "@interface"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "Interface"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "objective-c",
+"precise": "c:objc(cs)Interface"
+  },
+  "kind": {
+"displayName": "Class",
+"identifier": "objective-c.class"
+  },
+  "location": {
+"position": {
+  "character": 12,
+  "line": 3
+},
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "Interface"
+  }
+],
+"title": "Interface"
+  },
+  "pathComponents": [
+"Interface"
+  ]
+},
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "text",
+  "spelling": "- ("
+},
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:v",
+  "spelling": "void"
+},
+{
+  "kind": "text",
+  "spelling": ")"
+},
+{
+  "kind": "identifier",
+  "spelling": "InstanceMethod"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "objective-c",
+"precise": "c:objc(cs)Interface(im)InstanceMethod"
+  },
+  "kind": {
+"displayName": "Instance Method",
+"identifier": "objective-c.method"
+  },
+  "location": {
+"position": {
+  "character": 1,
+  "line": 8
+},
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"subHeading": [
+  {
+"kind": "text",
+"spelling": "- "
+  },
+  {
+"kind": "identifier",
+"spelling": "InstanceMethod"
+  }
+],
+"title": "InstanceMethod"
+  },
+  "pathComponents": [
+"Interface",
+"InstanceMethod"
+  ]
+},
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "text",
+ 

[PATCH] D122774: [clang][extract-api] Add Objective-C Category support

[Zixu Wang via Phabricator via cfe-commits]

zixuw updated this revision to Diff 420956.
zixuw added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122774/new/

https://reviews.llvm.org/D122774

Files:
  clang/include/clang/ExtractAPI/API.h
  clang/include/clang/ExtractAPI/DeclarationFragments.h
  clang/include/clang/ExtractAPI/Serialization/SymbolGraphSerializer.h
  clang/lib/ExtractAPI/API.cpp
  clang/lib/ExtractAPI/DeclarationFragments.cpp
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
  clang/test/ExtractAPI/objc_category.m

Index: clang/test/ExtractAPI/objc_category.m
===
--- /dev/null
+++ clang/test/ExtractAPI/objc_category.m
@@ -0,0 +1,311 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s@INPUT_DIR@%/t@g" %t/reference.output.json.in >> \
+// RUN: %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 Protocol;
+
+@interface Interface
+@end
+
+@interface Interface (Category) 
+@property int Property;
+- (void)InstanceMethod;
++ (void)ClassMethod;
+@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)Interface(im)InstanceMethod",
+  "target": "c:objc(cs)Interface"
+},
+{
+  "kind": "memberOf",
+  "source": "c:objc(cs)Interface(cm)ClassMethod",
+  "target": "c:objc(cs)Interface"
+},
+{
+  "kind": "memberOf",
+  "source": "c:objc(cs)Interface(py)Property",
+  "target": "c:objc(cs)Interface"
+},
+{
+  "kind": "conformsTo",
+  "source": "c:objc(cs)Interface",
+  "target": "c:objc(pl)Protocol"
+}
+  ],
+  "symbols": [
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "keyword",
+  "spelling": "@interface"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "Interface"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "objective-c",
+"precise": "c:objc(cs)Interface"
+  },
+  "kind": {
+"displayName": "Class",
+"identifier": "objective-c.class"
+  },
+  "location": {
+"position": {
+  "character": 12,
+  "line": 3
+},
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "Interface"
+  }
+],
+"title": "Interface"
+  },
+  "pathComponents": [
+"Interface"
+  ]
+},
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "text",
+  "spelling": "- ("
+},
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:v",
+  "spelling": "void"
+},
+{
+  "kind": "text",
+  "spelling": ")"
+},
+{
+  "kind": "identifier",
+  "spelling": "InstanceMethod"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "objective-c",
+"precise": "c:objc(cs)Interface(im)InstanceMethod"
+  },
+  "kind": {
+"displayName": "Instance Method",
+"identifier": "objective-c.method"
+  },
+  "location": {
+"position": {
+  "character": 1,
+  "line": 8,
+},
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"subHeading": [
+  {
+"kind": "text",
+"spelling": "- "
+  },
+  {
+"kind": "identifier",
+"spelling": "InstanceMethod"
+  }
+],
+"title": "InstanceMethod"
+  },
+  "pathComponents": [
+"Interface",
+"InstanceMethod"
+  ]
+},
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "text",
+  "spelling": "+ ("
+  

[PATCH] D123019: [clang][extract-api] Add support for typedefs

[Zixu Wang via Phabricator via cfe-commits]

zixuw accepted this revision.
zixuw added a comment.
This revision is now accepted and ready to land.

LGTM! 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123019/new/

https://reviews.llvm.org/D123019

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123045: [clang][extract-api] Fix small issues with SymbolGraphSerializer

[Zixu Wang via Phabricator via cfe-commits]

zixuw accepted this revision.
zixuw added a comment.

Unused include of declaration fragments in SymbolGraphSerializer. Otherwise 
LGTM!




Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:17
 #include "clang/ExtractAPI/API.h"
+#include "clang/ExtractAPI/DeclarationFragments.h"
 #include "llvm/Support/JSON.h"

zixuw wrote:
> Not needed
ping.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123045/new/

https://reviews.llvm.org/D123045

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123148: [clang][extract-api] Process only APIs declared in inputs

[Zixu Wang via Phabricator via cfe-commits]

zixuw added inline comments.



Comment at: clang/test/ExtractAPI/known_files_only.c:20
+// Let's make sure we aren't pulling in symbols from complex.h
+#include 
+double complex build_complex(double real, double imaginary);

I would just include another header split from this test file but not passed 
into the command line to avoid pulling in extra dependencies. Makes the test 
self-contained and more consistent, also simpler.



Comment at: clang/test/ExtractAPI/known_files_only.c:166
+"line": 5,
+"uri": 
"file:///Users/dgrumberg/VersionControlledDocuments/oss/llvm-project/build/tools/clang/test/ExtractAPI/Output/known_files_only.c.tmp/input1.h"
+  },

uri not normalized.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123148/new/

https://reviews.llvm.org/D123148

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123019: [clang][extract-api] Add support for typedefs

[Zixu Wang via Phabricator via cfe-commits]

zixuw added inline comments.



Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:611
+
+  (*Typedef)["type"] = Record.UnderlyingType.USR;
+

Curious: where does this come from the format spec? Is this required/correctly 
populated?
I see a `type` property in 
https://github.com/apple/swift-docc-symbolkit/blob/0a45209833f4a151212c1aa38e13cfc03b9462e4/openapi.yaml#L239-L242,
 but couldn't determine if it means the underlying type for a typedef.
cc. @QuietMisdreavus 



Comment at: clang/lib/ExtractAPI/TypedefUnderlyingTypeResolver.cpp:15
+#include "TypedefUnderlyingTypeResolver.h"
+
+#include "clang/Index/USRGeneration.h"

nit: empty line



Comment at: clang/lib/ExtractAPI/TypedefUnderlyingTypeResolver.h:36
+
+  TypedefUnderlyingTypeResolver(ASTContext ) : Context(Context) {}
+

nit: `explicit`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123019/new/

https://reviews.llvm.org/D123019

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123045: [clang][extract-api] Fix small issues with SymbolGraphSerializer

[Zixu Wang via Phabricator via cfe-commits]

zixuw added inline comments.



Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:510
   Symbols.emplace_back(std::move(*Obj));
+  PathComponentContext.pop_back();
 }

zixuw wrote:
> QuietMisdreavus wrote:
> > zixuw wrote:
> > > What's the cost/would it worth it to wrap the `emplace_back`s and 
> > > `pop_back`s of `PathComponentContext` in meaningful APIs like 
> > > `enterPathComponentContext` and `exitPathComponentContext`?
> > > That way the code is more self-explanatory and easier to read. It took me 
> > > some time and mental resources to figure out why the `pop_back` is placed 
> > > here.
> > What's the use of having the `emplace_back` call inside 
> > `serializeAPIRecord` but to pop it outside? It seems like it's easier to 
> > mess up for new kinds of records.
> The reason to `emplace_back` the path component inside `serializeAPIRecord` 
> is that the `pathComponent` field of the `Record` is serialized in there so 
> you want to include the name of the symbol itself in the path component list.
> The `pop_back` is delayed til all other additional serialization for a 
> specific kind of record, for example `serializeEnumRecord` handles all the 
> enum cases after processing the enum record itself using 
> `serializeAPIRecord`. So in order to correctly include the enum name in the 
> path components of all the enum cases, the enum name has to stay in 
> `PathComponentContext` until all members are serialized.
> 
> This is exactly the reason why I wanted a clearer API to make it easier to 
> see.
Hmm now that I thought about this, it seems that it would be easier to 
understand and avoid bugs if we lift 
`PathComponentContext.emplace_back`/`enterPathComponentContext` out of 
`serializeAPIRecord`, because we have access to the record name before that 
anyway.

So we establish a pre-condition of `serializeAPIRecord` that the correct path 
components would be set up in `PathComponentContext` before the call so we 
could still serialize the field inside that method. And in specific 
`serialize*Record` methods we push the record name, and pop out at the end.

This way the push and pop operations would appear in pairs in a single block, 
saving the confusion and mental work of jumping across functions to see how 
`PathComponentContext` is evolving.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123045/new/

https://reviews.llvm.org/D123045

STAMPS
actor(@zixuw) application(Differential) author(@dang) herald(H423) herald(H576) 
herald(H864) herald(H875) herald(H876) monogram(D123045) object-type(DREV) 
phid(PHID-DREV-d5goxxqici5xa3w2bgjy) reviewer(@QuietMisdreavus) 
reviewer(@ributzka) reviewer(@zixuw) revision-repository(rG) 
revision-status(needs-review) subscriber(@cfe-commits) tag(#all) tag(#clang) 
via(web)

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D122774: [clang][extract-api] Add Objective-C Category support

[Zixu Wang via Phabricator via cfe-commits]

zixuw updated this revision to Diff 420333.
zixuw marked an inline comment as done.
zixuw added a comment.

Address review comment:

- Use `llvm_unreachable` for the Objective-C category case in 
`serializeSymbolKind`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122774/new/

https://reviews.llvm.org/D122774

Files:
  clang/include/clang/ExtractAPI/API.h
  clang/include/clang/ExtractAPI/DeclarationFragments.h
  clang/include/clang/ExtractAPI/Serialization/SymbolGraphSerializer.h
  clang/lib/ExtractAPI/API.cpp
  clang/lib/ExtractAPI/DeclarationFragments.cpp
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
  clang/test/ExtractAPI/objc_category.m

STAMPS
actor(@zixuw) application(Differential) author(@zixuw) herald(H423) 
herald(H576) herald(H864) herald(H875) monogram(D122774) object-type(DREV) 
phid(PHID-DREV-o2c75ab3w5lvo3s7frw5) reviewer(@dang) reviewer(@QuietMisdreavus) 
reviewer(@ributzka) revision-repository(rG) revision-status(needs-review) 
subscriber(@cfe-commits) tag(#all) tag(#clang) via(conduit)

Index: clang/test/ExtractAPI/objc_category.m
===
--- /dev/null
+++ clang/test/ExtractAPI/objc_category.m
@@ -0,0 +1,284 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s@INPUT_DIR@%/t@g" %t/reference.output.json.in >> \
+// RUN: %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 Protocol;
+
+@interface Interface
+@end
+
+@interface Interface (Category) 
+@property int Property;
+- (void)InstanceMethod;
++ (void)ClassMethod;
+@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"
+}
+  },
+  "relationhips": [
+{
+  "kind": "memberOf",
+  "source": "c:objc(cs)Interface(im)InstanceMethod",
+  "target": "c:objc(cs)Interface"
+},
+{
+  "kind": "memberOf",
+  "source": "c:objc(cs)Interface(cm)ClassMethod",
+  "target": "c:objc(cs)Interface"
+},
+{
+  "kind": "memberOf",
+  "source": "c:objc(cs)Interface(py)Property",
+  "target": "c:objc(cs)Interface"
+},
+{
+  "kind": "conformsTo",
+  "source": "c:objc(cs)Interface",
+  "target": "c:objc(pl)Protocol"
+}
+  ],
+  "symbols": [
+{
+  "declarationFragments": [
+{
+  "kind": "keyword",
+  "spelling": "@interface"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "Interface"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "objective-c",
+"precise": "c:objc(cs)Interface"
+  },
+  "kind": {
+"displayName": "Class",
+"identifier": "objective-c.class"
+  },
+  "location": {
+"character": 12,
+"line": 3,
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "Interface"
+  }
+],
+"title": "Interface"
+  }
+},
+{
+  "declarationFragments": [
+{
+  "kind": "text",
+  "spelling": "- ("
+},
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:v",
+  "spelling": "void"
+},
+{
+  "kind": "text",
+  "spelling": ")"
+},
+{
+  "kind": "identifier",
+  "spelling": "InstanceMethod"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "objective-c",
+"precise": "c:objc(cs)Interface(im)InstanceMethod"
+  },
+  "kind": {
+"displayName": "Instance Method",
+"identifier": "objective-c.method"
+  },
+  "location": {
+"character": 1,
+"line": 8,
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"subHeading": [
+  {
+"kind": "text",
+"spelling": "- "
+  },
+  {
+"kind": "identifier",
+"spelling": 

[PATCH] D122774: [clang][extract-api] Add Objective-C Category support

[Zixu Wang via Phabricator via cfe-commits]

zixuw marked an inline comment as done.
zixuw added inline comments.



Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:396
 break;
+  case APIRecord::RK_ObjCCategory:
+Kind["identifier"] = AddLangPrefix("category");

dang wrote:
> Since we currently never actually emit a standalone category symbol object, 
> should we not just label this as `llvm_unreachable` with a comment explaining 
> why that is?
Yep, makes sense.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122774/new/

https://reviews.llvm.org/D122774

STAMPS
actor(@zixuw) application(Differential) author(@zixuw) herald(H423) 
herald(H576) herald(H864) herald(H875) monogram(D122774) object-type(DREV) 
phid(PHID-DREV-o2c75ab3w5lvo3s7frw5) reviewer(@dang) reviewer(@QuietMisdreavus) 
reviewer(@ributzka) revision-repository(rG) revision-status(needs-review) 
subscriber(@cfe-commits) tag(#all) tag(#clang) via(web)

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123045: [clang][extract-api] Fix small issues with SymbolGraphSerializer

[Zixu Wang via Phabricator via cfe-commits]

zixuw added inline comments.



Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:510
   Symbols.emplace_back(std::move(*Obj));
+  PathComponentContext.pop_back();
 }

QuietMisdreavus wrote:
> zixuw wrote:
> > What's the cost/would it worth it to wrap the `emplace_back`s and 
> > `pop_back`s of `PathComponentContext` in meaningful APIs like 
> > `enterPathComponentContext` and `exitPathComponentContext`?
> > That way the code is more self-explanatory and easier to read. It took me 
> > some time and mental resources to figure out why the `pop_back` is placed 
> > here.
> What's the use of having the `emplace_back` call inside `serializeAPIRecord` 
> but to pop it outside? It seems like it's easier to mess up for new kinds of 
> records.
The reason to `emplace_back` the path component inside `serializeAPIRecord` is 
that the `pathComponent` field of the `Record` is serialized in there so you 
want to include the name of the symbol itself in the path component list.
The `pop_back` is delayed til all other additional serialization for a specific 
kind of record, for example `serializeEnumRecord` handles all the enum cases 
after processing the enum record itself using `serializeAPIRecord`. So in order 
to correctly include the enum name in the path components of all the enum 
cases, the enum name has to stay in `PathComponentContext` until all members 
are serialized.

This is exactly the reason why I wanted a clearer API to make it easier to see.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123045/new/

https://reviews.llvm.org/D123045

STAMPS
actor(@zixuw) application(Differential) author(@dang) herald(H423) herald(H576) 
herald(H864) herald(H875) herald(H876) monogram(D123045) object-type(DREV) 
phid(PHID-DREV-d5goxxqici5xa3w2bgjy) reviewer(@QuietMisdreavus) 
reviewer(@ributzka) reviewer(@zixuw) revision-repository(rG) 
revision-status(needs-review) subscriber(@cfe-commits) tag(#all) tag(#clang) 
via(web)

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123045: [clang][extract-api] Fix small issues with SymbolGraphSerializer

[Zixu Wang via Phabricator via cfe-commits]

zixuw added a comment.

In D123045#3427699 , @QuietMisdreavus 
wrote:

> After a quick scan comparing the current output of these symbol graphs with 
> the primary library used for reading them 
> , the last thing i can spot 
> that's "off" is that the "function signature" is currently being serialized 
> under a `parameters` field instead of the required `functionSignature`.

Hmm, I was looking at the format specification at 
https://github.com/apple/swift-docc-symbolkit/blob/0a45209833f4a151212c1aa38e13cfc03b9462e4/openapi.yaml#L307,
 and I searched the term `functionSignature` in the spec but found no property 
with that name (except for the `FunctionSignature` schema that the `parameters` 
property is referring to). But anyway this should be a easy fix.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123045/new/

https://reviews.llvm.org/D123045

STAMPS
actor(@zixuw) application(Differential) author(@dang) herald(H423) herald(H576) 
herald(H864) herald(H875) herald(H876) mention(@QuietMisdreavus) 
monogram(D123045) object-type(DREV) phid(PHID-DREV-d5goxxqici5xa3w2bgjy) 
reviewer(@QuietMisdreavus) reviewer(@ributzka) reviewer(@zixuw) 
revision-repository(rG) revision-status(needs-review) subscriber(@cfe-commits) 
tag(#all) tag(#clang) via(web)

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123019: [clang][extract-api] Add support for typedefs

[Zixu Wang via Phabricator via cfe-commits]

zixuw added inline comments.



Comment at: clang/lib/ExtractAPI/DeclarationFragments.cpp:15
+#include "TypedefUnderlyingTypeResolver.h"
+
 #include "clang/ExtractAPI/DeclarationFragments.h"

Empty line



Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:598-605
+  // Typedefs of anonymous types have their entries unified with the underlying
+  // type.
+  bool ShouldDrop = Record.UnderlyingType.Name.empty();
+  // enums declared with `NS_OPTION` have a named enum and a named typedef, 
with
+  // the same name
+  ShouldDrop |= (Record.UnderlyingType.Name == Record.Name);
+  if (ShouldDrop)

Consider move the should-drop logic into 
`SymbolGraphSerializer::shouldSkip(const APIRecord ) const` so that we 
have a central place to see and manage which symbols get skipped.
This would also simplify things here as the filtering will automatically get 
handled in the following `serializeAPIRecord(Record)` line.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123019/new/

https://reviews.llvm.org/D123019

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123045: [clang][extract-api] Fix small issues with SymbolGraphSerializer

[Zixu Wang via Phabricator via cfe-commits]

zixuw added inline comments.



Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:17
 #include "clang/ExtractAPI/API.h"
+#include "clang/ExtractAPI/DeclarationFragments.h"
 #include "llvm/Support/JSON.h"

Not needed



Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:510
   Symbols.emplace_back(std::move(*Obj));
+  PathComponentContext.pop_back();
 }

What's the cost/would it worth it to wrap the `emplace_back`s and `pop_back`s 
of `PathComponentContext` in meaningful APIs like `enterPathComponentContext` 
and `exitPathComponentContext`?
That way the code is more self-explanatory and easier to read. It took me some 
time and mental resources to figure out why the `pop_back` is placed here.



Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:520-532
   for (const auto  : Record.Constants) {
 auto EnumConstant = serializeAPIRecord(*Constant);
+
+PathComponentContext.pop_back();
+
 if (!EnumConstant)
   continue;

Same comment as above, would be nice to have clear APIs for entering and 
exiting path component contexts.



Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:657
   Root["symbols"] = std::move(Symbols);
-  Root["relationhips"] = std::move(Relationships);
+  Root["relationships"] = std::move(Relationships);
 

oops  



Comment at: clang/test/ExtractAPI/macro_undefined.c:51
 {
+  "accessLevel": "public"m
   "declarationFragments": [

s/m/,/


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123045/new/

https://reviews.llvm.org/D123045

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123056: [clang][extract-api] Undefining macros should not result in a crash

[Zixu Wang via Phabricator via cfe-commits]

zixuw accepted this revision.
zixuw added a comment.
This revision is now accepted and ready to land.

Ha! Nice catch!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123056/new/

https://reviews.llvm.org/D123056

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D122798: [clang][extract-api][NFC] Add documentation

[Zixu Wang via Phabricator via cfe-commits]

zixuw added a comment.

Has this landed yet?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122798/new/

https://reviews.llvm.org/D122798

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D122774: [clang][extract-api] Add Objective-C Category support

[Zixu Wang via Phabricator via cfe-commits]

zixuw updated this revision to Diff 419474.
zixuw added a comment.

Add missing documentatin for ObjCCategoryRecord.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122774/new/

https://reviews.llvm.org/D122774

Files:
  clang/include/clang/ExtractAPI/API.h
  clang/include/clang/ExtractAPI/DeclarationFragments.h
  clang/include/clang/ExtractAPI/Serialization/SymbolGraphSerializer.h
  clang/lib/ExtractAPI/API.cpp
  clang/lib/ExtractAPI/DeclarationFragments.cpp
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
  clang/test/ExtractAPI/objc_category.m

Index: clang/test/ExtractAPI/objc_category.m
===
--- /dev/null
+++ clang/test/ExtractAPI/objc_category.m
@@ -0,0 +1,284 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s@INPUT_DIR@%/t@g" %t/reference.output.json.in >> \
+// RUN: %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 Protocol;
+
+@interface Interface
+@end
+
+@interface Interface (Category) 
+@property int Property;
+- (void)InstanceMethod;
++ (void)ClassMethod;
+@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"
+}
+  },
+  "relationhips": [
+{
+  "kind": "memberOf",
+  "source": "c:objc(cs)Interface(im)InstanceMethod",
+  "target": "c:objc(cs)Interface"
+},
+{
+  "kind": "memberOf",
+  "source": "c:objc(cs)Interface(cm)ClassMethod",
+  "target": "c:objc(cs)Interface"
+},
+{
+  "kind": "memberOf",
+  "source": "c:objc(cs)Interface(py)Property",
+  "target": "c:objc(cs)Interface"
+},
+{
+  "kind": "conformsTo",
+  "source": "c:objc(cs)Interface",
+  "target": "c:objc(pl)Protocol"
+}
+  ],
+  "symbols": [
+{
+  "declarationFragments": [
+{
+  "kind": "keyword",
+  "spelling": "@interface"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "Interface"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "objective-c",
+"precise": "c:objc(cs)Interface"
+  },
+  "kind": {
+"displayName": "Class",
+"identifier": "objective-c.class"
+  },
+  "location": {
+"character": 12,
+"line": 3,
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "Interface"
+  }
+],
+"title": "Interface"
+  }
+},
+{
+  "declarationFragments": [
+{
+  "kind": "text",
+  "spelling": "- ("
+},
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:v",
+  "spelling": "void"
+},
+{
+  "kind": "text",
+  "spelling": ")"
+},
+{
+  "kind": "identifier",
+  "spelling": "InstanceMethod"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "objective-c",
+"precise": "c:objc(cs)Interface(im)InstanceMethod"
+  },
+  "kind": {
+"displayName": "Instance Method",
+"identifier": "objective-c.method"
+  },
+  "location": {
+"character": 1,
+"line": 8,
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"subHeading": [
+  {
+"kind": "text",
+"spelling": "- "
+  },
+  {
+"kind": "identifier",
+"spelling": "InstanceMethod"
+  }
+],
+"title": "InstanceMethod"
+  }
+},
+{
+  "declarationFragments": [
+{
+  "kind": "text",
+  "spelling": "+ ("
+},
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:v",
+  "spelling": "void"
+},
+{
+  "kind": "text",
+  "spelling": ")"
+},
+{
+  "kind": "identifier",
+  

[PATCH] D122774: [clang][extract-api] Add Objective-C Category support

[Zixu Wang via Phabricator via cfe-commits]

zixuw updated this revision to Diff 419298.
zixuw added a comment.

Remove probably unnecessary includes added automatically by clangd.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122774/new/

https://reviews.llvm.org/D122774

Files:
  clang/include/clang/ExtractAPI/API.h
  clang/include/clang/ExtractAPI/DeclarationFragments.h
  clang/include/clang/ExtractAPI/Serialization/SymbolGraphSerializer.h
  clang/lib/ExtractAPI/API.cpp
  clang/lib/ExtractAPI/DeclarationFragments.cpp
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
  clang/test/ExtractAPI/objc_category.m

Index: clang/test/ExtractAPI/objc_category.m
===
--- /dev/null
+++ clang/test/ExtractAPI/objc_category.m
@@ -0,0 +1,284 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s@INPUT_DIR@%/t@g" %t/reference.output.json.in >> \
+// RUN: %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 Protocol;
+
+@interface Interface
+@end
+
+@interface Interface (Category) 
+@property int Property;
+- (void)InstanceMethod;
++ (void)ClassMethod;
+@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"
+}
+  },
+  "relationhips": [
+{
+  "kind": "memberOf",
+  "source": "c:objc(cs)Interface(im)InstanceMethod",
+  "target": "c:objc(cs)Interface"
+},
+{
+  "kind": "memberOf",
+  "source": "c:objc(cs)Interface(cm)ClassMethod",
+  "target": "c:objc(cs)Interface"
+},
+{
+  "kind": "memberOf",
+  "source": "c:objc(cs)Interface(py)Property",
+  "target": "c:objc(cs)Interface"
+},
+{
+  "kind": "conformsTo",
+  "source": "c:objc(cs)Interface",
+  "target": "c:objc(pl)Protocol"
+}
+  ],
+  "symbols": [
+{
+  "declarationFragments": [
+{
+  "kind": "keyword",
+  "spelling": "@interface"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "Interface"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "objective-c",
+"precise": "c:objc(cs)Interface"
+  },
+  "kind": {
+"displayName": "Class",
+"identifier": "objective-c.class"
+  },
+  "location": {
+"character": 12,
+"line": 3,
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "Interface"
+  }
+],
+"title": "Interface"
+  }
+},
+{
+  "declarationFragments": [
+{
+  "kind": "text",
+  "spelling": "- ("
+},
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:v",
+  "spelling": "void"
+},
+{
+  "kind": "text",
+  "spelling": ")"
+},
+{
+  "kind": "identifier",
+  "spelling": "InstanceMethod"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "objective-c",
+"precise": "c:objc(cs)Interface(im)InstanceMethod"
+  },
+  "kind": {
+"displayName": "Instance Method",
+"identifier": "objective-c.method"
+  },
+  "location": {
+"character": 1,
+"line": 8,
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"subHeading": [
+  {
+"kind": "text",
+"spelling": "- "
+  },
+  {
+"kind": "identifier",
+"spelling": "InstanceMethod"
+  }
+],
+"title": "InstanceMethod"
+  }
+},
+{
+  "declarationFragments": [
+{
+  "kind": "text",
+  "spelling": "+ ("
+},
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:v",
+  "spelling": "void"
+},
+{
+  "kind": "text",
+  "spelling": ")"
+},
+{
+  "kind": 

[PATCH] D122774: [clang][extract-api] Add Objective-C Category support

[Zixu Wang via Phabricator via cfe-commits]

zixuw created this revision.
Herald added a reviewer: dang.
Herald added a project: All.
zixuw requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Add (partial) support for Objective-C category records in ExtractAPI.
The current ExtractAPI collects everything for an Objective-C category,
but not fully serialized in the SymbolGraphSerializer. Categories
extending external interfaces are disgarded during serialization, and
categories extending known interfaces are merged (all members surfaced)
into the interfaces.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122774

Files:
  clang/include/clang/ExtractAPI/API.h
  clang/include/clang/ExtractAPI/DeclarationFragments.h
  clang/include/clang/ExtractAPI/Serialization/SymbolGraphSerializer.h
  clang/lib/ExtractAPI/API.cpp
  clang/lib/ExtractAPI/DeclarationFragments.cpp
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
  clang/test/ExtractAPI/objc_category.m

Index: clang/test/ExtractAPI/objc_category.m
===
--- /dev/null
+++ clang/test/ExtractAPI/objc_category.m
@@ -0,0 +1,284 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s@INPUT_DIR@%/t@g" %t/reference.output.json.in >> \
+// RUN: %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 Protocol;
+
+@interface Interface
+@end
+
+@interface Interface (Category) 
+@property int Property;
+- (void)InstanceMethod;
++ (void)ClassMethod;
+@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"
+}
+  },
+  "relationhips": [
+{
+  "kind": "memberOf",
+  "source": "c:objc(cs)Interface(im)InstanceMethod",
+  "target": "c:objc(cs)Interface"
+},
+{
+  "kind": "memberOf",
+  "source": "c:objc(cs)Interface(cm)ClassMethod",
+  "target": "c:objc(cs)Interface"
+},
+{
+  "kind": "memberOf",
+  "source": "c:objc(cs)Interface(py)Property",
+  "target": "c:objc(cs)Interface"
+},
+{
+  "kind": "conformsTo",
+  "source": "c:objc(cs)Interface",
+  "target": "c:objc(pl)Protocol"
+}
+  ],
+  "symbols": [
+{
+  "declarationFragments": [
+{
+  "kind": "keyword",
+  "spelling": "@interface"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "Interface"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "objective-c",
+"precise": "c:objc(cs)Interface"
+  },
+  "kind": {
+"displayName": "Class",
+"identifier": "objective-c.class"
+  },
+  "location": {
+"character": 12,
+"line": 3,
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "Interface"
+  }
+],
+"title": "Interface"
+  }
+},
+{
+  "declarationFragments": [
+{
+  "kind": "text",
+  "spelling": "- ("
+},
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:v",
+  "spelling": "void"
+},
+{
+  "kind": "text",
+  "spelling": ")"
+},
+{
+  "kind": "identifier",
+  "spelling": "InstanceMethod"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "objective-c",
+"precise": "c:objc(cs)Interface(im)InstanceMethod"
+  },
+  "kind": {
+"displayName": "Instance Method",
+"identifier": "objective-c.method"
+  },
+  "location": {
+"character": 1,
+"line": 8,
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"subHeading": [
+  {
+"kind": "text",
+"spelling": "- "
+  },
+  {
+"kind": "identifier",
+"spelling": "InstanceMethod"
+  }
+],
+"title": "InstanceMethod"
+  }
+   

[PATCH] D122611: [clang][extract-api] Add support for macros

[Zixu Wang via Phabricator via cfe-commits]

zixuw accepted this revision.
zixuw added a comment.
This revision is now accepted and ready to land.

LGTM!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122611/new/

https://reviews.llvm.org/D122611

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D122648: [clang][extractapi] Tie API and serialization to the FrontendAction

[Zixu Wang via Phabricator via cfe-commits]

zixuw accepted this revision.
zixuw added a comment.
This revision is now accepted and ready to land.

LGTM!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122648/new/

https://reviews.llvm.org/D122648

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D122611: [clang][extract-api] Add support for macros

[Zixu Wang via Phabricator via cfe-commits]

zixuw added inline comments.



Comment at: clang/include/clang/ExtractAPI/API.h:334
+  using RecordMap = llvm::MapVector>;
+  ///
   /// Get the target triple for the ExtractAPI invocation.

Extra/should-be-empty line here?



Comment at: clang/lib/ExtractAPI/API.cpp:19
 #include "clang/AST/RawCommentList.h"
+#include "clang/ExtractAPI/DeclarationFragments.h"
 #include "clang/Index/USRGeneration.h"

Do we need to include declaration fragments from API.cpp?



Comment at: clang/lib/ExtractAPI/API.cpp:32
+RecordTy *
+addTopLevelRecord(MapVector> ,
+  StringRef Name, CtorArgsTy &&...CtorArgs) {

dang wrote:
> zixuw wrote:
> > Does it make sense to just directly make the `*RecordMap` types in `APISet` 
> > as a `template using`?
> Yup agreed!
Now can we use `APISet::RecordMap` as the type of the first param 
here?
Can also get rid of including MapVector.h


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122611/new/

https://reviews.llvm.org/D122611

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D122511: [clang][extract-api] Add Objective-C protocol support

[Zixu Wang via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd1d34bafef56: [clang][extract-api] Add Objective-C protocol 
support (authored by zixuw).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122511/new/

https://reviews.llvm.org/D122511

Files:
  clang/include/clang/ExtractAPI/API.h
  clang/include/clang/ExtractAPI/DeclarationFragments.h
  clang/lib/ExtractAPI/API.cpp
  clang/lib/ExtractAPI/DeclarationFragments.cpp
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
  clang/test/ExtractAPI/objc_protocol.m

Index: clang/test/ExtractAPI/objc_protocol.m
===
--- /dev/null
+++ clang/test/ExtractAPI/objc_protocol.m
@@ -0,0 +1,146 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s@INPUT_DIR@%/t@g" %t/reference.output.json.in >> \
+// RUN: %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 Protocol
+@end
+
+@protocol AnotherProtocol 
+@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"
+}
+  },
+  "relationhips": [
+{
+  "kind": "conformsTo",
+  "source": "c:objc(pl)AnotherProtocol",
+  "target": "c:objc(pl)Protocol"
+}
+  ],
+  "symbols": [
+{
+  "declarationFragments": [
+{
+  "kind": "keyword",
+  "spelling": "@protocol"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "Protocol"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "objective-c",
+"precise": "c:objc(pl)Protocol"
+  },
+  "kind": {
+"displayName": "Protocol",
+"identifier": "objective-c.protocol"
+  },
+  "location": {
+"character": 11,
+"line": 1,
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "Protocol"
+  }
+],
+"title": "Protocol"
+  }
+},
+{
+  "declarationFragments": [
+{
+  "kind": "keyword",
+  "spelling": "@protocol"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "AnotherProtocol"
+},
+{
+  "kind": "text",
+  "spelling": " <"
+},
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:objc(pl)Protocol",
+  "spelling": "Protocol"
+},
+{
+  "kind": "text",
+  "spelling": ">"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "objective-c",
+"precise": "c:objc(pl)AnotherProtocol"
+  },
+  "kind": {
+"displayName": "Protocol",
+"identifier": "objective-c.protocol"
+  },
+  "location": {
+"character": 11,
+"line": 4,
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "AnotherProtocol"
+  }
+],
+"title": "AnotherProtocol"
+  }
+}
+  ]
+}
Index: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
===
--- clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
+++ clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
@@ -393,6 +393,10 @@
 Kind["identifier"] = AddLangPrefix("class");
 Kind["displayName"] = "Class";
 break;
+  case APIRecord::RK_ObjCProtocol:
+Kind["identifier"] = AddLangPrefix("protocol");
+Kind["displayName"] = "Protocol";
+break;
   }
 
   return Kind;
@@ -593,6 +597,10 @@
   for (const auto  : API.getObjCInterfaces())
 serializeObjCContainerRecord(*ObjCInterface.second);
 
+  // Serialize Objective-C protocol records in the API set.
+  for 

[PATCH] D122446: [clang][extract-api] Add Objective-C interface support

[Zixu Wang via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
zixuw marked an inline comment as done.
Closed by commit rG9b36e126fdb1: [clang][extract-api] Add Objective-C interface 
support (authored by zixuw).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122446/new/

https://reviews.llvm.org/D122446

Files:
  clang/include/clang/ExtractAPI/API.h
  clang/include/clang/ExtractAPI/DeclarationFragments.h
  clang/include/clang/ExtractAPI/Serialization/SymbolGraphSerializer.h
  clang/lib/ExtractAPI/API.cpp
  clang/lib/ExtractAPI/DeclarationFragments.cpp
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
  clang/test/ExtractAPI/objc_interface.m

Index: clang/test/ExtractAPI/objc_interface.m
===
--- /dev/null
+++ clang/test/ExtractAPI/objc_interface.m
@@ -0,0 +1,402 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s@INPUT_DIR@%/t@g" %t/reference.output.json.in >> \
+// RUN: %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 Protocol;
+
+@interface Super 
+@property(readonly, getter=getProperty) unsigned Property;
++ (id)getWithProperty:(unsigned) Property;
+@end
+
+@interface Derived : Super {
+  char Ivar;
+}
+- (char)getIvar;
+@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"
+}
+  },
+  "relationhips": [
+{
+  "kind": "memberOf",
+  "source": "c:objc(cs)Super(cm)getWithProperty:",
+  "target": "c:objc(cs)Super"
+},
+{
+  "kind": "memberOf",
+  "source": "c:objc(cs)Super(py)Property",
+  "target": "c:objc(cs)Super"
+},
+{
+  "kind": "conformsTo",
+  "source": "c:objc(cs)Super",
+  "target": "c:objc(pl)Protocol"
+},
+{
+  "kind": "memberOf",
+  "source": "c:objc(cs)Derived@Ivar",
+  "target": "c:objc(cs)Derived"
+},
+{
+  "kind": "memberOf",
+  "source": "c:objc(cs)Derived(im)getIvar",
+  "target": "c:objc(cs)Derived"
+},
+{
+  "kind": "inheritsFrom",
+  "source": "c:objc(cs)Derived",
+  "target": "c:objc(cs)Super"
+}
+  ],
+  "symbols": [
+{
+  "declarationFragments": [
+{
+  "kind": "keyword",
+  "spelling": "@interface"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "Super"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "objective-c",
+"precise": "c:objc(cs)Super"
+  },
+  "kind": {
+"displayName": "Class",
+"identifier": "objective-c.class"
+  },
+  "location": {
+"character": 12,
+"line": 3,
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "Super"
+  }
+],
+"title": "Super"
+  }
+},
+{
+  "declarationFragments": [
+{
+  "kind": "text",
+  "spelling": "+ ("
+},
+{
+  "kind": "keyword",
+  "spelling": "id"
+},
+{
+  "kind": "text",
+  "spelling": ")"
+},
+{
+  "kind": "identifier",
+  "spelling": "getWithProperty"
+},
+{
+  "kind": "text",
+  "spelling": ":"
+},
+{
+  "kind": "text",
+  "spelling": "("
+},
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:i",
+  "spelling": "unsigned int"
+},
+{
+  "kind": "text",
+  "spelling": ")"
+},
+{
+  "kind": "internalParam",
+  "spelling": "Property"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "objective-c",
+"precise": "c:objc(cs)Super(cm)getWithProperty:"
+  },
+  "kind": {
+"displayName": "Type Method",
+

[PATCH] D122648: [clang][extractapi] Tie API and serialization to the FrontendAction

[Zixu Wang via Phabricator via cfe-commits]

zixuw added a comment.

Mostly LGTM after addressing the inline comments.




Comment at: clang/include/clang/ExtractAPI/FrontendActions.h:42-58
   /// Prepare to execute the action on the given CompilerInstance.
   ///
   /// This is called before executing the action on any inputs. This generates 
a
   /// single header that includes all of CI's inputs and replaces CI's input
   /// list with it before actually executing the action.
   bool PrepareToExecuteAction(CompilerInstance ) override;
 

Should these methods be private?



Comment at: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:335
+  // target triple, let's create the APISet before anyone uses it.
+  API = std::make_unique(CI.getTarget().getTriple(), CI.getLangOpts());
+

Need rebase to pull in the `Language` fix D122495


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122648/new/

https://reviews.llvm.org/D122648

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D122446: [clang][extract-api] Add Objective-C interface support

[Zixu Wang via Phabricator via cfe-commits]

zixuw marked an inline comment as done.
zixuw added inline comments.



Comment at: clang/lib/ExtractAPI/DeclarationFragments.cpp:642-648
+// Instantiate template for FunctionDecl.
+template FunctionSignature
+DeclarationFragmentsBuilder::getFunctionSignature(const FunctionDecl *);
+
+// Instantiate template for ObjCMethodDecl.
+template FunctionSignature
+DeclarationFragmentsBuilder::getFunctionSignature(const ObjCMethodDecl *);

dang wrote:
> Do we need to explicitly instantiate them? Wouldn't they get instantiated as 
> needed?
Yeah since we are implementing the templated method here instead of in the 
header file, we need to explicitly instantiate it so that the symbol ends up in 
`DeclarationFragments.o` and picked up by the linker.



Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:375
+  case APIRecord::RK_ObjCIvar:
+Kind["identifier"] = AddLangPrefix("ivar");
+Kind["displayName"] = "Instance Variable";

dang wrote:
> zixuw wrote:
> > dang wrote:
> > > this should probably be more explicit to keep in line with how things are 
> > > currently done. Maybe something like "instance.variable"
> > Right, naming completely new things here so worth the discussion.
> > To keep in line with the current convention, I don't see instance methods 
> > having an `instance.` prefix. It's just `method` as opposed to 
> > `type.method`.
> > Also global variable is just `var`, if we really need to add the 
> > `instance.` prefix (which I still don't think makes much sense for the 
> > above reason), wouldn't it be `instance.var`?
> Actually, now that I think about it, we should just make them properties. 
> There is already a precedent of doing that for struct fields, and ivars are 
> pretty much the equivalent of a struct field for an interface.
For my understanding, struct fields got named "property" because they behave 
similar as properties, and there are no other possible member kinds in a struct 
to overload that concept. While in an Objective-C container, there are actual 
literal Objective-C "properties." How do we distinguish between ivars and 
properties if they're the same kind?

We might also reconsider the kind for struct fields as "instance property" 
isn't really accurate and largely Objective-C-ish.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122446/new/

https://reviews.llvm.org/D122446

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D122495: [clang][extract-api] Use correct language info from inputs

[Zixu Wang via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG15bf0e567375: [clang][extract-api] Use correct language info 
from inputs (authored by zixuw).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122495/new/

https://reviews.llvm.org/D122495

Files:
  clang/include/clang/ExtractAPI/API.h
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
  clang/test/ExtractAPI/language.c

Index: clang/test/ExtractAPI/language.c
===
--- /dev/null
+++ clang/test/ExtractAPI/language.c
@@ -0,0 +1,166 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s@INPUT_DIR@%/t@g" %t/c.reference.output.json.in >> \
+// RUN: %t/c.reference.output.json
+// RUN: sed -e "s@INPUT_DIR@%/t@g" %t/objc.reference.output.json.in >> \
+// RUN: %t/objc.reference.output.json
+
+// RUN: %clang -extract-api -x c-header -target arm64-apple-macosx \
+// RUN: %t/c.h -o %t/c.output.json | FileCheck -allow-empty %s
+// RUN: %clang -extract-api -x objective-c-header -target arm64-apple-macosx \
+// RUN: %t/objc.h -o %t/objc.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/c.output.json >> %t/c.output-normalized.json
+// RUN: sed -e "s@\"generator\": \".*\"@\"generator\": \"?\"@g" \
+// RUN: %t/objc.output.json >> %t/objc.output-normalized.json
+
+// RUN: diff %t/c.reference.output.json %t/c.output-normalized.json
+// RUN: diff %t/objc.reference.output.json %t/objc.output-normalized.json
+
+// CHECK-NOT: error:
+// CHECK-NOT: warning:
+
+//--- c.h
+char c;
+
+//--- objc.h
+char objc;
+
+//--- c.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"
+}
+  },
+  "relationhips": [],
+  "symbols": [
+{
+  "declarationFragments": [
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:C",
+  "spelling": "char"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "c"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "c",
+"precise": "c:@c"
+  },
+  "kind": {
+"displayName": "Global Variable",
+"identifier": "c.var"
+  },
+  "location": {
+"character": 6,
+"line": 1,
+"uri": "file://INPUT_DIR/c.h"
+  },
+  "names": {
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "c"
+  }
+],
+"title": "c"
+  }
+}
+  ]
+}
+//--- objc.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"
+}
+  },
+  "relationhips": [],
+  "symbols": [
+{
+  "declarationFragments": [
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:C",
+  "spelling": "char"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "objc"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "objective-c",
+"precise": "c:@objc"
+  },
+  "kind": {
+"displayName": "Global Variable",
+"identifier": "objective-c.var"
+  },
+  "location": {
+"character": 6,
+"line": 1,
+"uri": "file://INPUT_DIR/objc.h"
+  },
+  "names": {
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "objc"
+  }
+],
+"title": "objc"
+  }
+}
+  ]
+}
Index: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
===
--- clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
+++ clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
@@ -151,11 +151,9 @@
   return Availbility;
 }
 
-/// Get the short language name string for interface language references.

[PATCH] D122611: [clang][extract-api] Add support for macros

[Zixu Wang via Phabricator via cfe-commits]

zixuw added inline comments.



Comment at: 
clang/include/clang/ExtractAPI/Serialization/SymbolGraphSerializer.h:22
 #include "clang/ExtractAPI/Serialization/SerializerBase.h"
+#include "clang/Lex/PreprocessingRecord.h"
 #include "llvm/Support/JSON.h"

Why do we need this include?



Comment at: clang/lib/ExtractAPI/API.cpp:32
+RecordTy *
+addTopLevelRecord(MapVector> ,
+  StringRef Name, CtorArgsTy &&...CtorArgs) {

Does it make sense to just directly make the `*RecordMap` types in `APISet` as 
a `template using`?



Comment at: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:446-457
+void ExtractAPIAction::EndSourceFileAction() {
+  if (!OS)
+return;
+
+  // Setup a SymbolGraphSerializer to write out collected API information in
+  // the Symbol Graph format.
+  // FIXME: Make the kind of APISerializer configurable.

Worth moving this change and the `APISet` move and relevant structural changes 
into its own patch, as mentioned in D122446.



Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:17
 #include "clang/ExtractAPI/API.h"
+#include "clang/Lex/PreprocessingRecord.h"
 #include "llvm/Support/JSON.h"

Why do we need this include?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122611/new/

https://reviews.llvm.org/D122611

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D122511: [clang][extract-api] Add Objective-C protocol support

[Zixu Wang via Phabricator via cfe-commits]

zixuw added inline comments.



Comment at: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:269
+// Collect symbol information.
+StringRef Name = Decl->getName();
+StringRef USR = API.recordUSR(Decl);

dang wrote:
> I think we should be recording this in StringAllocator
Same reply and concern in D122446


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122511/new/

https://reviews.llvm.org/D122511

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D122511: [clang][extract-api] Add Objective-C protocol support

[Zixu Wang via Phabricator via cfe-commits]

zixuw updated this revision to Diff 418744.
zixuw marked 2 inline comments as done.
zixuw added a comment.

Rebase upstream changes in D122446 :

- Move the change of the `objc_interface.m` test to D122446 
.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122511/new/

https://reviews.llvm.org/D122511

Files:
  clang/include/clang/ExtractAPI/API.h
  clang/include/clang/ExtractAPI/DeclarationFragments.h
  clang/lib/ExtractAPI/API.cpp
  clang/lib/ExtractAPI/DeclarationFragments.cpp
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
  clang/test/ExtractAPI/objc_protocol.m

Index: clang/test/ExtractAPI/objc_protocol.m
===
--- /dev/null
+++ clang/test/ExtractAPI/objc_protocol.m
@@ -0,0 +1,146 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s@INPUT_DIR@%/t@g" %t/reference.output.json.in >> \
+// RUN: %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 Protocol
+@end
+
+@protocol AnotherProtocol 
+@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"
+}
+  },
+  "relationhips": [
+{
+  "kind": "conformsTo",
+  "source": "c:objc(pl)AnotherProtocol",
+  "target": "c:objc(pl)Protocol"
+}
+  ],
+  "symbols": [
+{
+  "declarationFragments": [
+{
+  "kind": "keyword",
+  "spelling": "@protocol"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "Protocol"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "objective-c",
+"precise": "c:objc(pl)Protocol"
+  },
+  "kind": {
+"displayName": "Protocol",
+"identifier": "objective-c.protocol"
+  },
+  "location": {
+"character": 11,
+"line": 1,
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "Protocol"
+  }
+],
+"title": "Protocol"
+  }
+},
+{
+  "declarationFragments": [
+{
+  "kind": "keyword",
+  "spelling": "@protocol"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "AnotherProtocol"
+},
+{
+  "kind": "text",
+  "spelling": " <"
+},
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:objc(pl)Protocol",
+  "spelling": "Protocol"
+},
+{
+  "kind": "text",
+  "spelling": ">"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "objective-c",
+"precise": "c:objc(pl)AnotherProtocol"
+  },
+  "kind": {
+"displayName": "Protocol",
+"identifier": "objective-c.protocol"
+  },
+  "location": {
+"character": 11,
+"line": 4,
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "AnotherProtocol"
+  }
+],
+"title": "AnotherProtocol"
+  }
+}
+  ]
+}
Index: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
===
--- clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
+++ clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
@@ -392,6 +392,10 @@
 Kind["identifier"] = AddLangPrefix("class");
 Kind["displayName"] = "Class";
 break;
+  case APIRecord::RK_ObjCProtocol:
+Kind["identifier"] = AddLangPrefix("protocol");
+Kind["displayName"] = "Protocol";
+break;
   }
 
   return Kind;
@@ -592,6 +596,10 @@
   for (const auto  : API.getObjCInterfaces())
 serializeObjCContainerRecord(*ObjCInterface.second);
 
+  // Serialize Objective-C 

[PATCH] D122446: [clang][extract-api] Add Objective-C interface support

[Zixu Wang via Phabricator via cfe-commits]

zixuw added inline comments.



Comment at: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:245
+if (const auto *SuperClassDecl = Decl->getSuperClass()) {
+  SuperClass.Name = SuperClassDecl->getObjCRuntimeNameAsString();
+  SuperClass.USR = API.recordUSR(SuperClassDecl);

dang wrote:
> Shouldn't we be recording this string in the StringAllocator for reuse later. 
> I have a patch in progress for macro support that will do the serialization 
> once the AST isn't available anymore so we really shouldn't be taking in 
> StringRefs for stuff in the AST.
Right but that only makes sense once we kill the AST before serialization 
right? I mean I'm happy to wrap these in `copyString` now if we know for sure 
that we're doing that in a later patch. But it feels beyond the scope of this 
particular patch. Especially that we'd also need to go through the previous 
code to copy the names of global records, enums, etc. anyway.

I feel like that the change to surface `APISet` and punt serialization should 
be separated out from the macro change. And we can wrap all the name strings in 
that patch so that it's a meaningful atomic commit.



Comment at: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:455
+for (const auto *Protocol : Protocols)
+  Container->Protocols.emplace_back(Protocol->getName(),
+API.recordUSR(Protocol));

dang wrote:
> I think we need to allocate these string in the StringAllocator.
Same as above.



Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:375
+  case APIRecord::RK_ObjCIvar:
+Kind["identifier"] = AddLangPrefix("ivar");
+Kind["displayName"] = "Instance Variable";

dang wrote:
> this should probably be more explicit to keep in line with how things are 
> currently done. Maybe something like "instance.variable"
Right, naming completely new things here so worth the discussion.
To keep in line with the current convention, I don't see instance methods 
having an `instance.` prefix. It's just `method` as opposed to `type.method`.
Also global variable is just `var`, if we really need to add the `instance.` 
prefix (which I still don't think makes much sense for the above reason), 
wouldn't it be `instance.var`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122446/new/

https://reviews.llvm.org/D122446

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D122446: [clang][extract-api] Add Objective-C interface support

[Zixu Wang via Phabricator via cfe-commits]

zixuw updated this revision to Diff 418738.
zixuw marked 4 inline comments as done.
zixuw added a comment.

- Address review comments:
  - Use template to reuse logic for building function signatures for 
FunctionDecl and ObjCMethodDecl.
- Move the change of `objc_interface.m` test in this patch from D12251 
.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122446/new/

https://reviews.llvm.org/D122446

Files:
  clang/include/clang/ExtractAPI/API.h
  clang/include/clang/ExtractAPI/DeclarationFragments.h
  clang/include/clang/ExtractAPI/Serialization/SymbolGraphSerializer.h
  clang/lib/ExtractAPI/API.cpp
  clang/lib/ExtractAPI/DeclarationFragments.cpp
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
  clang/test/ExtractAPI/objc_interface.m

Index: clang/test/ExtractAPI/objc_interface.m
===
--- /dev/null
+++ clang/test/ExtractAPI/objc_interface.m
@@ -0,0 +1,402 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s@INPUT_DIR@%/t@g" %t/reference.output.json.in >> \
+// RUN: %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 Protocol;
+
+@interface Super 
+@property(readonly, getter=getProperty) unsigned Property;
++ (id)getWithProperty:(unsigned) Property;
+@end
+
+@interface Derived : Super {
+  char Ivar;
+}
+- (char)getIvar;
+@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"
+}
+  },
+  "relationhips": [
+{
+  "kind": "memberOf",
+  "source": "c:objc(cs)Super(cm)getWithProperty:",
+  "target": "c:objc(cs)Super"
+},
+{
+  "kind": "memberOf",
+  "source": "c:objc(cs)Super(py)Property",
+  "target": "c:objc(cs)Super"
+},
+{
+  "kind": "conformsTo",
+  "source": "c:objc(cs)Super",
+  "target": "c:objc(pl)Protocol"
+},
+{
+  "kind": "memberOf",
+  "source": "c:objc(cs)Derived@Ivar",
+  "target": "c:objc(cs)Derived"
+},
+{
+  "kind": "memberOf",
+  "source": "c:objc(cs)Derived(im)getIvar",
+  "target": "c:objc(cs)Derived"
+},
+{
+  "kind": "inheritsFrom",
+  "source": "c:objc(cs)Derived",
+  "target": "c:objc(cs)Super"
+}
+  ],
+  "symbols": [
+{
+  "declarationFragments": [
+{
+  "kind": "keyword",
+  "spelling": "@interface"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "Super"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "objective-c",
+"precise": "c:objc(cs)Super"
+  },
+  "kind": {
+"displayName": "Class",
+"identifier": "objective-c.class"
+  },
+  "location": {
+"character": 12,
+"line": 3,
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "Super"
+  }
+],
+"title": "Super"
+  }
+},
+{
+  "declarationFragments": [
+{
+  "kind": "text",
+  "spelling": "+ ("
+},
+{
+  "kind": "keyword",
+  "spelling": "id"
+},
+{
+  "kind": "text",
+  "spelling": ")"
+},
+{
+  "kind": "identifier",
+  "spelling": "getWithProperty"
+},
+{
+  "kind": "text",
+  "spelling": ":"
+},
+{
+  "kind": "text",
+  "spelling": "("
+},
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:i",
+  "spelling": "unsigned int"
+},
+{
+  "kind": "text",
+  "spelling": ")"
+},
+{
+  "kind": "internalParam",
+  "spelling": "Property"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "objective-c",
+"precise": "c:objc(cs)Super(cm)getWithProperty:"
+  },
+