[PATCH] D158646: [clang-tools-extra][ExtractAPI] create clang-symbolgraph-merger

[Daniel Grumberg via Phabricator via cfe-commits]

dang added a comment.

As per https://discourse.llvm.org/t/pull-request-migration-schedule/71595 we 
should move this review to GitHub to make sure we don't lose track of it.


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

https://reviews.llvm.org/D158646

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

[PATCH] D157810: [clang][ExtractAPI] Create extractapi::RecordLocation

[Daniel Grumberg via Phabricator via cfe-commits]

dang added a comment.

As per https://discourse.llvm.org/t/pull-request-migration-schedule/71595 we 
should move this review to GitHub to make sure we don't lose track of it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157810

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

[PATCH] D158671: [NFC][Clang] Fix static analyzer concerns

[Daniel Grumberg via Phabricator via cfe-commits]

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

LGTM




Comment at: clang/include/clang/ExtractAPI/ExtractAPIVisitor.h:195
 SymbolReference Context;
-auto Record = dyn_cast(Decl->getDeclContext());
+auto Record = cast(Decl->getDeclContext());
 Context.Name = Record->getName();

NIT: It's not immediately obvious that the check `if 
(Decl->isStaticDataMember())` guarantees that `Decl->getDeclContext()` is a 
`RecordDecl`. Would you mind adding a comment static that?


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

https://reviews.llvm.org/D158671

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

[PATCH] D157810: [clang][ExtractAPI] Create extractapi::RecordLocation

[Daniel Grumberg via Phabricator via cfe-commits]

dang added a comment.

LGTM




Comment at: clang/include/clang/ExtractAPI/API.h:137
 
-/// DocComment is a vector of RawComment::CommentLine.
+/// Slightly cut down version of PresumedLoc to suite the needs of
+/// ExtractAPI.

s/suite/suit


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

https://reviews.llvm.org/D157810

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

[PATCH] D158474: [clang][ExtractAPI] Fix bool spelling coming from the macro definition.

[Daniel Grumberg via Phabricator via cfe-commits]

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

Nice!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158474

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

[PATCH] D158239: [clang][ExtractAPI] Add support for namespaces

[Daniel Grumberg via Phabricator via cfe-commits]

dang accepted this revision.
dang 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/D158239/new/

https://reviews.llvm.org/D158239

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

[PATCH] D158239: [clang][ExtractAPI] Add support for namespaces

[Daniel Grumberg via Phabricator via cfe-commits]

dang added inline comments.



Comment at: clang/include/clang/ExtractAPI/ExtractAPIVisitor.h:38
+namespace {
+inline SmallString<128> DetermineParentDecl(const DeclContext *Context) {
+  SmallString<128> ParentUSR;

I think this should compute both the parent decl and its USR.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158239

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

[PATCH] D158031: [clang][ExtractAPI] Refactor C++ method and field visitation

[Daniel Grumberg via Phabricator via cfe-commits]

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

Did this change not affect ordering of the symbols in the generated JSON? Looks 
pretty good to me otherwise!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158031

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

[PATCH] D158029: [clang][ExtractAPI] Add support for C++ member templates

[Daniel Grumberg via Phabricator via cfe-commits]

dang accepted this revision.
dang 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/D158029/new/

https://reviews.llvm.org/D158029

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

[PATCH] D157810: [clang][ExtractAPI] Create extractapi::RecordLocation

[Daniel Grumberg via Phabricator via cfe-commits]

dang added inline comments.



Comment at: clang/include/clang/ExtractAPI/ExtractAPIVisitor.h:182-189
   DocComment Comment;
   if (auto *RawComment =
-  getDerivedExtractAPIVisitor().fetchRawCommentForDecl(Decl))
-Comment = RawComment->getFormattedLines(Context.getSourceManager(),
-Context.getDiagnostics());
+  getDerivedExtractAPIVisitor().fetchRawCommentForDecl(Decl)) {
+auto RawCommentVec = RawComment->getFormattedLines(
+Context.getSourceManager(), Context.getDiagnostics());
+std::copy(RawCommentVec.begin(), RawCommentVec.end(),
+  std::back_inserter(Comment));

Can you refactor this code to construct the DocComment into it's own function?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157810

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

[PATCH] D158027: [clang][ExtractAPI] Visit method templates with better scheme

[Daniel Grumberg via Phabricator via cfe-commits]

dang accepted this revision.
dang 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/D158027/new/

https://reviews.llvm.org/D158027

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

[PATCH] D157579: [clang][ExtractAPI] Add support for C++ global function templates

[Daniel Grumberg via Phabricator via cfe-commits]

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

LGTM




Comment at: clang/include/clang/ExtractAPI/API.h:1077
 : public std::true_type {};
+template <>
+struct has_template : public std::true_type {};

Mega nit, can we keep the GlobalFunction stuff grouped together?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157579

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

[PATCH] D158239: [clang][ExtractAPI] Add support for namespaces

[Daniel Grumberg via Phabricator via cfe-commits]

dang added inline comments.



Comment at: clang/include/clang/ExtractAPI/API.h:1129
 template <>
-struct has_function_signature : public std::true_type 
{};
+struct has_function_signature : public std::true_type 
{
+};

Can we get rid of these types of changes to keep the commit history. In general 
we don't reformat code as part of a bigger patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158239

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

[PATCH] D157350: [clang][ExtractAPI] Add support for C++ variable templates

[Daniel Grumberg via Phabricator via cfe-commits]

dang accepted this revision.
dang 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/D157350/new/

https://reviews.llvm.org/D157350

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

[PATCH] D157076: [clang][ExtractAPI] Add support for C++ class templates and concepts

[Daniel Grumberg via Phabricator via cfe-commits]

dang accepted this revision.
dang 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/D157076/new/

https://reviews.llvm.org/D157076

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

[PATCH] D157076: [clang][ExtractAPI] Add support for C++ class templates and concepts

[Daniel Grumberg via Phabricator via cfe-commits]

dang added a comment.

Looks mostly good. Quick Question how do we handle inheritance to a template 
parameter?




Comment at: clang/include/clang/ExtractAPI/API.h:665
+
+struct ClassTemplateSpecRecord : CXXClassRecord {
+  ClassTemplateSpecRecord(StringRef USR, StringRef Name, PresumedLoc Loc,

Minor nit, I would prefer for Specialization to be fully spelled out.



Comment at: clang/include/clang/ExtractAPI/DeclarationFragments.h:191
 
+class Template {
+  struct TemplateParameter {

This is really a model type and should live either in it's own file or in API.h



Comment at: clang/include/clang/ExtractAPI/DeclarationFragments.h:313
+  /// Get template details from a template function, class, or variable
+  static Template getTemplate(const TemplateDecl *Decl) {
+Template Template;

I feel this should be a constructor for Template seeing that this never fails.



Comment at: clang/include/clang/ExtractAPI/ExtractAPIVisitor.h:435
+Decl->getDescribedClassTemplate()));
+// Cast to easily use previous declaration to get bases.
+CXXClassRecord = API.addClassTemplate(

Where is the cast?



Comment at: clang/lib/ExtractAPI/DeclarationFragments.cpp:754
+  Fragments.append(TemplateParam->getTypeConstraint()
+   ->getNamedConcept()
+   ->getName()

is this clang-format formatted?



Comment at: clang/lib/ExtractAPI/DeclarationFragments.cpp:784
+dyn_cast(TemplateParameters[i]);
+if (TypeParameter.compare("type-parameter-" +
+  std::to_string(Parameter->getDepth()) + "-" +

Kinda sad we have to do this. I guess there is no easy way to change the AST to 
support this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157076

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

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

[Daniel Grumberg via Phabricator via cfe-commits]

dang accepted this revision.
dang added a comment.

LGTM


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] D152770: [clang][ExtractAPI] Add support for Objective-C categories

[Daniel Grumberg via Phabricator via cfe-commits]

dang added inline comments.



Comment at: clang/test/ExtractAPI/objc_various_categories.m:8
+// RUN: %t/myclass_1.h \
+// RUN: %t/input.h -o %t/output.json | FileCheck -allow-empty %s
+

I don't think you need to have MyClass2 and the associated myclass2.h. It's 
fine to check that everything looks right just for the NSString categories.


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] D152770: [clang][ExtractAPI] Add support for Objective-C categories

[Daniel Grumberg via Phabricator via cfe-commits]

dang accepted this revision.
dang 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/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] D157007: [clang][ExtractAPI] Add support for C++ classes with fix

[Daniel Grumberg via Phabricator via cfe-commits]

dang accepted this revision.
dang 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/D157007/new/

https://reviews.llvm.org/D157007

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

[PATCH] D153557: [clang][ExtractAPI] Add support for C++ classes

[Daniel Grumberg via Phabricator via cfe-commits]

dang accepted this revision.
dang 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/D153557/new/

https://reviews.llvm.org/D153557

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

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

[Daniel Grumberg via Phabricator via cfe-commits]

dang added inline comments.



Comment at: clang/include/clang/ExtractAPI/ExtractAPIVisitor.h:488
+  bool IsFromExternalModule = true;
+  for (const auto  : API.getObjCInterfaces()) {
+if (InterfaceDecl->getName() == Interface.second.get()->Name) {

I think this is fine for now but there must be a better way of doing this that 
doesn't involve traversing the whole APISet



Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:219
   Object Identifier;
-  Identifier["precise"] = Record.USR;
+  if (auto *CategoryRecord =
+  dyn_cast_or_null())

I don't think this is right. My expectation is that category records that need 
to get emitted would still have their own unique USR.



Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:759
+  // Check if the current Category Record has been visited before, if not add.
+  if (!(visitedCategories.contains(Record.Interface.Name) > 0)) {
+visitedCategories.insert(Record.Interface.Name);

What happens when there are multiple categories to the same type though?



Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:775
+  Relationship["targetFallback"] = Record.Interface.Name;
+  Relationship["kind"] = getRelationshipString(RelationshipKind::MemberOf);
+  Relationships.emplace_back(std::move(Relationship));

I would expect this to be a new relationship kind that reads as "extensionTo"


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] D154038: [clang][ExtractAPI] Add semicolons to vars and fields and to test reference JSON

[Daniel Grumberg via Phabricator via cfe-commits]

dang accepted this revision.
dang 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/D154038/new/

https://reviews.llvm.org/D154038

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

[PATCH] D153557: [clang][ExtractAPI] Add support for C++ classes

[Daniel Grumberg via Phabricator via cfe-commits]

dang added a comment.

Looking pretty good, if you can address the last few bits of feedback I am 
happy to merge this.




Comment at: clang/include/clang/ExtractAPI/API.h:770
+template <>
+struct has_function_signature : public std::true_type {};
+

Does `CXXInstanceMethodRecord` need one of these as well?



Comment at: clang/include/clang/ExtractAPI/DeclarationFragments.h:26
 #include "clang/AST/DeclObjC.h"
+#include "clang/Basic/Specifiers.h"
 #include "clang/Lex/MacroInfo.h"

Are these necessary?



Comment at: clang/include/clang/ExtractAPI/DeclarationFragments.h:190
+public:
+  AccessControl() = default;
+

For ergonomics would it make more sense to have a constructor that takes the 
string in directly and moves it to `Access`? I don't think there are any 
situation where you want to change the access string after the fact?



Comment at: clang/include/clang/ExtractAPI/DeclarationFragments.h:251
+  template 
+  static FunctionSignature getFunctionSignature(const FunctionT *Function) {
+FunctionSignature Signature;

I assume the implementation of this moved because it was needed for C++ 
methods? Can you update the comment and maybe move the implementation to the 
bottom of the `.h` file?



Comment at: clang/lib/ExtractAPI/DeclarationFragments.cpp:614
+  } else if (isa(Method))
+// TODO: add '~'
+Name = cast(Method->getDeclContext())->getName();

Please address this, it's pretty important imo.



Comment at: clang/test/ExtractAPI/methods.cpp:13
+
+// CHECK-NOT: error:
+// CHECK-NOT: warning:

I think these are redundant since you have `// expected-no-diagnostics` below 
and pass ``--verify`` to cc1.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153557

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

[PATCH] D152356: [clang][ExtractAPI] Add --emit-symbol-graph option

[Daniel Grumberg via Phabricator via cfe-commits]

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

LGTM thanks for working on this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152356

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

[PATCH] D154038: [clang][ExtractAPI] Add semicolons to vars and fields and to test reference JSON

[Daniel Grumberg via Phabricator via cfe-commits]

dang added inline comments.



Comment at: clang/lib/ExtractAPI/DeclarationFragments.cpp:437
   .append(Var->getName(), DeclarationFragments::FragmentKind::Identifier)
+  .append(";", DeclarationFragments::FragmentKind::Text)
   .append(std::move(After));

Should this be after the line below?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154038

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

[PATCH] D152356: [clang][ExtractAPI] Add --emit-symbol-graph option

[Daniel Grumberg via Phabricator via cfe-commits]

dang added inline comments.



Comment at: clang/include/clang/ExtractAPI/ExtractAPIActionBase.h:25
+///
+/// Deriving from this class equipts an action with all the necessary tools to
+/// generate ExractAPI information in form of symbol-graphs





Comment at: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:227
 
+struct BasicExtractAPIVisitor : ExtractAPIVisitor {
+  BasicExtractAPIVisitor(ASTContext , APISet )

I don't think this is needed `ExtractAPIVisitor` was written so that it could 
be used as is. If it isn't then it's a bug we should fix.



Comment at: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:255
 
+class SymbolGraphConsumer : public ASTConsumer {
+public:

Not sure I like the name `SymbolGraphConsumer`, but I don't have a great 
suggestion, maybe `WrappingExtractAPIConsumer` so that at least it's clear that 
it is intended to be used by `WrappingExtractAPIAction`?



Comment at: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:285
 
-class MacroCallback : public PPCallbacks {
+class MacroCallBack : public PPCallbacks {
 public:

the name change here is unnecessary



Comment at: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:567-575
+bool IsQuoted = false;
+for (const FrontendInputFile  : Inputs) {
+  StringRef FilePath = FIF.getFile();
+  if (auto RelativeName = getRelativeIncludeName(CI, FilePath, ))
+KnownInputFiles.emplace_back(
+static_cast>(*RelativeName), IsQuoted);
+  else

I don't think we need to do all this computation since we just serialize symbol 
graphs for all symbols.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152356

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

[PATCH] D153557: [clang][ExtractAPI] Add support for C++ classes

[Daniel Grumberg via Phabricator via cfe-commits]

dang added a comment.

Starting to come together nicely, but I think now would be a good time to write 
some tests. I am particularly interested in more complex situations like 
inheritance hierarchies.




Comment at: clang/include/clang/ExtractAPI/API.h:25
 #include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/Specifiers.h"
 #include "clang/ExtractAPI/AvailabilityInfo.h"

Inste



Comment at: clang/include/clang/ExtractAPI/API.h:637
+return (Record->getKind() == RK_CXXClass ||
+Record->getKind() == RK_Struct || Record->getKind() == RK_Union);
+  }

Not sure this is correct, the semantic relationship is that structs and unions 
can be classes. This expresses that all struct records and unions are 
CXXClassRecords. It would be best to keep these entirely separate as `classof` 
is used by the type casting functions, e.g. `llvm::dynamic_cast` and we 
certainly don't want arbitrary struct or union record to be cast-able to 
CXXClassRecord.



Comment at: clang/include/clang/ExtractAPI/API.h:771
 
+template <>
+struct has_function_signature : public std::true_type {};

mega nit: I suggest grouping together the `has_function_signature` trait and 
the `has_access` trait separately, i.e. moving the empty line from 770 to 
between 772 and 773



Comment at: clang/include/clang/ExtractAPI/DeclarationFragments.h:187
 
+class AccessControl {
+public:

Should this be API.h?



Comment at: clang/include/clang/ExtractAPI/DeclarationFragments.h:198
+private:
+  StringRef Access;
+};

Instead of storing a reference to the string here, it might make sense to store 
the string itself, I am worried that for future usages in libclang the string 
references would go stale.



Comment at: clang/include/clang/ExtractAPI/ExtractAPIVisitor.h:318-319
+bool ExtractAPIVisitorBase::WalkUpFromRecordDecl(
+const RecordDecl *Decl) {
+  VisitRecordDecl(Decl);
+  return true;

Is it to short circuit to only call `VisitRecordDecl` if so I think that it 
should call `getDerivedExtractAPIVisitor().VisitRecordDecl`



Comment at: clang/include/clang/ExtractAPI/ExtractAPIVisitor.h:400
+  // FIXME: store AccessSpecifier given by inheritance
+  for (const auto BaseSpecifier : Decl->bases()) {
+SymbolReference BaseClass;

Is there a way of only looking at public bases here? Non-public inheritance is 
not part of the API surface of the class.



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

AccessControl should move into API.h so we don't need to pull in 
DeclarationFragments.h



Comment at: clang/lib/ExtractAPI/API.cpp:44
 }
-
 } // namespace

This just pollutes the git history, would be best to remove this.



Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:607
-  // correctly here.
-  Obj["accessLevel"] = "public";
   SmallVector PathComponentsNames;

We still need all symbols to have an "accessLevel" specifier. Maybe the 
false_type overload of `serializeAccessMixin` could make "public" the default.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153557

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

[PATCH] D151477: [clang][ExtractAPI] Refactor serializer to the CRTP

[Daniel Grumberg via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG06ff9770477d: [clang][ExtractAPI] Refactor serializer to the 
CRTP (authored by evelez7, committed by dang).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151477

Files:
  clang/include/clang/ExtractAPI/Serialization/SerializerBase.h
  clang/include/clang/ExtractAPI/Serialization/SymbolGraphSerializer.h
  clang/lib/ExtractAPI/CMakeLists.txt
  clang/lib/ExtractAPI/Serialization/SerializerBase.cpp
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp

Index: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
===
--- clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
+++ clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
@@ -14,16 +14,11 @@
 #include "clang/ExtractAPI/Serialization/SymbolGraphSerializer.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Version.h"
-#include "clang/ExtractAPI/API.h"
-#include "clang/ExtractAPI/APIIgnoresList.h"
 #include "clang/ExtractAPI/DeclarationFragments.h"
-#include "clang/ExtractAPI/Serialization/SerializerBase.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/STLFunctionalExtras.h"
-#include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Compiler.h"
-#include "llvm/Support/JSON.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/VersionTuple.h"
 #include 
@@ -541,19 +536,16 @@
 Array generateParentContexts(const RecordTy , const APISet ,
  Language Lang) {
   Array ParentContexts;
-  generatePathComponents(Record, API,
- [Lang, ](const PathComponent ) {
-   ParentContexts.push_back(
-   serializeParentContext(PC, Lang));
- });
+  generatePathComponents(
+  Record, API, [Lang, ](const PathComponent ) {
+ParentContexts.push_back(serializeParentContext(PC, Lang));
+  });
 
   return ParentContexts;
 }
 
 } // namespace
 
-void SymbolGraphSerializer::anchor() {}
-
 /// Defines the format version emitted by SymbolGraphSerializer.
 const VersionTuple SymbolGraphSerializer::FormatVersion{0, 5, 3};
 
@@ -670,7 +662,7 @@
   Relationships.emplace_back(std::move(Relationship));
 }
 
-void SymbolGraphSerializer::serializeGlobalFunctionRecord(
+void SymbolGraphSerializer::visitGlobalFunctionRecord(
 const GlobalFunctionRecord ) {
   auto Obj = serializeAPIRecord(Record);
   if (!Obj)
@@ -679,7 +671,7 @@
   Symbols.emplace_back(std::move(*Obj));
 }
 
-void SymbolGraphSerializer::serializeGlobalVariableRecord(
+void SymbolGraphSerializer::visitGlobalVariableRecord(
 const GlobalVariableRecord ) {
   auto Obj = serializeAPIRecord(Record);
   if (!Obj)
@@ -688,7 +680,7 @@
   Symbols.emplace_back(std::move(*Obj));
 }
 
-void SymbolGraphSerializer::serializeEnumRecord(const EnumRecord ) {
+void SymbolGraphSerializer::visitEnumRecord(const EnumRecord ) {
   auto Enum = serializeAPIRecord(Record);
   if (!Enum)
 return;
@@ -697,7 +689,7 @@
   serializeMembers(Record, Record.Constants);
 }
 
-void SymbolGraphSerializer::serializeStructRecord(const StructRecord ) {
+void SymbolGraphSerializer::visitStructRecord(const StructRecord ) {
   auto Struct = serializeAPIRecord(Record);
   if (!Struct)
 return;
@@ -706,7 +698,7 @@
   serializeMembers(Record, Record.Fields);
 }
 
-void SymbolGraphSerializer::serializeObjCContainerRecord(
+void SymbolGraphSerializer::visitObjCContainerRecord(
 const ObjCContainerRecord ) {
   auto ObjCContainer = serializeAPIRecord(Record);
   if (!ObjCContainer)
@@ -743,7 +735,7 @@
   }
 }
 
-void SymbolGraphSerializer::serializeMacroDefinitionRecord(
+void SymbolGraphSerializer::visitMacroDefinitionRecord(
 const MacroDefinitionRecord ) {
   auto Macro = serializeAPIRecord(Record);
 
@@ -758,28 +750,28 @@
   case APIRecord::RK_Unknown:
 llvm_unreachable("Records should have a known kind!");
   case APIRecord::RK_GlobalFunction:
-serializeGlobalFunctionRecord(*cast(Record));
+visitGlobalFunctionRecord(*cast(Record));
 break;
   case APIRecord::RK_GlobalVariable:
-serializeGlobalVariableRecord(*cast(Record));
+visitGlobalVariableRecord(*cast(Record));
 break;
   case APIRecord::RK_Enum:
-serializeEnumRecord(*cast(Record));
+visitEnumRecord(*cast(Record));
 break;
   case APIRecord::RK_Struct:
-serializeStructRecord(*cast(Record));
+visitStructRecord(*cast(Record));
 break;
   case APIRecord::RK_ObjCInterface:
-serializeObjCContainerRecord(*cast(Record));
+visitObjCContainerRecord(*cast(Record));
 break;
   case APIRecord::RK_ObjCProtocol:
-serializeObjCContainerRecord(*cast(Record));
+visitObjCContainerRecord(*cast(Record));
 break;
   case APIRecord::RK_MacroDefinition:
-

[PATCH] D151477: [clang][ExtractAPI] Refactor serializer to the CRTP

[Daniel Grumberg via Phabricator via cfe-commits]

dang added a comment.

LGTM with minor changes




Comment at: clang/include/clang/ExtractAPI/Serialization/SerializerBase.h:58
 
-  virtual ~APISerializer() = default;
 };

It would be nice to keep this as default, i.e. 
```
~APISetVisitor() = default;
```



Comment at: 
clang/include/clang/ExtractAPI/Serialization/SymbolGraphSerializer.h:20
 
-#include "clang/ExtractAPI/API.h"
 #include "clang/ExtractAPI/APIIgnoresList.h"

In LLVM we tend to explicitly include any header we use and we definitely use 
the definitions ins API.h



Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:748
 
-void SymbolGraphSerializer::serializeSingleRecord(const APIRecord *Record) {
+void SymbolGraphSerializer::visitSingleRecord(const APIRecord *Record) {
   switch (Record->getKind()) {

This isn't part of the visitation scheme but more of a way of serializing a 
single record. I would prefer to keep this as `serializeSingleRecord` 



Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:808
 Object SymbolGraphSerializer::serialize() {
-  // Serialize global variables in the API set.
-  for (const auto  : API.getGlobalVariables())
-serializeGlobalVariableRecord(*GlobalVar.second);
-
-  for (const auto  : API.getGlobalFunctions())
-serializeGlobalFunctionRecord(*GlobalFunction.second);
-
-  // Serialize enum records in the API set.
-  for (const auto  : API.getEnums())
-serializeEnumRecord(*Enum.second);
-
-  // Serialize struct records in the API set.
-  for (const auto  : API.getStructs())
-serializeStructRecord(*Struct.second);
-
-  // Serialize Objective-C interface records in the API set.
-  for (const auto  : API.getObjCInterfaces())
-serializeObjCContainerRecord(*ObjCInterface.second);
-
-  // Serialize Objective-C protocol records in the API set.
-  for (const auto  : API.getObjCProtocols())
-serializeObjCContainerRecord(*ObjCProtocol.second);
-
-  for (const auto  : API.getMacros())
-serializeMacroDefinitionRecord(*Macro.second);
-
-  for (const auto  : API.getTypedefs())
-serializeTypedefRecord(*Typedef.second);
-
+  APISetVisitor::traverseAPISet();
   return serializeCurrentGraph();

Does this need to be explicit like this? Would it not work to just call 
`traverseAPISet();` since we would inherit it through inheritance.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151477

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

[PATCH] D151293: [clang][ExtractAPI] Refactor serializer to the CRTP

[Daniel Grumberg via Phabricator via cfe-commits]

dang requested changes to this revision.
dang added a comment.
This revision now requires changes to proceed.

Great start but there are still some rough edges to polish!




Comment at: clang/include/clang/ExtractAPI/Serialization/SerializerBase.h:19
 #include "clang/ExtractAPI/APIIgnoresList.h"
+#include "llvm/Support/JSON.h"
 #include "llvm/Support/raw_ostream.h"

This shouldn't be needed at this point.



Comment at: clang/include/clang/ExtractAPI/Serialization/SerializerBase.h:28-31
+struct APISetVisitorOption {
   /// Do not include unnecessary whitespaces to save space.
   bool Compact;
 };

This is very specific to SymbolGraphSerializer or at last to JSON.



Comment at: clang/include/clang/ExtractAPI/Serialization/SerializerBase.h:37
+  /// Traverse the API information to output to \p os.
+  void traverseAPISet(raw_ostream ){};
+

This shouldn't be needed anymore.



Comment at: clang/include/clang/ExtractAPI/Serialization/SerializerBase.h:84
+for (const auto  : API.getObjCProtocols())
+  getDerived()->visitObjCContainerRecord(*Protocol.second);
+  }

This should visit some sort Protocol Records specifically. Likewise for 
interfaces.



Comment at: clang/include/clang/ExtractAPI/Serialization/SerializerBase.h:100
+  /// Visit a global variable record.
+  void visitGlobalVariableRecord(const GlobalVariableRecord );
+

This should have a default implementation.



Comment at: clang/include/clang/ExtractAPI/Serialization/SerializerBase.h:138
 protected:
-  APISerializer(const APISet , const APIIgnoresList ,
-APISerializerOption Options = {})
+  APISetVisitor(const APISet , const APIIgnoresList ,
+APISetVisitorOption Options = {})

I think that `APIIgnoresList` Should be pushed down into the 
SymbolGraphSerializer.



Comment at: clang/include/clang/ExtractAPI/Serialization/SerializerBase.h:142
 
-  virtual ~APISerializer() = default;
+  virtual ~APISetVisitor() = default;
+  Derived *getDerived() { return static_cast(this); };

Don't need a virtual destructor anymore.



Comment at: 
clang/include/clang/ExtractAPI/Serialization/SymbolGraphSerializer.h:12
 ///
-/// Implement an APISerializer for the Symbol Graph format for ExtractAPI.
+/// Implement an APISetVisitor for the Symbol Graph format for ExtractAPI.
 /// See https://github.com/apple/swift-docc-symbolkit.





Comment at: 
clang/include/clang/ExtractAPI/Serialization/SymbolGraphSerializer.h:94
   /// Just serialize the currently recorded objects in Symbol Graph format.
-  Object serializeCurrentGraph();
+  Object visitCurrentGraph();
 

I think these should still be called serializeXXX as they aren't part of the 
visitation interface and are very specific to the symbol graph format.



Comment at: 
clang/include/clang/ExtractAPI/Serialization/SymbolGraphSerializer.h:129
   template 
-  void serializeMembers(const APIRecord ,
-const SmallVector> );
+  void visitMembers(const APIRecord ,
+const SmallVector> );

I think we shouldn't need to have this method. APISetVisitor should handle 
calling the relevant visitMethods for the members.



Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:221
 /// references, and the interface language name.
-Object serializeIdentifier(const APIRecord , Language Lang) {
+Object visitIdentifier(const APIRecord , Language Lang) {
   Object Identifier;

I think all these helper methods for serializing specific chunks of symbol 
graphs should still be names serialize


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151293

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

[PATCH] D151048: [clang][ExtractAPI] Modify declaration fragment methods to add a new fragment at an arbitrary offset.

[Daniel Grumberg via Phabricator via cfe-commits]

dang requested changes to this revision.
dang added inline comments.
This revision now requires changes to proceed.



Comment at: clang/include/clang/ExtractAPI/DeclarationFragments.h:102
 
-  // Add a new Fragment to the beginning of the Fragments.
-  DeclarationFragments (StringRef Spelling, FragmentKind Kind,
-StringRef PreciseIdentifier = "",
-const Decl *Declaration = nullptr) {
-Fragments.emplace(Fragments.begin(), Spelling, Kind, PreciseIdentifier,
-  Declaration);
+  size_t calculateOffset(intmax_t Index) const {
+if (Index >= 0) {

This shouldn't need to be part of the public interface of the 
DeclarationFragments type and can just be a static function in 
DeclarationFragments.cpp.



Comment at: clang/include/clang/ExtractAPI/DeclarationFragments.h:114
+  // Add a new Fragment at an arbitrary offset.
+  DeclarationFragments (intmax_t Index, StringRef Spelling,
+  FragmentKind Kind,

Is this parameter `intmax_t` to allow negative indices? If so I really don't 
think it's a good idea. I would prefer if we exposed an iterator interface to 
DeclarationFragments to make the API more like that of a vector type. (The 
iterator would just need to be the iterator for the underlying vector).



Comment at: clang/include/clang/ExtractAPI/ExtractAPIVisitor.h:119
+  .insertAtIndex(1, " ", DeclarationFragments::FragmentKind::Text)
+  .insertAtIndex(-1, " { ... } ",
+ DeclarationFragments::FragmentKind::Text)

I don't think the API naming makes it clear that you can and what it means to 
insert at a negative offset. I would much rather we had an iterator based 
interface and didn't necessarily do chained calls. It could look something like 
this:
```
auto  = Record.second->Declaration;
DeclFrag.insert(DeclFrag.begin(), "typedef", 
DeclarationGragments::FragmentKind::Keyword, "", nullptr);
// the rest of the code
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151048

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

[PATCH] D149737: [clang][ExtractAPI] Add semicolon to function declaration fragments

[Daniel Grumberg via Phabricator via cfe-commits]

dang accepted this revision.
dang 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/D149737/new/

https://reviews.llvm.org/D149737

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

[PATCH] D146385: [clang][ExtractAPI] Complete declaration fragments for TagDecl types defined in a typedef

[Daniel Grumberg 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 rG3ac550984e83: [clang][ExtractAPI] Complete declaration 
fragments for TagDecl types defined in… (authored by Ruturaj4, committed by 
dang).

Changed prior to commit:
  https://reviews.llvm.org/D146385?vs=509473=513286#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146385

Files:
  clang/include/clang/ExtractAPI/DeclarationFragments.h
  clang/include/clang/ExtractAPI/ExtractAPIVisitor.h
  clang/test/ExtractAPI/typedef_struct_enum.c

Index: clang/test/ExtractAPI/typedef_struct_enum.c
===
--- /dev/null
+++ clang/test/ExtractAPI/typedef_struct_enum.c
@@ -0,0 +1,441 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
+// RUN: %t/reference.output.json.in >> %t/reference.output.json
+// RUN: %clang -extract-api -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
+typedef struct Test {
+} Test;
+
+typedef enum Test2 {
+  simple
+} Test2;
+
+struct Foo;
+typedef struct Foo TypedefedFoo;
+struct Foo {
+int bar;
+};
+
+//--- 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:@E@Test2@simple",
+  "target": "c:@E@Test2",
+  "targetFallback": "Test2"
+},
+{
+  "kind": "memberOf",
+  "source": "c:@S@Foo@FI@bar",
+  "target": "c:@S@Foo",
+  "targetFallback": "Foo"
+}
+  ],
+  "symbols": [
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "keyword",
+  "spelling": "typedef"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "keyword",
+  "spelling": "enum"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "Test2"
+},
+{
+  "kind": "text",
+  "spelling": ": "
+},
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:i",
+  "spelling": "unsigned int"
+},
+{
+  "kind": "text",
+  "spelling": " { ... } "
+},
+{
+  "kind": "identifier",
+  "spelling": "Test2"
+},
+{
+  "kind": "text",
+  "spelling": ";"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "c",
+"precise": "c:@E@Test2"
+  },
+  "kind": {
+"displayName": "Enumeration",
+"identifier": "c.enum"
+  },
+  "location": {
+"position": {
+  "character": 14,
+  "line": 4
+},
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"navigator": [
+  {
+"kind": "identifier",
+"spelling": "Test2"
+  }
+],
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "Test2"
+  }
+],
+"title": "Test2"
+  },
+  "pathComponents": [
+"Test2"
+  ]
+},
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "identifier",
+  "spelling": "simple"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "c",
+"precise": "c:@E@Test2@simple"
+  },
+  "kind": {
+"displayName": "Enumeration Case",
+"identifier": "c.enum.case"
+  },
+  "location": {
+"position": {
+  "character": 3,
+  "line": 5
+},
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"navigator": [
+  {
+"kind": "identifier",
+"spelling": "simple"
+  }
+],
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "simple"
+  }
+],
+"title": "simple"
+  },
+  

[PATCH] D147901: [NFC][CLANG][API] Fix coverity remarks about large copies by values

[Daniel Grumberg via Phabricator via cfe-commits]

dang added inline comments.



Comment at: clang/include/clang/ExtractAPI/API.h:138
   APIRecord(RecordKind Kind, StringRef USR, StringRef Name,
-PresumedLoc Location, AvailabilitySet Availabilities,
+PresumedLoc Location, const AvailabilitySet ,
 LinkageInfo Linkage, const DocComment ,

RKSimon wrote:
> aaron.ballman wrote:
> > A lot of these changes look to be regressions, so I think Coverity is 
> > incorrect to flag these. The old code is passing an `AvailabilitySet` by 
> > value because it's doing a move operation on initialization: 
> > `Availabilities(std::move(Availabilities))`. Making this into a const 
> > reference defeats that optimization because you can't steal resources from 
> > a const object (so this turns a move into a copy).
> > 
> > You should look through the rest of the patch for similar problematic 
> > changes.
> I've never been sure whether coverity is being particularly poor at 
> recognising the std::move pattern or particularly smart at realising it can't 
> occur for some reason.
Yeah this certainly doesn't look right, since the new version tries to move 
from the const reference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147901

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

[PATCH] D147234: [clang][ExtractAPI] Reland ExtractAPI for libclang improvements

[Daniel Grumberg via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG142c3d9d1414: [clang][ExtractAPI] Reland ExtractAPI for 
libclang improvements (authored by dang).

Changed prior to commit:
  https://reviews.llvm.org/D147234?vs=509660=509714#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147234

Files:
  clang/include/clang/ExtractAPI/ExtractAPIVisitor.h
  clang/include/clang/ExtractAPI/TypedefUnderlyingTypeResolver.h
  clang/lib/ExtractAPI/CMakeLists.txt
  clang/lib/ExtractAPI/DeclarationFragments.cpp
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/lib/ExtractAPI/ExtractAPIVisitor.cpp
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
  clang/lib/ExtractAPI/TypedefUnderlyingTypeResolver.cpp
  clang/lib/ExtractAPI/TypedefUnderlyingTypeResolver.h
  clang/test/Index/extract-api-cursor.m
  clang/test/Index/extract-api-usr.m
  clang/tools/c-index-test/c-index-test.c
  clang/tools/libclang/CXExtractAPI.cpp

Index: clang/tools/libclang/CXExtractAPI.cpp
===
--- clang/tools/libclang/CXExtractAPI.cpp
+++ clang/tools/libclang/CXExtractAPI.cpp
@@ -18,6 +18,7 @@
 #include "clang-c/Index.h"
 #include "clang-c/Platform.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/DeclObjC.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/ExtractAPI/API.h"
 #include "clang/ExtractAPI/ExtractAPIVisitor.h"
@@ -34,13 +35,73 @@
 using namespace clang;
 using namespace clang::extractapi;
 
+namespace {
+struct LibClangExtractAPIVisitor
+: ExtractAPIVisitor {
+  using Base = ExtractAPIVisitor;
+
+  LibClangExtractAPIVisitor(ASTContext , APISet )
+  : ExtractAPIVisitor(Context, API) {}
+
+  const RawComment *fetchRawCommentForDecl(const Decl *D) const {
+return Context.getRawCommentForAnyRedecl(D);
+  }
+
+  // We need to visit implementations as well to ensure that when a user clicks
+  // on a method defined only within the implementation that we can still
+  // provide a symbol graph for it.
+  bool VisitObjCImplementationDecl(const ObjCImplementationDecl *Decl) {
+if (!shouldDeclBeIncluded(Decl))
+  return true;
+
+const ObjCInterfaceDecl *Interface = Decl->getClassInterface();
+StringRef Name = Interface->getName();
+StringRef USR = API.recordUSR(Decl);
+PresumedLoc Loc =
+Context.getSourceManager().getPresumedLoc(Decl->getLocation());
+LinkageInfo Linkage = Decl->getLinkageAndVisibility();
+DocComment Comment;
+if (auto *RawComment = fetchRawCommentForDecl(Interface))
+  Comment = RawComment->getFormattedLines(Context.getSourceManager(),
+  Context.getDiagnostics());
+
+// Build declaration fragments and sub-heading by generating them for the
+// interface.
+DeclarationFragments Declaration =
+DeclarationFragmentsBuilder::getFragmentsForObjCInterface(Interface);
+DeclarationFragments SubHeading =
+DeclarationFragmentsBuilder::getSubHeading(Decl);
+
+// Collect super class information.
+SymbolReference SuperClass;
+if (const auto *SuperClassDecl = Decl->getSuperClass()) {
+  SuperClass.Name = SuperClassDecl->getObjCRuntimeNameAsString();
+  SuperClass.USR = API.recordUSR(SuperClassDecl);
+}
+
+ObjCInterfaceRecord *ObjCInterfaceRecord = API.addObjCInterface(
+Name, USR, Loc, AvailabilitySet(Decl), Linkage, Comment, Declaration,
+SubHeading, SuperClass, isInSystemHeader(Decl));
+
+// Record all methods (selectors). This doesn't include automatically
+// synthesized property methods.
+recordObjCMethods(ObjCInterfaceRecord, Decl->methods());
+recordObjCProperties(ObjCInterfaceRecord, Decl->properties());
+recordObjCInstanceVariables(ObjCInterfaceRecord, Decl->ivars());
+
+return true;
+  }
+};
+} // namespace
+
 DEFINE_SIMPLE_CONVERSION_FUNCTIONS(APISet, CXAPISet)
 
-static void WalkupFromMostDerivedType(ExtractAPIVisitor , Decl *D);
+static void WalkupFromMostDerivedType(LibClangExtractAPIVisitor ,
+  Decl *D);
 
 template 
 static bool WalkupParentContext(DeclContext *Parent,
-ExtractAPIVisitor ) {
+LibClangExtractAPIVisitor ) {
   if (auto *D = dyn_cast(Parent)) {
 WalkupFromMostDerivedType(Visitor, D);
 return true;
@@ -48,7 +109,8 @@
   return false;
 }
 
-static void WalkupFromMostDerivedType(ExtractAPIVisitor , Decl *D) {
+static void WalkupFromMostDerivedType(LibClangExtractAPIVisitor ,
+  Decl *D) {
   switch (D->getKind()) {
 #define ABSTRACT_DECL(DECL)
 #define DECL(CLASS, BASE)  \
@@ -84,8 +146,7 @@
   auto Lang = Unit->getInputKind().getLanguage();
   APISet *API = new APISet(Ctx.getTargetInfo().getTriple(), Lang,
 

[PATCH] D147234: [clang][ExtractAPI] Reland ExtractAPI for libclang improvements

[Daniel Grumberg via Phabricator via cfe-commits]

dang added inline comments.



Comment at: clang/tools/c-index-test/c-index-test.c:4898
+
+  clang_disposeString(SGFData);
+}

The previous version didn't free the string here which was the source of the 
leak @hctim 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147234

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

[PATCH] D147234: [clang][ExtractAPI] Reland ExtractAPI for libclang improvements

[Daniel Grumberg via Phabricator via cfe-commits]

dang created this revision.
dang added reviewers: hctim, zixuw.
Herald added subscribers: ChuanqiXu, arphaman.
Herald added a reviewer: ributzka.
Herald added a project: All.
dang requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This relands the changes that were originally introduced by:

- https://reviews.llvm.org/D146656
- https://reviews.llvm.org/D147138

This also fixes the leak that led to these changes being reverted


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147234

Files:
  clang/include/clang/ExtractAPI/ExtractAPIVisitor.h
  clang/include/clang/ExtractAPI/TypedefUnderlyingTypeResolver.h
  clang/lib/ExtractAPI/CMakeLists.txt
  clang/lib/ExtractAPI/DeclarationFragments.cpp
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/lib/ExtractAPI/ExtractAPIVisitor.cpp
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
  clang/lib/ExtractAPI/TypedefUnderlyingTypeResolver.cpp
  clang/lib/ExtractAPI/TypedefUnderlyingTypeResolver.h
  clang/test/Index/extract-api-cursor.m
  clang/test/Index/extract-api-usr.m
  clang/tools/c-index-test/c-index-test.c
  clang/tools/libclang/CXExtractAPI.cpp

Index: clang/tools/libclang/CXExtractAPI.cpp
===
--- clang/tools/libclang/CXExtractAPI.cpp
+++ clang/tools/libclang/CXExtractAPI.cpp
@@ -18,6 +18,7 @@
 #include "clang-c/Index.h"
 #include "clang-c/Platform.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/DeclObjC.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/ExtractAPI/API.h"
 #include "clang/ExtractAPI/ExtractAPIVisitor.h"
@@ -34,13 +35,73 @@
 using namespace clang;
 using namespace clang::extractapi;
 
+namespace {
+struct LibClangExtractAPIVisitor
+: ExtractAPIVisitor {
+  using Base = ExtractAPIVisitor;
+
+  LibClangExtractAPIVisitor(ASTContext , APISet )
+  : ExtractAPIVisitor(Context, API) {}
+
+  const RawComment *fetchRawCommentForDecl(const Decl *D) const {
+return Context.getRawCommentForAnyRedecl(D);
+  }
+
+  // We need to visit implementations as well to ensure that when a user clicks
+  // on a method defined only within the implementation that we can still
+  // provide a symbol graph for it.
+  bool VisitObjCImplementationDecl(const ObjCImplementationDecl *Decl) {
+if (!shouldDeclBeIncluded(Decl))
+  return true;
+
+const ObjCInterfaceDecl *Interface = Decl->getClassInterface();
+StringRef Name = Interface->getName();
+StringRef USR = API.recordUSR(Decl);
+PresumedLoc Loc =
+Context.getSourceManager().getPresumedLoc(Decl->getLocation());
+LinkageInfo Linkage = Decl->getLinkageAndVisibility();
+DocComment Comment;
+if (auto *RawComment = fetchRawCommentForDecl(Interface))
+  Comment = RawComment->getFormattedLines(Context.getSourceManager(),
+  Context.getDiagnostics());
+
+// Build declaration fragments and sub-heading by generating them for the
+// interface.
+DeclarationFragments Declaration =
+DeclarationFragmentsBuilder::getFragmentsForObjCInterface(Interface);
+DeclarationFragments SubHeading =
+DeclarationFragmentsBuilder::getSubHeading(Decl);
+
+// Collect super class information.
+SymbolReference SuperClass;
+if (const auto *SuperClassDecl = Decl->getSuperClass()) {
+  SuperClass.Name = SuperClassDecl->getObjCRuntimeNameAsString();
+  SuperClass.USR = API.recordUSR(SuperClassDecl);
+}
+
+ObjCInterfaceRecord *ObjCInterfaceRecord = API.addObjCInterface(
+Name, USR, Loc, AvailabilitySet(Decl), Linkage, Comment, Declaration,
+SubHeading, SuperClass, isInSystemHeader(Decl));
+
+// Record all methods (selectors). This doesn't include automatically
+// synthesized property methods.
+recordObjCMethods(ObjCInterfaceRecord, Decl->methods());
+recordObjCProperties(ObjCInterfaceRecord, Decl->properties());
+recordObjCInstanceVariables(ObjCInterfaceRecord, Decl->ivars());
+
+return true;
+  }
+};
+} // namespace
+
 DEFINE_SIMPLE_CONVERSION_FUNCTIONS(APISet, CXAPISet)
 
-static void WalkupFromMostDerivedType(ExtractAPIVisitor , Decl *D);
+static void WalkupFromMostDerivedType(LibClangExtractAPIVisitor ,
+  Decl *D);
 
 template 
 static bool WalkupParentContext(DeclContext *Parent,
-ExtractAPIVisitor ) {
+LibClangExtractAPIVisitor ) {
   if (auto *D = dyn_cast(Parent)) {
 WalkupFromMostDerivedType(Visitor, D);
 return true;
@@ -48,7 +109,8 @@
   return false;
 }
 
-static void WalkupFromMostDerivedType(ExtractAPIVisitor , Decl *D) {
+static void WalkupFromMostDerivedType(LibClangExtractAPIVisitor ,
+  Decl *D) {
   switch (D->getKind()) {
 #define ABSTRACT_DECL(DECL)
 #define DECL(CLASS, BASE)  \
@@ 

[PATCH] D147138: [clang][ExtractAPI] Add queried symbol to parent contexts in libclang

[Daniel Grumberg 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 rG1cfe1e732ad8: [clang][ExtractAPI] Add queried symbol to 
parent contexts in libclang (authored by dang).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147138

Files:
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
  clang/test/Index/extract-api-cursor.m
  clang/test/Index/extract-api-usr.m

Index: clang/test/Index/extract-api-usr.m
===
--- clang/test/Index/extract-api-usr.m
+++ clang/test/Index/extract-api-usr.m
@@ -28,7 +28,7 @@
 
 // Checking for Foo
 // RUN: c-index-test "-single-symbol-sgf-for=c:@S@Foo" %s | FileCheck -check-prefix=CHECK-FOO %s
-// CHECK-FOO: "parentContexts":[]
+// CHECK-FOO: "parentContexts":[{"kind":"objective-c.struct","name":"Foo","usr":"c:@S@Foo"}]
 // CHECK-FOO-SAME: "relatedSymbols":[]
 // CHECK-FOO-SAME: "relationships":[]
 // CHECK-FOO-SAME: "text":"Foo docs"
@@ -38,7 +38,7 @@
 
 // Checking for bar
 // RUN: c-index-test "-single-symbol-sgf-for=c:@S@Foo@FI@bar" %s | FileCheck -check-prefix=CHECK-BAR %s
-// CHECK-BAR: "parentContexts":[{"kind":"objective-c.struct","name":"Foo","usr":"c:@S@Foo"}]
+// CHECK-BAR: "parentContexts":[{"kind":"objective-c.struct","name":"Foo","usr":"c:@S@Foo"},{"kind":"objective-c.property","name":"bar","usr":"c:@S@Foo@FI@bar"}]
 // CHECK-BAR-SAME: "relatedSymbols":[]
 // CHECK-BAR-SAME: "relationships":[{"kind":"memberOf","source":"c:@S@Foo@FI@bar","target":"c:@S@Foo"
 // CHECK-BAR-SAME: "text":"Bar docs"
@@ -47,7 +47,7 @@
 
 // Checking for Base
 // RUN: c-index-test "-single-symbol-sgf-for=c:objc(cs)Base" %s | FileCheck -check-prefix=CHECK-BASE %s
-// CHECK-BASE: "parentContexts":[]
+// CHECK-BASE: "parentContexts":[{"kind":"objective-c.class","name":"Base","usr":"c:objc(cs)Base"}]
 // CHECK-BASE-SAME: "relatedSymbols":[]
 // CHECK-BASE-SAME: "relationships":[]
 // CHECK-BASE-SAME: "text":"Base docs"
@@ -56,7 +56,7 @@
 
 // Checking for baseProperty
 // RUN: c-index-test "-single-symbol-sgf-for=c:objc(cs)Base(py)baseProperty" %s | FileCheck -check-prefix=CHECK-BASEPROP %s
-// CHECK-BASEPROP: "parentContexts":[{"kind":"objective-c.class","name":"Base","usr":"c:objc(cs)Base"}]
+// CHECK-BASEPROP: "parentContexts":[{"kind":"objective-c.class","name":"Base","usr":"c:objc(cs)Base"},{"kind":"objective-c.property","name":"baseProperty","usr":"c:objc(cs)Base(py)baseProperty"}]
 // CHECK-BASEPROP-SAME:"relatedSymbols":[{"accessLevel":"public","declarationLanguage":"objective-c"
 // CHECK-BASEPROP-SAME: "isSystem":false
 // CHECK-BASEPROP-SAME: "usr":"c:@S@Foo"}]
@@ -67,7 +67,7 @@
 
 // Checking for baseMethodWithArg
 // RUN: c-index-test "-single-symbol-sgf-for=c:objc(cs)Base(im)baseMethodWithArg:" %s | FileCheck -check-prefix=CHECK-BASEMETHOD %s
-// CHECK-BASEMETHOD: "parentContexts":[{"kind":"objective-c.class","name":"Base","usr":"c:objc(cs)Base"}]
+// CHECK-BASEMETHOD: "parentContexts":[{"kind":"objective-c.class","name":"Base","usr":"c:objc(cs)Base"},{"kind":"objective-c.method","name":"baseMethodWithArg:","usr":"c:objc(cs)Base(im)baseMethodWithArg:"}]
 // CHECK-BASEMETHOD-SAME:"relatedSymbols":[]
 // CHECK-BASEMETHOD-SAME: "relationships":[{"kind":"memberOf","source":"c:objc(cs)Base(im)baseMethodWithArg:","target":"c:objc(cs)Base"
 // CHECK-BASEMETHOD-SAME: "text":"Base method docs"
@@ -76,7 +76,7 @@
 
 // Checking for Protocol
 // RUN: c-index-test "-single-symbol-sgf-for=c:objc(pl)Protocol" %s | FileCheck -check-prefix=CHECK-PROT %s
-// CHECK-PROT: "parentContexts":[]
+// CHECK-PROT: "parentContexts":[{"kind":"objective-c.protocol","name":"Protocol","usr":"c:objc(pl)Protocol"}]
 // CHECK-PROT-SAME: "relatedSymbols":[]
 // CHECK-PROT-SAME: "relationships":[]
 // CHECK-PROT-SAME: "text":"Protocol docs"
@@ -85,7 +85,7 @@
 
 // Checking for protocolProperty
 // RUN: c-index-test "-single-symbol-sgf-for=c:objc(pl)Protocol(py)protocolProperty" %s | FileCheck -check-prefix=CHECK-PROTPROP %s
-// CHECK-PROTPROP: "parentContexts":[{"kind":"objective-c.protocol","name":"Protocol","usr":"c:objc(pl)Protocol"}]
+// CHECK-PROTPROP: "parentContexts":[{"kind":"objective-c.protocol","name":"Protocol","usr":"c:objc(pl)Protocol"},{"kind":"objective-c.property","name":"protocolProperty","usr":"c:objc(pl)Protocol(py)protocolProperty"}]
 // CHECK-PROTPROP-SAME:"relatedSymbols":[{"accessLevel":"public","declarationLanguage":"objective-c"
 // CHECK-PROTPROP-SAME: "isSystem":false
 // CHECK-PROTPROP-SAME: "usr":"c:@S@Foo"}]
@@ -96,7 +96,7 @@
 
 // Checking for Derived
 // RUN: c-index-test "-single-symbol-sgf-for=c:objc(cs)Derived" %s | FileCheck -check-prefix=CHECK-DERIVED %s
-// CHECK-DERIVED: "parentContexts":[]
+// CHECK-DERIVED: "parentContexts":[{"kind":"objective-c.class","name":"Derived","usr":"c:objc(cs)Derived"}]
 // 

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

[Daniel Grumberg via Phabricator via cfe-commits]

dang added inline comments.



Comment at: clang/lib/ExtractAPI/DeclarationFragments.cpp:176
+
+// id is an qualified id type
+if (!T->getAs()->isObjCQualifiedIdType()) {





Comment at: clang/lib/ExtractAPI/DeclarationFragments.cpp:176
+
+// id is an qualified id type
+if (!T->getAs()->isObjCQualifiedIdType()) {

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146866

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

[PATCH] D146385: [clang][ExtractAPI] Complete declaration fragments for TagDecl types defined in a typedef

[Daniel Grumberg via Phabricator via cfe-commits]

dang added a comment.

You will need to rebase this as I made some changes recently to how 
`ExtractAPIVisitor` is structured. We can either set up a time to talk about it 
and do it together or I can handle doing this work once we are happy with this.




Comment at: clang/lib/ExtractAPI/ExtractAPIVisitor.cpp:52
+template 
+void modifyRecord(const T , const llvm::StringRef ) {
+  for (const auto  : Records) {

this should be a marked `static` or put in an anonymous namespace.



Comment at: clang/lib/ExtractAPI/ExtractAPIVisitor.cpp:56
+  Record.second.get()->Declaration.removeLast();
+  Record.second.get()
+  ->Declaration

I think this warrants adding methods to declaration fragments to streamline 
this sort of operation (merging fragments at arbitrary offsets. This should 
probably be done as a follow up patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146385

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

[PATCH] D147138: [clang][ExtractAPI] Add queried symbol to parent contexts in libclang

[Daniel Grumberg via Phabricator via cfe-commits]

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

Ensure that the current symbol is added to the parent contexts in the
output of libclang function for generating symbol graphs for single symbols.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147138

Files:
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
  clang/test/Index/extract-api-cursor.m
  clang/test/Index/extract-api-usr.m

Index: clang/test/Index/extract-api-usr.m
===
--- clang/test/Index/extract-api-usr.m
+++ clang/test/Index/extract-api-usr.m
@@ -28,7 +28,7 @@
 
 // Checking for Foo
 // RUN: c-index-test "-single-symbol-sgf-for=c:@S@Foo" %s | FileCheck -check-prefix=CHECK-FOO %s
-// CHECK-FOO: "parentContexts":[]
+// CHECK-FOO: "parentContexts":[{"kind":"objective-c.struct","name":"Foo","usr":"c:@S@Foo"}]
 // CHECK-FOO-SAME: "relatedSymbols":[]
 // CHECK-FOO-SAME: "relationships":[]
 // CHECK-FOO-SAME: "text":"Foo docs"
@@ -38,7 +38,7 @@
 
 // Checking for bar
 // RUN: c-index-test "-single-symbol-sgf-for=c:@S@Foo@FI@bar" %s | FileCheck -check-prefix=CHECK-BAR %s
-// CHECK-BAR: "parentContexts":[{"kind":"objective-c.struct","name":"Foo","usr":"c:@S@Foo"}]
+// CHECK-BAR: "parentContexts":[{"kind":"objective-c.struct","name":"Foo","usr":"c:@S@Foo"},{"kind":"objective-c.property","name":"bar","usr":"c:@S@Foo@FI@bar"}]
 // CHECK-BAR-SAME: "relatedSymbols":[]
 // CHECK-BAR-SAME: "relationships":[{"kind":"memberOf","source":"c:@S@Foo@FI@bar","target":"c:@S@Foo"
 // CHECK-BAR-SAME: "text":"Bar docs"
@@ -47,7 +47,7 @@
 
 // Checking for Base
 // RUN: c-index-test "-single-symbol-sgf-for=c:objc(cs)Base" %s | FileCheck -check-prefix=CHECK-BASE %s
-// CHECK-BASE: "parentContexts":[]
+// CHECK-BASE: "parentContexts":[{"kind":"objective-c.class","name":"Base","usr":"c:objc(cs)Base"}]
 // CHECK-BASE-SAME: "relatedSymbols":[]
 // CHECK-BASE-SAME: "relationships":[]
 // CHECK-BASE-SAME: "text":"Base docs"
@@ -56,7 +56,7 @@
 
 // Checking for baseProperty
 // RUN: c-index-test "-single-symbol-sgf-for=c:objc(cs)Base(py)baseProperty" %s | FileCheck -check-prefix=CHECK-BASEPROP %s
-// CHECK-BASEPROP: "parentContexts":[{"kind":"objective-c.class","name":"Base","usr":"c:objc(cs)Base"}]
+// CHECK-BASEPROP: "parentContexts":[{"kind":"objective-c.class","name":"Base","usr":"c:objc(cs)Base"},{"kind":"objective-c.property","name":"baseProperty","usr":"c:objc(cs)Base(py)baseProperty"}]
 // CHECK-BASEPROP-SAME:"relatedSymbols":[{"accessLevel":"public","declarationLanguage":"objective-c"
 // CHECK-BASEPROP-SAME: "isSystem":false
 // CHECK-BASEPROP-SAME: "usr":"c:@S@Foo"}]
@@ -67,7 +67,7 @@
 
 // Checking for baseMethodWithArg
 // RUN: c-index-test "-single-symbol-sgf-for=c:objc(cs)Base(im)baseMethodWithArg:" %s | FileCheck -check-prefix=CHECK-BASEMETHOD %s
-// CHECK-BASEMETHOD: "parentContexts":[{"kind":"objective-c.class","name":"Base","usr":"c:objc(cs)Base"}]
+// CHECK-BASEMETHOD: "parentContexts":[{"kind":"objective-c.class","name":"Base","usr":"c:objc(cs)Base"},{"kind":"objective-c.method","name":"baseMethodWithArg:","usr":"c:objc(cs)Base(im)baseMethodWithArg:"}]
 // CHECK-BASEMETHOD-SAME:"relatedSymbols":[]
 // CHECK-BASEMETHOD-SAME: "relationships":[{"kind":"memberOf","source":"c:objc(cs)Base(im)baseMethodWithArg:","target":"c:objc(cs)Base"
 // CHECK-BASEMETHOD-SAME: "text":"Base method docs"
@@ -76,7 +76,7 @@
 
 // Checking for Protocol
 // RUN: c-index-test "-single-symbol-sgf-for=c:objc(pl)Protocol" %s | FileCheck -check-prefix=CHECK-PROT %s
-// CHECK-PROT: "parentContexts":[]
+// CHECK-PROT: "parentContexts":[{"kind":"objective-c.protocol","name":"Protocol","usr":"c:objc(pl)Protocol"}]
 // CHECK-PROT-SAME: "relatedSymbols":[]
 // CHECK-PROT-SAME: "relationships":[]
 // CHECK-PROT-SAME: "text":"Protocol docs"
@@ -85,7 +85,7 @@
 
 // Checking for protocolProperty
 // RUN: c-index-test "-single-symbol-sgf-for=c:objc(pl)Protocol(py)protocolProperty" %s | FileCheck -check-prefix=CHECK-PROTPROP %s
-// CHECK-PROTPROP: "parentContexts":[{"kind":"objective-c.protocol","name":"Protocol","usr":"c:objc(pl)Protocol"}]
+// CHECK-PROTPROP: "parentContexts":[{"kind":"objective-c.protocol","name":"Protocol","usr":"c:objc(pl)Protocol"},{"kind":"objective-c.property","name":"protocolProperty","usr":"c:objc(pl)Protocol(py)protocolProperty"}]
 // CHECK-PROTPROP-SAME:"relatedSymbols":[{"accessLevel":"public","declarationLanguage":"objective-c"
 // CHECK-PROTPROP-SAME: "isSystem":false
 // CHECK-PROTPROP-SAME: "usr":"c:@S@Foo"}]
@@ -96,7 +96,7 @@
 
 // Checking for Derived
 // RUN: c-index-test "-single-symbol-sgf-for=c:objc(cs)Derived" %s | FileCheck -check-prefix=CHECK-DERIVED %s
-// CHECK-DERIVED: "parentContexts":[]
+// CHECK-DERIVED: 

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

[Daniel Grumberg 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 rG21750a1ae8c8: [clang][ExtractAPI] Refactor ExtractAPIVisitor 
to make it more extensible (authored by dang).

Changed prior to commit:
  https://reviews.llvm.org/D146656?vs=507544=508696#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146656

Files:
  clang/test/Index/extract-api-cursor.m
  clang/tools/c-index-test/c-index-test.c

Index: clang/tools/c-index-test/c-index-test.c
===
--- clang/tools/c-index-test/c-index-test.c
+++ clang/tools/c-index-test/c-index-test.c
@@ -3,6 +3,7 @@
 #include "clang-c/BuildSystem.h"
 #include "clang-c/CXCompilationDatabase.h"
 #include "clang-c/CXErrorCode.h"
+#include "clang-c/CXSourceLocation.h"
 #include "clang-c/CXString.h"
 #include "clang-c/Documentation.h"
 #include "clang-c/Index.h"
@@ -4881,6 +4882,21 @@
   return result;
 }
 
+static void inspect_single_symbol_sgf_cursor(CXCursor Cursor) {
+  CXSourceLocation CursorLoc;
+  CXString SGFData;
+  const char *SGF;
+  unsigned line, column;
+  CursorLoc = clang_getCursorLocation(Cursor);
+  clang_getSpellingLocation(CursorLoc, 0, , , 0);
+  printf("%d:%d: ", line, column);
+
+  SGFData = clang_getSymbolGraphForCursor(Cursor);
+  SGF = clang_getCString(SGFData);
+  if (SGF)
+printf("%s\n", SGF);
+}
+
 /**/
 /* Command line processing.   */
 /**/
@@ -4940,6 +4956,7 @@
 "   c-index-test -print-usr-file \n");
   fprintf(stderr,
   "   c-index-test -single-symbol-sgfs  {*}\n"
+  "   c-index-test -single-symbol-sgf-at= {*}\n"
   "   c-index-test -single-symbol-sgf-for= {}*\n");
   fprintf(stderr,
 "   c-index-test -write-pch  \n"
@@ -5076,6 +5093,9 @@
   else if (argc > 3 && strcmp(argv[1], "-single-symbol-sgfs") == 0)
 return perform_test_load_source(argc - 3, argv + 3, argv[2],
 PrintSingleSymbolSGFs, NULL);
+  else if (argc > 2 && strstr(argv[1], "-single-symbol-sgf-at=") == argv[1])
+return inspect_cursor_at(
+argc, argv, "-single-symbol-sgf-at=", inspect_single_symbol_sgf_cursor);
   else if (argc > 2 && strstr(argv[1], "-single-symbol-sgf-for=") == argv[1])
 return perform_test_single_symbol_sgf(argv[1], argc - 2, argv + 2);
 
Index: clang/test/Index/extract-api-cursor.m
===
--- clang/test/Index/extract-api-cursor.m
+++ clang/test/Index/extract-api-cursor.m
@@ -1,3 +1,5 @@
+// Test is line- and column-sensitive. Run lines are below
+
 /// Foo docs
 struct Foo {
 /// Bar docs
@@ -25,91 +27,94 @@
 - (void)derivedMethodWithValue:(id)value;
 @end
 
-/// This won't show up in docs because we can't serialize it
-@interface Derived ()
-/// Derived method in category docs, won't show up either.
-- (void)derivedMethodInCategory;
+@implementation Derived
+- (void)derivedMethodWithValue:(id)value {
+int a = 5;
+}
 @end
 
-// RUN: c-index-test -single-symbol-sgfs local %s | FileCheck %s
-
-// Checking for Foo
-// CHECK: "parentContexts":[]
-// CHECK-SAME: "relatedSymbols":[]
-// CHECK-SAME: "relationships":[]
-// CHECK-SAME: "text":"Foo docs"
-// CHECK-SAME: "kind":{"displayName":"Structure","identifier":"objective-c.struct"}
-// CHECK-SAME: "title":"Foo"
-
-// Checking for bar
-// CHECK-NEXT: "parentContexts":[{"kind":"objective-c.struct","name":"Foo","usr":"c:@S@Foo"}]
-// CHECK-SAME: "relatedSymbols":[]
-// CHECK-SAME: "relationships":[{"kind":"memberOf","source":"c:@S@Foo@FI@bar","target":"c:@S@Foo"
-// CHECK-SAME: "text":"Bar docs"
-// CHECK-SAME: "kind":{"displayName":"Instance Property","identifier":"objective-c.property"}
-// CHECK-SAME: "title":"bar"
-
-// Checking for Base
-// CHECK-NEXT: "parentContexts":[]
-// CHECK-SAME: "relatedSymbols":[]
-// CHECK-SAME: "relationships":[]
-// CHECK-SAME: "text":"Base docs"
-// CHECK-SAME: "kind":{"displayName":"Class","identifier":"objective-c.class"}
-// CHECK-SAME: "title":"Base"
-
-// Checking for baseProperty
-// CHECK-NEXT: "parentContexts":[{"kind":"objective-c.class","name":"Base","usr":"c:objc(cs)Base"}]
-// CHECK-SAME: "relatedSymbols":[{"accessLevel":"public","declarationLanguage":"objective-c"
-// CHECK-SAME: "isSystem":false
-// CHECK-SAME: "usr":"c:@S@Foo"}]
-// CHECK-SAME: "relationships":[{"kind":"memberOf","source":"c:objc(cs)Base(py)baseProperty","target":"c:objc(cs)Base"
-// CHECK-SAME: "text":"Base property docs"
-// CHECK-SAME: "kind":{"displayName":"Instance Property","identifier":"objective-c.property"}
-// CHECK-SAME: "title":"baseProperty"
-
-// Checking for baseMethodWithArg
-// 

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

[Daniel Grumberg via Phabricator via cfe-commits]

dang added a comment.

In D146656#4220022 , @zixuw wrote:

> LGTM for the `ExtractAPIVisitor` part.
> Remaining:
>
> - update test with `@LINE`
> - the libclang side

I have decided against doing that, because we can't specify `@LINE` in the 
`c-index-test` command line.
@bnbarham are you happy with the libclang bits?




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

dang wrote:
> zixuw wrote:
> > 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.
> Not fully sure myself, but I think that using the one from 
> `RecursiveASTVisitor` would work fine here. I will give it a go and change it 
> if it works.
turns out it is needed but not sure why, I will rename to make things clearer.



Comment at: clang/test/Index/extract-api-cursor.m:36
 
-// RUN: c-index-test -single-symbol-sgfs local %s | FileCheck %s
-
-// Checking for Foo
-// CHECK: "parentContexts":[]
-// CHECK-SAME: "relatedSymbols":[]
-// CHECK-SAME: "relationships":[]
-// CHECK-SAME: "text":"Foo docs"
-// CHECK-SAME: 
"kind":{"displayName":"Structure","identifier":"objective-c.struct"}
-// CHECK-SAME: "title":"Foo"
-
-// Checking for bar
-// CHECK-NEXT: 
"parentContexts":[{"kind":"objective-c.struct","name":"Foo","usr":"c:@S@Foo"}]
-// CHECK-SAME: "relatedSymbols":[]
-// CHECK-SAME: 
"relationships":[{"kind":"memberOf","source":"c:@S@Foo@FI@bar","target":"c:@S@Foo"
-// CHECK-SAME: "text":"Bar docs"
-// CHECK-SAME: "kind":{"displayName":"Instance 
Property","identifier":"objective-c.property"}
-// CHECK-SAME: "title":"bar"
-
-// Checking for Base
-// CHECK-NEXT: "parentContexts":[]
-// CHECK-SAME: "relatedSymbols":[]
-// CHECK-SAME: "relationships":[]
-// CHECK-SAME: "text":"Base docs"
-// CHECK-SAME: "kind":{"displayName":"Class","identifier":"objective-c.class"}
-// CHECK-SAME: "title":"Base"
-
-// Checking for baseProperty
-// CHECK-NEXT: 
"parentContexts":[{"kind":"objective-c.class","name":"Base","usr":"c:objc(cs)Base"}]
-// CHECK-SAME: 
"relatedSymbols":[{"accessLevel":"public","declarationLanguage":"objective-c"
-// CHECK-SAME: "isSystem":false
-// CHECK-SAME: "usr":"c:@S@Foo"}]
-// CHECK-SAME: 
"relationships":[{"kind":"memberOf","source":"c:objc(cs)Base(py)baseProperty","target":"c:objc(cs)Base"
-// CHECK-SAME: "text":"Base property docs"
-// CHECK-SAME: "kind":{"displayName":"Instance 
Property","identifier":"objective-c.property"}
-// CHECK-SAME: "title":"baseProperty"
-
-// Checking for baseMethodWithArg
-// CHECK-NEXT: 
"parentContexts":[{"kind":"objective-c.class","name":"Base","usr":"c:objc(cs)Base"}]
-// CHECK-SAME: "relatedSymbols":[]
-// CHECK-SAME: 
"relationships":[{"kind":"memberOf","source":"c:objc(cs)Base(im)baseMethodWithArg:","target":"c:objc(cs)Base"
-// CHECK-SAME: "text":"Base method docs"
-// CHECK-SAME: "kind":{"displayName":"Instance 
Method","identifier":"objective-c.method"}
-// CHECK-SAME: "title":"baseMethodWithArg:"
-
-// Checking for Protocol
-// CHECK-NEXT: "parentContexts":[]
-// CHECK-SAME: "relatedSymbols":[]
-// CHECK-SAME: "relationships":[]
-// CHECK-SAME: "text":"Protocol docs"
-// CHECK-SAME: 
"kind":{"displayName":"Protocol","identifier":"objective-c.protocol"}
-// CHECK-SAME: "title":"Protocol"
-
-// Checking for protocolProperty
-// CHECK-NEXT: 
"parentContexts":[{"kind":"objective-c.protocol","name":"Protocol","usr":"c:objc(pl)Protocol"}]
-// CHECK-SAME: 
"relatedSymbols":[{"accessLevel":"public","declarationLanguage":"objective-c"
-// CHECK-SAME: "isSystem":false
-// CHECK-SAME: "usr":"c:@S@Foo"}]
-// CHECK-SAME: 
"relationships":[{"kind":"memberOf","source":"c:objc(pl)Protocol(py)protocolProperty","target":"c:objc(pl)Protocol"
-// CHECK-SAME: "text":"Protocol property docs"
-// CHECK-SAME: "kind":{"displayName":"Instance 
Property","identifier":"objective-c.property"}
-// CHECK-SAME: "title":"protocolProperty"
-
-// Checking for Derived
-// CHECK-NEXT: "parentContexts":[]
-// CHECK-SAME: 
"relatedSymbols":[{"accessLevel":"public","declarationLanguage":"objective-c"
-// CHECK-SAME: "isSystem":false
-// CHECK-SAME: "usr":"c:objc(cs)Base"}]
-// CHECK-SAME: 
"relationships":[{"kind":"inheritsFrom","source":"c:objc(cs)Derived","target":"c:objc(cs)Base"
-// CHECK-SAME: "text":"Derived docs"
-// CHECK-SAME: "kind":{"displayName":"Class","identifier":"objective-c.class"}
-// CHECK-SAME: "title":"Derived"
-
-// Checking for derivedMethodWithValue
-// CHECK-NEXT: 

[PATCH] D146385: [clang][ExtractAPI] Complete declaration fragments for TagDecl types defined in a typedef

[Daniel Grumberg via Phabricator via cfe-commits]

dang added inline comments.



Comment at: clang/lib/ExtractAPI/ExtractAPIVisitor.cpp:342
+  // Add the notion of typedef for tag type (struct or enum) of the same name.
+  if (const ElaboratedType *ET =
+  dyn_cast(Decl->getUnderlyingType())) {

This doesn't quite produce the output we want, this would generate `typedef 
struct Foo;` instead of the expected `typedef struct Foo { ... } Bar;` where 
Foo is the name of the struct and Bar is the name of the typedef. This should 
be easy enough to fix.



Comment at: clang/lib/ExtractAPI/ExtractAPIVisitor.cpp:347
+if (TagTy->getDecl()->isStruct()) {
+  for (const auto  : API.getStructs()) {
+if (Decl->getName() == Struct.second.get()->Name) {

What happens if the visitation hasn't already encountered the definition of the 
struct? We wouldn't find it in the the APISet and this doesn't work. The 
approach I would suggest here is to generate the fragments for the underlying 
Decl directly. For example what happens for the following code:

```c
struct Foo;
typedef struct Foo TypedefedFoo;
struct Foo {
int bar;
};
```



Comment at: clang/lib/ExtractAPI/ExtractAPIVisitor.cpp:349-354
+  Struct.second.get()
+  ->Declaration
+  .appendFront(" ", DeclarationFragments::FragmentKind::Text)
+  .appendFront("typedef",
+   DeclarationFragments::FragmentKind::Keyword, "",
+   nullptr);

This logic is very similar to the one below we could use the same code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146385

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

[PATCH] D146671: [clang][ExtractAPI]Fix Declaration fragments for instancetype in the type position degrade to id

[Daniel Grumberg via Phabricator via cfe-commits]

dang accepted this revision.
dang added a comment.

Yup looks fine to me!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146671

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

[PATCH] D146671: [clang][ExtractAPI]Fix Declaration fragments for instancetype in the type position degrade to id

[Daniel Grumberg via Phabricator via cfe-commits]

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

LGTM, but you should  also check in the test that `id` still renders as 
expected.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146671

___
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

[Daniel Grumberg via Phabricator via cfe-commits]

dang updated this revision to Diff 507544.
dang added a comment.

Adding back missing diffs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146656

Files:
  clang/include/clang/ExtractAPI/ExtractAPIVisitor.h
  clang/include/clang/ExtractAPI/TypedefUnderlyingTypeResolver.h
  clang/lib/ExtractAPI/CMakeLists.txt
  clang/lib/ExtractAPI/DeclarationFragments.cpp
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/lib/ExtractAPI/ExtractAPIVisitor.cpp
  clang/lib/ExtractAPI/TypedefUnderlyingTypeResolver.cpp
  clang/lib/ExtractAPI/TypedefUnderlyingTypeResolver.h
  clang/test/Index/extract-api-cursor.m
  clang/tools/c-index-test/c-index-test.c
  clang/tools/libclang/CXExtractAPI.cpp

Index: clang/tools/libclang/CXExtractAPI.cpp
===
--- clang/tools/libclang/CXExtractAPI.cpp
+++ clang/tools/libclang/CXExtractAPI.cpp
@@ -18,6 +18,7 @@
 #include "clang-c/Index.h"
 #include "clang-c/Platform.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/DeclObjC.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/ExtractAPI/API.h"
 #include "clang/ExtractAPI/ExtractAPIVisitor.h"
@@ -34,13 +35,73 @@
 using namespace clang;
 using namespace clang::extractapi;
 
+namespace {
+struct LibClangExtractAPIVisitor
+: ExtractAPIVisitor {
+  using Base = ExtractAPIVisitor;
+
+  LibClangExtractAPIVisitor(ASTContext , APISet )
+  : ExtractAPIVisitor(Context, API) {}
+
+  const RawComment *fetchRawCommentForDecl(const Decl *D) const {
+return Context.getRawCommentForAnyRedecl(D);
+  }
+
+  // We need to visit implementations as well to ensure that when a user clicks
+  // on a method defined only within the implementation that we can still
+  // provide a symbol graph for it.
+  bool VisitObjCImplementationDecl(const ObjCImplementationDecl *Decl) {
+if (!shouldDeclBeIncluded(Decl))
+  return true;
+
+const ObjCInterfaceDecl *Interface = Decl->getClassInterface();
+StringRef Name = Interface->getName();
+StringRef USR = API.recordUSR(Decl);
+PresumedLoc Loc =
+Context.getSourceManager().getPresumedLoc(Decl->getLocation());
+LinkageInfo Linkage = Decl->getLinkageAndVisibility();
+DocComment Comment;
+if (auto *RawComment = fetchRawCommentForDecl(Interface))
+  Comment = RawComment->getFormattedLines(Context.getSourceManager(),
+  Context.getDiagnostics());
+
+// Build declaration fragments and sub-heading by generating them for the
+// interface.
+DeclarationFragments Declaration =
+DeclarationFragmentsBuilder::getFragmentsForObjCInterface(Interface);
+DeclarationFragments SubHeading =
+DeclarationFragmentsBuilder::getSubHeading(Decl);
+
+// Collect super class information.
+SymbolReference SuperClass;
+if (const auto *SuperClassDecl = Decl->getSuperClass()) {
+  SuperClass.Name = SuperClassDecl->getObjCRuntimeNameAsString();
+  SuperClass.USR = API.recordUSR(SuperClassDecl);
+}
+
+ObjCInterfaceRecord *ObjCInterfaceRecord = API.addObjCInterface(
+Name, USR, Loc, AvailabilitySet(Decl), Linkage, Comment, Declaration,
+SubHeading, SuperClass, isInSystemHeader(Decl));
+
+// Record all methods (selectors). This doesn't include automatically
+// synthesized property methods.
+recordObjCMethods(ObjCInterfaceRecord, Decl->methods());
+recordObjCProperties(ObjCInterfaceRecord, Decl->properties());
+recordObjCInstanceVariables(ObjCInterfaceRecord, Decl->ivars());
+
+return true;
+  }
+};
+} // namespace
+
 DEFINE_SIMPLE_CONVERSION_FUNCTIONS(APISet, CXAPISet)
 
-static void WalkupFromMostDerivedType(ExtractAPIVisitor , Decl *D);
+static void WalkupFromMostDerivedType(LibClangExtractAPIVisitor ,
+  Decl *D);
 
 template 
 static bool WalkupParentContext(DeclContext *Parent,
-ExtractAPIVisitor ) {
+LibClangExtractAPIVisitor ) {
   if (auto *D = dyn_cast(Parent)) {
 WalkupFromMostDerivedType(Visitor, D);
 return true;
@@ -48,7 +109,8 @@
   return false;
 }
 
-static void WalkupFromMostDerivedType(ExtractAPIVisitor , Decl *D) {
+static void WalkupFromMostDerivedType(LibClangExtractAPIVisitor ,
+  Decl *D) {
   switch (D->getKind()) {
 #define ABSTRACT_DECL(DECL)
 #define DECL(CLASS, BASE)  \
@@ -84,8 +146,7 @@
   auto Lang = Unit->getInputKind().getLanguage();
   APISet *API = new APISet(Ctx.getTargetInfo().getTriple(), Lang,
Unit->getMainFileName().str());
-  ExtractAPIVisitor Visitor(
-  Ctx, [](SourceLocation Loc) { return true; }, *API);
+  LibClangExtractAPIVisitor Visitor(Ctx, *API);
 
   for (auto It = Unit->top_level_begin(); It != Unit->top_level_end(); ++It) {
 

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

[Daniel Grumberg via Phabricator via cfe-commits]

dang updated this revision to Diff 507543.
dang added a comment.

Addressing code review feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146656

Files:
  clang/include/clang/ExtractAPI/ExtractAPIVisitor.h
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp

Index: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
===
--- clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
+++ clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
@@ -167,7 +167,7 @@
 
 struct LocationFileChecker {
   bool operator()(SourceLocation Loc) {
-// If the loc refersSourceLocationxpansion we need to first get the file
+// If the loc refers to a macro expansion we need to first get the file
 // location of the expansion.
 auto  = CI.getSourceManager();
 auto FileLoc = SM.getFileLoc(Loc);
Index: clang/include/clang/ExtractAPI/ExtractAPIVisitor.h
===
--- clang/include/clang/ExtractAPI/ExtractAPIVisitor.h
+++ clang/include/clang/ExtractAPI/ExtractAPIVisitor.h
@@ -30,10 +30,11 @@
 
 template 
 class ExtractAPIVisitorBase : public RecursiveASTVisitor {
-public:
+protected:
   ExtractAPIVisitorBase(ASTContext , APISet )
   : Context(Context), API(API) {}
 
+public:
   const APISet () const { return API; }
 
   bool VisitVarDecl(const VarDecl *Decl);
@@ -99,7 +100,9 @@
   }
 
 private:
-  Derived () { return *static_cast(this); }
+  Derived () {
+return *static_cast(this);
+  }
 };
 
 template 
@@ -121,7 +124,7 @@
   Decl->getTemplateSpecializationKind() == TSK_Undeclared)
 return true;
 
-  if (!getConcrete().shouldDeclBeIncluded(Decl))
+  if (!getDerivedExtractAPIVisitor().shouldDeclBeIncluded(Decl))
 return true;
 
   // Collect symbol information.
@@ -131,7 +134,8 @@
   Context.getSourceManager().getPresumedLoc(Decl->getLocation());
   LinkageInfo Linkage = Decl->getLinkageAndVisibility();
   DocComment Comment;
-  if (auto *RawComment = getConcrete().fetchRawCommentForDecl(Decl))
+  if (auto *RawComment =
+  getDerivedExtractAPIVisitor().fetchRawCommentForDecl(Decl))
 Comment = RawComment->getFormattedLines(Context.getSourceManager(),
 Context.getDiagnostics());
 
@@ -183,7 +187,7 @@
 return true;
   }
 
-  if (!getConcrete().shouldDeclBeIncluded(Decl))
+  if (!getDerivedExtractAPIVisitor().shouldDeclBeIncluded(Decl))
 return true;
 
   // Collect symbol information.
@@ -193,7 +197,8 @@
   Context.getSourceManager().getPresumedLoc(Decl->getLocation());
   LinkageInfo Linkage = Decl->getLinkageAndVisibility();
   DocComment Comment;
-  if (auto *RawComment = getConcrete().fetchRawCommentForDecl(Decl))
+  if (auto *RawComment =
+  getDerivedExtractAPIVisitor().fetchRawCommentForDecl(Decl))
 Comment = RawComment->getFormattedLines(Context.getSourceManager(),
 Context.getDiagnostics());
 
@@ -214,7 +219,7 @@
 
 template 
 bool ExtractAPIVisitorBase::VisitEnumDecl(const EnumDecl *Decl) {
-  if (!getConcrete().shouldDeclBeIncluded(Decl))
+  if (!getDerivedExtractAPIVisitor().shouldDeclBeIncluded(Decl))
 return true;
 
   SmallString<128> QualifiedNameBuffer;
@@ -232,7 +237,8 @@
   PresumedLoc Loc =
   Context.getSourceManager().getPresumedLoc(Decl->getLocation());
   DocComment Comment;
-  if (auto *RawComment = getConcrete().fetchRawCommentForDecl(Decl))
+  if (auto *RawComment =
+  getDerivedExtractAPIVisitor().fetchRawCommentForDecl(Decl))
 Comment = RawComment->getFormattedLines(Context.getSourceManager(),
 Context.getDiagnostics());
 
@@ -247,7 +253,8 @@
   Comment, Declaration, SubHeading, isInSystemHeader(Decl));
 
   // Now collect information about the enumerators in this enum.
-  getConcrete().recordEnumConstants(EnumRecord, Decl->enumerators());
+  getDerivedExtractAPIVisitor().recordEnumConstants(EnumRecord,
+Decl->enumerators());
 
   return true;
 }
@@ -259,7 +266,7 @@
   if (isa(Decl))
 return true;
 
-  if (!getConcrete().shouldDeclBeIncluded(Decl))
+  if (!getDerivedExtractAPIVisitor().shouldDeclBeIncluded(Decl))
 return true;
 
   // Collect symbol information.
@@ -273,7 +280,8 @@
   PresumedLoc Loc =
   Context.getSourceManager().getPresumedLoc(Decl->getLocation());
   DocComment Comment;
-  if (auto *RawComment = getConcrete().fetchRawCommentForDecl(Decl))
+  if (auto *RawComment =
+  getDerivedExtractAPIVisitor().fetchRawCommentForDecl(Decl))
 Comment = RawComment->getFormattedLines(Context.getSourceManager(),
 Context.getDiagnostics());
 
@@ -288,7 +296,8 @@
 SubHeading, isInSystemHeader(Decl));
 
   // Now collect information about the fields in 

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

[Daniel Grumberg via Phabricator via cfe-commits]

dang added inline comments.



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

zixuw wrote:
> 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?
I chose to name it "Base" as this seems to be the convention used for ADT, 
where they use the "Base" suffix for types clients shouldn't be using.

As with the base class, we need to provide `RecursiveASTVisitor` the most 
derived class so that when it does the `getDerived()` dance it can use the most 
derived methods.



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) {}

zixuw wrote:
> Should the constructor be made `protected` for a CRTP base?
agreed



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

zixuw wrote:
> Why does comment-fetching need to be dispatched?
This is for the libclang use case. If you request symbol graph data for a 
symbol in an implementation block for example you should get back the doc 
comment from the header. The main reason for doing this refactoring is so that 
we can avoid dynamic dispatch when doing source location lookups and now 
fetching documentation comments.



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

zixuw wrote:
> 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.
Not fully sure myself, but I think that using the one from 
`RecursiveASTVisitor` would work fine here. I will give it a go and change it 
if it works.



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

zixuw wrote:
> Same as above, should the constructor be `protected`? I guess it depends if 
> we want people to just instantiate and use a `ExtractAPIVisitor`.
The aim was for clients to be able to instantiate this one, hence why we go 
through a fair amount of effort to provide base class the most derived type 
with the `std::conditional_t` usage above.



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.

zixuw wrote:
> bnbarham wrote:
> > Accidental?
> Missed change?
yup definitely accidental will fix.



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();

zixuw wrote:
> 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.
I moved this behavior out of the base type into here, we used to do these 
checks in the individual `VisitXXX` methods and bail early, but I needed this 
behavior to be configurable for the libclang work. I figured it semantically 
belonged here.



Comment at: clang/test/Index/extract-api-cursor.m:36
 
-// RUN: c-index-test -single-symbol-sgfs local %s | FileCheck %s
-
-// Checking for Foo
-// CHECK: "parentContexts":[]
-// CHECK-SAME: "relatedSymbols":[]
-// CHECK-SAME: "relationships":[]
-// CHECK-SAME: "text":"Foo docs"
-// CHECK-SAME: 
"kind":{"displayName":"Structure","identifier":"objective-c.struct"}
-// 

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

[Daniel Grumberg via Phabricator via cfe-commits]

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

Use CRTP to enable creating statically dispatched subclasses of
ExtractAPIVisitor.
This enables adding extension points and customising the behavior more
easily.

This is used in CXExtractAPI.cpp to create a specialized visitor for
Libclang as well as streamlining the batch implementation in 
ExtractAPIConsumer.cpp

[clang][ExtractAPI] Improve tests for clang_getSymbolGraphForCursor

Adds a new mode to c-index-test that can fetch a single symbol symbol
graph for a given source location. This way we can be more precise when
writing tests for clang_getSymbolGraphForCursor.
Additionaly this makes it easier to debug the function.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146656

Files:
  clang/include/clang/ExtractAPI/ExtractAPIVisitor.h
  clang/include/clang/ExtractAPI/TypedefUnderlyingTypeResolver.h
  clang/lib/ExtractAPI/CMakeLists.txt
  clang/lib/ExtractAPI/DeclarationFragments.cpp
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/lib/ExtractAPI/ExtractAPIVisitor.cpp
  clang/lib/ExtractAPI/TypedefUnderlyingTypeResolver.cpp
  clang/lib/ExtractAPI/TypedefUnderlyingTypeResolver.h
  clang/test/Index/extract-api-cursor.m
  clang/tools/c-index-test/c-index-test.c
  clang/tools/libclang/CXExtractAPI.cpp

Index: clang/tools/libclang/CXExtractAPI.cpp
===
--- clang/tools/libclang/CXExtractAPI.cpp
+++ clang/tools/libclang/CXExtractAPI.cpp
@@ -18,6 +18,7 @@
 #include "clang-c/Index.h"
 #include "clang-c/Platform.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/DeclObjC.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/ExtractAPI/API.h"
 #include "clang/ExtractAPI/ExtractAPIVisitor.h"
@@ -34,13 +35,73 @@
 using namespace clang;
 using namespace clang::extractapi;
 
+namespace {
+struct LibClangExtractAPIVisitor
+: ExtractAPIVisitor {
+  using Base = ExtractAPIVisitor;
+
+  LibClangExtractAPIVisitor(ASTContext , APISet )
+  : ExtractAPIVisitor(Context, API) {}
+
+  const RawComment *fetchRawCommentForDecl(const Decl *D) const {
+return Context.getRawCommentForAnyRedecl(D);
+  }
+
+  // We need to visit implementations as well to ensure that when a user clicks
+  // on a method defined only within the implementation that we can still
+  // provide a symbol graph for it.
+  bool VisitObjCImplementationDecl(const ObjCImplementationDecl *Decl) {
+if (!shouldDeclBeIncluded(Decl))
+  return true;
+
+const ObjCInterfaceDecl *Interface = Decl->getClassInterface();
+StringRef Name = Interface->getName();
+StringRef USR = API.recordUSR(Decl);
+PresumedLoc Loc =
+Context.getSourceManager().getPresumedLoc(Decl->getLocation());
+LinkageInfo Linkage = Decl->getLinkageAndVisibility();
+DocComment Comment;
+if (auto *RawComment = fetchRawCommentForDecl(Interface))
+  Comment = RawComment->getFormattedLines(Context.getSourceManager(),
+  Context.getDiagnostics());
+
+// Build declaration fragments and sub-heading by generating them for the
+// interface.
+DeclarationFragments Declaration =
+DeclarationFragmentsBuilder::getFragmentsForObjCInterface(Interface);
+DeclarationFragments SubHeading =
+DeclarationFragmentsBuilder::getSubHeading(Decl);
+
+// Collect super class information.
+SymbolReference SuperClass;
+if (const auto *SuperClassDecl = Decl->getSuperClass()) {
+  SuperClass.Name = SuperClassDecl->getObjCRuntimeNameAsString();
+  SuperClass.USR = API.recordUSR(SuperClassDecl);
+}
+
+ObjCInterfaceRecord *ObjCInterfaceRecord = API.addObjCInterface(
+Name, USR, Loc, AvailabilitySet(Decl), Linkage, Comment, Declaration,
+SubHeading, SuperClass, isInSystemHeader(Decl));
+
+// Record all methods (selectors). This doesn't include automatically
+// synthesized property methods.
+recordObjCMethods(ObjCInterfaceRecord, Decl->methods());
+recordObjCProperties(ObjCInterfaceRecord, Decl->properties());
+recordObjCInstanceVariables(ObjCInterfaceRecord, Decl->ivars());
+
+return true;
+  }
+};
+} // namespace
+
 DEFINE_SIMPLE_CONVERSION_FUNCTIONS(APISet, CXAPISet)
 
-static void WalkupFromMostDerivedType(ExtractAPIVisitor , Decl *D);
+static void WalkupFromMostDerivedType(LibClangExtractAPIVisitor ,
+  Decl *D);
 
 template 
 static bool WalkupParentContext(DeclContext *Parent,
-ExtractAPIVisitor ) {
+LibClangExtractAPIVisitor ) {
   if (auto *D = dyn_cast(Parent)) {
 WalkupFromMostDerivedType(Visitor, D);
 return true;
@@ -48,7 +109,8 @@
   return false;
 }
 
-static void 

[PATCH] D146354: [clang][ExtractAPI] Add semicolons for enum, typedef, struct declaration fragments

[Daniel Grumberg 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 rGafce10c5b60f: [clang][ExtractAPI] Add semicolons for enum, 
typedef, struct declaration… (authored by chaitanyav, committed by dang).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146354

Files:
  clang/lib/ExtractAPI/DeclarationFragments.cpp
  clang/test/ExtractAPI/anonymous_record_no_typedef.c
  clang/test/ExtractAPI/enum.c
  clang/test/ExtractAPI/struct.c
  clang/test/ExtractAPI/typedef.c
  clang/test/ExtractAPI/typedef_anonymous_record.c
  clang/test/ExtractAPI/typedef_chain.c
  clang/test/ExtractAPI/underscored.c

Index: clang/test/ExtractAPI/underscored.c
===
--- clang/test/ExtractAPI/underscored.c
+++ clang/test/ExtractAPI/underscored.c
@@ -135,6 +135,10 @@
 {
   "kind": "identifier",
   "spelling": "ExposedRecord"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -296,6 +300,10 @@
 {
   "kind": "identifier",
   "spelling": "ExposedTypedef"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -356,6 +364,10 @@
 {
   "kind": "identifier",
   "spelling": "ExposedTypedefToHidden"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
Index: clang/test/ExtractAPI/typedef_chain.c
===
--- clang/test/ExtractAPI/typedef_chain.c
+++ clang/test/ExtractAPI/typedef_chain.c
@@ -68,6 +68,10 @@
 {
   "kind": "identifier",
   "spelling": "MyInt"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -128,6 +132,10 @@
 {
   "kind": "identifier",
   "spelling": "MyIntInt"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -188,6 +196,10 @@
 {
   "kind": "identifier",
   "spelling": "MyIntIntInt"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
Index: clang/test/ExtractAPI/typedef_anonymous_record.c
===
--- clang/test/ExtractAPI/typedef_anonymous_record.c
+++ clang/test/ExtractAPI/typedef_anonymous_record.c
@@ -75,6 +75,10 @@
 {
   "kind": "identifier",
   "spelling": "MyEnum"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -170,6 +174,10 @@
 {
   "kind": "identifier",
   "spelling": "MyStruct"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -223,6 +231,10 @@
 {
   "kind": "identifier",
   "spelling": "MyStructStruct"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -283,6 +295,10 @@
 {
   "kind": "identifier",
   "spelling": "MyStructStructStruct"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -343,6 +359,10 @@
 {
   "kind": "identifier",
   "spelling": "MyEnumEnum"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
@@ -403,6 +423,10 @@
 {
   "kind": "identifier",
   "spelling": "MyEnumEnumEnum"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
Index: clang/test/ExtractAPI/typedef.c
===
--- clang/test/ExtractAPI/typedef.c
+++ clang/test/ExtractAPI/typedef.c
@@ -66,6 +66,10 @@
 {
   "kind": "identifier",
   "spelling": "MyInt"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "identifier": {
Index: clang/test/ExtractAPI/struct.c
===
--- clang/test/ExtractAPI/struct.c
+++ clang/test/ExtractAPI/struct.c
@@ -89,6 +89,10 @@
 {
   "kind": "identifier",
   "spelling": "Color"
+},
+{
+  "kind": "text",
+  "spelling": ";"
 }
   ],
   "docComment": {
Index: clang/test/ExtractAPI/enum.c
===
--- clang/test/ExtractAPI/enum.c
+++ 

[PATCH] D146354: [clang][ExtractAPI] Add semicolons for enum, typedef, struct declaration fragments

[Daniel Grumberg via Phabricator via cfe-commits]

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

LGTM! I think it's fine to go ahead and land this (premerge check are not a 
requirement). Have you contributed to LLVM before? If not I will need to commit 
it on your behalf. Once that is done you should follow the instructions at 
https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access to gain 
commit access if you wish to continue contributing to the project and LLVM in 
general.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146354

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

[PATCH] D146385: [clang][ExtractAPI] Complete declaration fragments for TagDecl types defined in a typedef

[Daniel Grumberg via Phabricator via cfe-commits]

dang requested changes to this revision.
dang added a comment.
This revision now requires changes to proceed.

I think there might be some code missing here. Also can you add a test?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146385

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

[PATCH] D145869: [clang][ExtractAPI] Add multiple file support to --extract-api-ignores

[Daniel Grumberg via Phabricator via cfe-commits]

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

LGTM once you fix the test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145869

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

[PATCH] D145869: [clang][ExtractAPI] Add multiple file support to --extract-api-ignores

[Daniel Grumberg via Phabricator via cfe-commits]

dang added inline comments.



Comment at: clang/test/ExtractAPI/ignored-symbols-multifile.c:27
+
+// CHECK-NOT: IGNORED_1_FILE3
+// CHECK-NOT: IGNORED_7_FILE3

Should this not be a 6?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145869

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

[PATCH] D145869: [clang][ExtractAPI] Add multiple file support to --extract-api-ignores

[Daniel Grumberg via Phabricator via cfe-commits]

dang added inline comments.



Comment at: clang/test/ExtractAPI/ignored-symbols-multifile.c:34
+//--- ignores-list1
+IGNORED_FILE1_1
+IGNORED_FILE1_2

This test doesn't check the symbol name sorting behavior across multiple files, 
which is crucial `APIIgnoresList::shouldIgnore(StringRef SymbolName)` because 
it uses binary search to find the symbol in the list. Would you be able to 
shuffle the order of the macros so that they aren't pre-sorted?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145869

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

[PATCH] D144940: [clang][ExtractAPI] Handle platform specific unavailability correctly

[Daniel Grumberg 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 rG65f7a84cf38b: [clang][ExtractAPI] Handle platform specific 
unavailability correctly (authored by Arsenic, committed by dang).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144940

Files:
  clang/include/clang/ExtractAPI/AvailabilityInfo.h
  clang/lib/ExtractAPI/AvailabilityInfo.cpp
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
  clang/test/ExtractAPI/availability.c


Index: clang/test/ExtractAPI/availability.c
===
--- clang/test/ExtractAPI/availability.c
+++ clang/test/ExtractAPI/availability.c
@@ -26,6 +26,9 @@
 void f(void) __attribute__((unavailable)) __attribute__((availability(macos, 
introduced=11.0)));
 
 void d(void) __attribute__((availability(tvos, introduced=15.0)));
+
+void e(void) __attribute__((availability(tvos, unavailable)));
+
 ///expected-no-diagnostics
 
 //--- reference.output.json.in
@@ -391,6 +394,10 @@
 "minor": 0,
 "patch": 0
   }
+},
+{
+  "domain": "tvos",
+  "isUnconditionallyUnavailable": true
 }
   ],
   "declarationFragments": [
Index: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
===
--- clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
+++ clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
@@ -171,12 +171,16 @@
   for (const auto  : Availabilities) {
 Object Availability;
 Availability["domain"] = AvailInfo.Domain;
-serializeObject(Availability, "introducedVersion",
-serializeSemanticVersion(AvailInfo.Introduced));
-serializeObject(Availability, "deprecatedVersion",
-serializeSemanticVersion(AvailInfo.Deprecated));
-serializeObject(Availability, "obsoletedVersion",
-serializeSemanticVersion(AvailInfo.Obsoleted));
+if (AvailInfo.Unavailable)
+  Availability["isUnconditionallyUnavailable"] = true;
+else {
+  serializeObject(Availability, "introducedVersion",
+  serializeSemanticVersion(AvailInfo.Introduced));
+  serializeObject(Availability, "deprecatedVersion",
+  serializeSemanticVersion(AvailInfo.Deprecated));
+  serializeObject(Availability, "obsoletedVersion",
+  serializeSemanticVersion(AvailInfo.Obsoleted));
+}
 AvailabilityArray.emplace_back(std::move(Availability));
   }
 
Index: clang/lib/ExtractAPI/AvailabilityInfo.cpp
===
--- clang/lib/ExtractAPI/AvailabilityInfo.cpp
+++ clang/lib/ExtractAPI/AvailabilityInfo.cpp
@@ -42,8 +42,8 @@
   Availability->Obsoleted = Attr->getObsoleted();
   } else {
 Availabilities.emplace_back(Domain, Attr->getIntroduced(),
-Attr->getDeprecated(),
-Attr->getObsoleted());
+Attr->getDeprecated(), 
Attr->getObsoleted(),
+Attr->getUnavailable());
   }
 }
   }
Index: clang/include/clang/ExtractAPI/AvailabilityInfo.h
===
--- clang/include/clang/ExtractAPI/AvailabilityInfo.h
+++ clang/include/clang/ExtractAPI/AvailabilityInfo.h
@@ -33,12 +33,14 @@
   VersionTuple Introduced;
   VersionTuple Deprecated;
   VersionTuple Obsoleted;
+  bool Unavailable;
 
   AvailabilityInfo() = default;
 
   AvailabilityInfo(StringRef Domain, VersionTuple I, VersionTuple D,
-   VersionTuple O)
-  : Domain(Domain), Introduced(I), Deprecated(D), Obsoleted(O) {}
+   VersionTuple O, bool U)
+  : Domain(Domain), Introduced(I), Deprecated(D), Obsoleted(O),
+Unavailable(U) {}
 };
 
 class AvailabilitySet {


Index: clang/test/ExtractAPI/availability.c
===
--- clang/test/ExtractAPI/availability.c
+++ clang/test/ExtractAPI/availability.c
@@ -26,6 +26,9 @@
 void f(void) __attribute__((unavailable)) __attribute__((availability(macos, introduced=11.0)));
 
 void d(void) __attribute__((availability(tvos, introduced=15.0)));
+
+void e(void) __attribute__((availability(tvos, unavailable)));
+
 ///expected-no-diagnostics
 
 //--- reference.output.json.in
@@ -391,6 +394,10 @@
 "minor": 0,
 "patch": 0
   }
+},
+{
+  "domain": "tvos",
+  "isUnconditionallyUnavailable": true
 }
   ],
   "declarationFragments": [
Index: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp

[PATCH] D144940: [clang][ExtractAPI] Handle platform specific unavailability correctly

[Daniel Grumberg via Phabricator via cfe-commits]

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

LGTM, It's worth noting that if the user specifies that an API is unavailable 
in a later redeclaration, this will be ignored. For example if I add a line to 
the test `void e(void) __attribute__((availability(macos, unavailable)));` it 
will be ignored. Up to you whether you want to fix it now or at a later date.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144940

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

[PATCH] D144940: [clang][ExtractAPI] Handle platform specific unavailability correctly

[Daniel Grumberg via Phabricator via cfe-commits]

dang added a comment.

In D144940#4158143 , @Arsenic wrote:

> In D144940#4158020 , @dang wrote:
>
>> Nice! glad to see this getting fixed. You should add a lit test to ensure we 
>> don't regress this behavior in the future.
>
> I see a test checking for availability attribute already exists ( 
> `clang/test/ExtractAPI/availability.c` ) would it be better if I update it 
> with another function having a platform specific unavailability attribute or 
> should I create a new test file ?

Yes I think updating that test with another API that has a platform specific 
unavailability attribute is the best way to do this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144940

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

[PATCH] D144940: [clang][ExtractAPI] Handle platform specific unavailability correctly

[Daniel Grumberg via Phabricator via cfe-commits]

dang requested changes to this revision.
dang added a comment.
This revision now requires changes to proceed.

Nice! glad to see this getting fixed. You should add a lit test to ensure we 
don't regress this behavior in the future.




Comment at: clang/include/clang/ExtractAPI/AvailabilityInfo.h:48
 private:
-  using AvailabilityList = llvm::SmallVector;
+  using AvailabilityList = llvm::SmallVector;
   AvailabilityList Availabilities;

I don't think this change here is necessary.



Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:546
+  Record, API, [Lang, ](const PathComponent ) {
+ParentContexts.push_back(serializeParentContext(PC, Lang));
+  });

Arsenic wrote:
> These changes seems to have added by clang-format. 
> Is there a way to not let clang format edit the code that I haven't touched ?
Yes you can use `git-clang-format`, instructions here for setup: 
https://offlinemark.com/2021/04/02/surgical-formatting-with-git-clang-format/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144940

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

[PATCH] D142101: [clang] [extract-api] Don't crash for category in libclang APIs

[Daniel Grumberg 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 rG7da2d644e039: [clang] [extract-api] Dont crash for 
category in libclang APIs (authored by dang).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142101

Files:
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
  clang/test/Index/extract-api-cursor.m

Index: clang/test/Index/extract-api-cursor.m
===
--- clang/test/Index/extract-api-cursor.m
+++ clang/test/Index/extract-api-cursor.m
@@ -25,6 +25,12 @@
 - (void)derivedMethodWithValue:(id)value;
 @end
 
+/// This won't show up in docs because we can't serialize it
+@interface Derived ()
+/// Derived method in category docs, won't show up either.
+- (void)derivedMethodInCategory;
+@end
+
 // RUN: c-index-test -single-symbol-sgfs local %s | FileCheck %s
 
 // Checking for Foo
@@ -53,7 +59,7 @@
 
 // Checking for baseProperty
 // CHECK-NEXT: "parentContexts":[{"kind":"objective-c.class","name":"Base","usr":"c:objc(cs)Base"}]
-// CHECK-SAME:"relatedSymbols":[{"accessLevel":"public","declarationLanguage":"objective-c"
+// CHECK-SAME: "relatedSymbols":[{"accessLevel":"public","declarationLanguage":"objective-c"
 // CHECK-SAME: "isSystem":false
 // CHECK-SAME: "usr":"c:@S@Foo"}]
 // CHECK-SAME: "relationships":[{"kind":"memberOf","source":"c:objc(cs)Base(py)baseProperty","target":"c:objc(cs)Base"
@@ -63,7 +69,7 @@
 
 // Checking for baseMethodWithArg
 // CHECK-NEXT: "parentContexts":[{"kind":"objective-c.class","name":"Base","usr":"c:objc(cs)Base"}]
-// CHECK-SAME:"relatedSymbols":[]
+// CHECK-SAME: "relatedSymbols":[]
 // CHECK-SAME: "relationships":[{"kind":"memberOf","source":"c:objc(cs)Base(im)baseMethodWithArg:","target":"c:objc(cs)Base"
 // CHECK-SAME: "text":"Base method docs"
 // CHECK-SAME: "kind":{"displayName":"Instance Method","identifier":"objective-c.method"}
@@ -79,7 +85,7 @@
 
 // Checking for protocolProperty
 // CHECK-NEXT: "parentContexts":[{"kind":"objective-c.protocol","name":"Protocol","usr":"c:objc(pl)Protocol"}]
-// CHECK-SAME:"relatedSymbols":[{"accessLevel":"public","declarationLanguage":"objective-c"
+// CHECK-SAME: "relatedSymbols":[{"accessLevel":"public","declarationLanguage":"objective-c"
 // CHECK-SAME: "isSystem":false
 // CHECK-SAME: "usr":"c:@S@Foo"}]
 // CHECK-SAME: "relationships":[{"kind":"memberOf","source":"c:objc(pl)Protocol(py)protocolProperty","target":"c:objc(pl)Protocol"
@@ -89,7 +95,7 @@
 
 // Checking for Derived
 // CHECK-NEXT: "parentContexts":[]
-// CHECK-SAME:"relatedSymbols":[{"accessLevel":"public","declarationLanguage":"objective-c"
+// CHECK-SAME: "relatedSymbols":[{"accessLevel":"public","declarationLanguage":"objective-c"
 // CHECK-SAME: "isSystem":false
 // CHECK-SAME: "usr":"c:objc(cs)Base"}]
 // CHECK-SAME: "relationships":[{"kind":"inheritsFrom","source":"c:objc(cs)Derived","target":"c:objc(cs)Base"
@@ -99,8 +105,11 @@
 
 // Checking for derivedMethodWithValue
 // CHECK-NEXT: "parentContexts":[{"kind":"objective-c.class","name":"Derived","usr":"c:objc(cs)Derived"}]
-// CHECK-SAME:"relatedSymbols":[]
+// CHECK-SAME: "relatedSymbols":[]
 // CHECK-SAME: "relationships":[{"kind":"memberOf","source":"c:objc(cs)Derived(im)derivedMethodWithValue:","target":"c:objc(cs)Derived"
 // CHECK-SAME: "text":"Derived method docs"
 // CHECK-SAME: "kind":{"displayName":"Instance Method","identifier":"objective-c.method"}
 // CHECK-SAME: "title":"derivedMethodWithValue:"
+
+// CHECK-NOT: This won't show up in docs because we can't serialize it
+// CHECK-NOT: Derived method in category docs, won't show up either.
Index: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
===
--- clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
+++ clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
@@ -487,6 +487,7 @@
   SmallVector ReverseComponenents;
   ReverseComponenents.emplace_back(Record.USR, Record.Name, Record.getKind());
   const auto *CurrentParent = 
+  bool FailedToFindParent = false;
   while (CurrentParent && !CurrentParent->empty()) {
 PathComponent CurrentParentComponent(CurrentParent->ParentUSR,
  CurrentParent->ParentName,
@@ -509,8 +510,10 @@
 
 // The parent record doesn't exist which means the symbol shouldn't be
 // treated as part of the current product.
-if (!ParentRecord)
-  return true;
+if (!ParentRecord) {
+  FailedToFindParent = true;
+  break;
+}
 
 ReverseComponenents.push_back(std::move(CurrentParentComponent));
 CurrentParent = >ParentInformation;
@@ -519,8 +522,9 @@
   for (const auto  : reverse(ReverseComponenents))
 ComponentTransformer(PC);
 
-  return false;
+  return FailedToFindParent;
 }
+
 Object 

[PATCH] D141961: [clang][lex] Pass hash location to more PPCallbacks methods

[Daniel Grumberg via Phabricator via cfe-commits]

dang added a comment.

LGTM for the ExtractAPI changes.


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

https://reviews.llvm.org/D141961

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

[PATCH] D142101: [clang] [extract-api] Don't crash for category in libclang APIs

[Daniel Grumberg via Phabricator via cfe-commits]

dang added inline comments.



Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:537-544
+  generatePathComponents(Record, API,
+ [Lang, ](const PathComponent ) {
+   ParentContexts.push_back(
+   serializeParentContext(PC, Lang));
+ });
+
+  // The last component would be the record itself so let's remove it.

bnbarham wrote:
> It's not really clear to me what the intended behavior was meant to be here, 
> ie. whether if any parent fails we skip writing all *or* if we should be 
> writing all valid parents. I'll leave that to someone that knows more about 
> the symbolgraph output.
> 
> But this fixes the obvious crash, so that part is fine with me.
I think it is a bit of a grey area, having parent contexts and related symbols 
allows downstream clients to perform link resolution on declaration fragment 
chunks e.t.c I figured it would be best to do a best effort and provide the 
known parents.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142101

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

[PATCH] D142101: [clang] [extract-api] Don't crash for category in libclang APIs

[Daniel Grumberg via Phabricator via cfe-commits]

dang updated this revision to Diff 490447.
dang added a comment.

Formatting fixes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142101

Files:
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
  clang/test/Index/extract-api-cursor.m

Index: clang/test/Index/extract-api-cursor.m
===
--- clang/test/Index/extract-api-cursor.m
+++ clang/test/Index/extract-api-cursor.m
@@ -25,6 +25,12 @@
 - (void)derivedMethodWithValue:(id)value;
 @end
 
+/// This won't show up in docs because we can't serialize it
+@interface Derived ()
+/// Derived method in category docs, won't show up either.
+- (void)derivedMethodInCategory;
+@end
+
 // RUN: c-index-test -single-symbol-sgfs local %s | FileCheck %s
 
 // Checking for Foo
@@ -53,7 +59,7 @@
 
 // Checking for baseProperty
 // CHECK-NEXT: "parentContexts":[{"kind":"objective-c.class","name":"Base","usr":"c:objc(cs)Base"}]
-// CHECK-SAME:"relatedSymbols":[{"accessLevel":"public","declarationLanguage":"objective-c"
+// CHECK-SAME: "relatedSymbols":[{"accessLevel":"public","declarationLanguage":"objective-c"
 // CHECK-SAME: "isSystem":false
 // CHECK-SAME: "usr":"c:@S@Foo"}]
 // CHECK-SAME: "relationships":[{"kind":"memberOf","source":"c:objc(cs)Base(py)baseProperty","target":"c:objc(cs)Base"
@@ -63,7 +69,7 @@
 
 // Checking for baseMethodWithArg
 // CHECK-NEXT: "parentContexts":[{"kind":"objective-c.class","name":"Base","usr":"c:objc(cs)Base"}]
-// CHECK-SAME:"relatedSymbols":[]
+// CHECK-SAME: "relatedSymbols":[]
 // CHECK-SAME: "relationships":[{"kind":"memberOf","source":"c:objc(cs)Base(im)baseMethodWithArg:","target":"c:objc(cs)Base"
 // CHECK-SAME: "text":"Base method docs"
 // CHECK-SAME: "kind":{"displayName":"Instance Method","identifier":"objective-c.method"}
@@ -79,7 +85,7 @@
 
 // Checking for protocolProperty
 // CHECK-NEXT: "parentContexts":[{"kind":"objective-c.protocol","name":"Protocol","usr":"c:objc(pl)Protocol"}]
-// CHECK-SAME:"relatedSymbols":[{"accessLevel":"public","declarationLanguage":"objective-c"
+// CHECK-SAME: "relatedSymbols":[{"accessLevel":"public","declarationLanguage":"objective-c"
 // CHECK-SAME: "isSystem":false
 // CHECK-SAME: "usr":"c:@S@Foo"}]
 // CHECK-SAME: "relationships":[{"kind":"memberOf","source":"c:objc(pl)Protocol(py)protocolProperty","target":"c:objc(pl)Protocol"
@@ -89,7 +95,7 @@
 
 // Checking for Derived
 // CHECK-NEXT: "parentContexts":[]
-// CHECK-SAME:"relatedSymbols":[{"accessLevel":"public","declarationLanguage":"objective-c"
+// CHECK-SAME: "relatedSymbols":[{"accessLevel":"public","declarationLanguage":"objective-c"
 // CHECK-SAME: "isSystem":false
 // CHECK-SAME: "usr":"c:objc(cs)Base"}]
 // CHECK-SAME: "relationships":[{"kind":"inheritsFrom","source":"c:objc(cs)Derived","target":"c:objc(cs)Base"
@@ -99,8 +105,11 @@
 
 // Checking for derivedMethodWithValue
 // CHECK-NEXT: "parentContexts":[{"kind":"objective-c.class","name":"Derived","usr":"c:objc(cs)Derived"}]
-// CHECK-SAME:"relatedSymbols":[]
+// CHECK-SAME: "relatedSymbols":[]
 // CHECK-SAME: "relationships":[{"kind":"memberOf","source":"c:objc(cs)Derived(im)derivedMethodWithValue:","target":"c:objc(cs)Derived"
 // CHECK-SAME: "text":"Derived method docs"
 // CHECK-SAME: "kind":{"displayName":"Instance Method","identifier":"objective-c.method"}
 // CHECK-SAME: "title":"derivedMethodWithValue:"
+
+// CHECK-NOT: This won't show up in docs because we can't serialize it
+// CHECK-NOT: Derived method in category docs, won't show up either.
Index: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
===
--- clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
+++ clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
@@ -484,6 +484,7 @@
   SmallVector ReverseComponenents;
   ReverseComponenents.emplace_back(Record.USR, Record.Name, Record.getKind());
   const auto *CurrentParent = 
+  bool FailedToFindParent = false;
   while (CurrentParent && !CurrentParent->empty()) {
 PathComponent CurrentParentComponent(CurrentParent->ParentUSR,
  CurrentParent->ParentName,
@@ -506,8 +507,10 @@
 
 // The parent record doesn't exist which means the symbol shouldn't be
 // treated as part of the current product.
-if (!ParentRecord)
-  return true;
+if (!ParentRecord) {
+  FailedToFindParent = true;
+  break;
+}
 
 ReverseComponenents.push_back(std::move(CurrentParentComponent));
 CurrentParent = >ParentInformation;
@@ -516,8 +519,9 @@
   for (const auto  : reverse(ReverseComponenents))
 ComponentTransformer(PC);
 
-  return false;
+  return FailedToFindParent;
 }
+
 Object serializeParentContext(const PathComponent , Language Lang) {
   Object ParentContextElem;
   ParentContextElem["usr"] = PC.USR;
@@ -530,12 +534,15 @@
 Array 

[PATCH] D142101: [clang] [extract-api] Don't crash for category in libclang APIs

[Daniel Grumberg via Phabricator via cfe-commits]

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

Remove failure conditions for categories in libclang and return empty
content instead.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D142101

Files:
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
  clang/test/Index/extract-api-cursor.m

Index: clang/test/Index/extract-api-cursor.m
===
--- clang/test/Index/extract-api-cursor.m
+++ clang/test/Index/extract-api-cursor.m
@@ -25,6 +25,12 @@
 - (void)derivedMethodWithValue:(id)value;
 @end
 
+/// This won't show up in docs because we can't serialize it
+@interface Derived ()
+/// Derived method in category docs, won't show up either.
+- (void)derivedMethodInCategory;
+@end
+
 // RUN: c-index-test -single-symbol-sgfs local %s | FileCheck %s
 
 // Checking for Foo
@@ -53,7 +59,7 @@
 
 // Checking for baseProperty
 // CHECK-NEXT: "parentContexts":[{"kind":"objective-c.class","name":"Base","usr":"c:objc(cs)Base"}]
-// CHECK-SAME:"relatedSymbols":[{"accessLevel":"public","declarationLanguage":"objective-c"
+// CHECK-SAME: "relatedSymbols":[{"accessLevel":"public","declarationLanguage":"objective-c"
 // CHECK-SAME: "isSystem":false
 // CHECK-SAME: "usr":"c:@S@Foo"}]
 // CHECK-SAME: "relationships":[{"kind":"memberOf","source":"c:objc(cs)Base(py)baseProperty","target":"c:objc(cs)Base"
@@ -63,7 +69,7 @@
 
 // Checking for baseMethodWithArg
 // CHECK-NEXT: "parentContexts":[{"kind":"objective-c.class","name":"Base","usr":"c:objc(cs)Base"}]
-// CHECK-SAME:"relatedSymbols":[]
+// CHECK-SAME: "relatedSymbols":[]
 // CHECK-SAME: "relationships":[{"kind":"memberOf","source":"c:objc(cs)Base(im)baseMethodWithArg:","target":"c:objc(cs)Base"
 // CHECK-SAME: "text":"Base method docs"
 // CHECK-SAME: "kind":{"displayName":"Instance Method","identifier":"objective-c.method"}
@@ -79,7 +85,7 @@
 
 // Checking for protocolProperty
 // CHECK-NEXT: "parentContexts":[{"kind":"objective-c.protocol","name":"Protocol","usr":"c:objc(pl)Protocol"}]
-// CHECK-SAME:"relatedSymbols":[{"accessLevel":"public","declarationLanguage":"objective-c"
+// CHECK-SAME: "relatedSymbols":[{"accessLevel":"public","declarationLanguage":"objective-c"
 // CHECK-SAME: "isSystem":false
 // CHECK-SAME: "usr":"c:@S@Foo"}]
 // CHECK-SAME: "relationships":[{"kind":"memberOf","source":"c:objc(pl)Protocol(py)protocolProperty","target":"c:objc(pl)Protocol"
@@ -89,7 +95,7 @@
 
 // Checking for Derived
 // CHECK-NEXT: "parentContexts":[]
-// CHECK-SAME:"relatedSymbols":[{"accessLevel":"public","declarationLanguage":"objective-c"
+// CHECK-SAME: "relatedSymbols":[{"accessLevel":"public","declarationLanguage":"objective-c"
 // CHECK-SAME: "isSystem":false
 // CHECK-SAME: "usr":"c:objc(cs)Base"}]
 // CHECK-SAME: "relationships":[{"kind":"inheritsFrom","source":"c:objc(cs)Derived","target":"c:objc(cs)Base"
@@ -99,8 +105,11 @@
 
 // Checking for derivedMethodWithValue
 // CHECK-NEXT: "parentContexts":[{"kind":"objective-c.class","name":"Derived","usr":"c:objc(cs)Derived"}]
-// CHECK-SAME:"relatedSymbols":[]
+// CHECK-SAME: "relatedSymbols":[]
 // CHECK-SAME: "relationships":[{"kind":"memberOf","source":"c:objc(cs)Derived(im)derivedMethodWithValue:","target":"c:objc(cs)Derived"
 // CHECK-SAME: "text":"Derived method docs"
 // CHECK-SAME: "kind":{"displayName":"Instance Method","identifier":"objective-c.method"}
 // CHECK-SAME: "title":"derivedMethodWithValue:"
+//
+// CHECK-NOT: This won't show up in docs because we can't serialize it
+// CHECK-NOT: Derived method in category docs, won't show up either.
Index: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
===
--- clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
+++ clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
@@ -390,7 +390,7 @@
 break;
   case APIRecord::RK_ObjCCategory:
 // We don't serialize out standalone Objective-C category symbols yet.
-llvm_unreachable("Serializing standalone Objective-C category symbols is "
+llvm_unreachable("Serializing standalone Objective-C category symbols is"
  "not supported.");
 break;
   case APIRecord::RK_ObjCProtocol:
@@ -484,6 +484,7 @@
   SmallVector ReverseComponenents;
   ReverseComponenents.emplace_back(Record.USR, Record.Name, Record.getKind());
   const auto *CurrentParent = 
+  bool FailedToFindParent = false;
   while (CurrentParent && !CurrentParent->empty()) {
 PathComponent CurrentParentComponent(CurrentParent->ParentUSR,
  CurrentParent->ParentName,
@@ -506,8 +507,10 @@
 
 // The parent record doesn't exist which means the symbol shouldn't be
 // treated as part of the 

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

[Daniel Grumberg via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8dcb629aa4cc: [clang][ExtractAPI] Fix naming of 
typedefd anonymous enums (authored by dang).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140010

Files:
  clang/lib/ExtractAPI/ExtractAPIVisitor.cpp
  clang/test/ExtractAPI/typedef_anonymous_record.c

Index: clang/test/ExtractAPI/typedef_anonymous_record.c
===
--- clang/test/ExtractAPI/typedef_anonymous_record.c
+++ clang/test/ExtractAPI/typedef_anonymous_record.c
@@ -2,21 +2,22 @@
 // RUN: split-file %s %t
 // RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
 // RUN: %t/reference.output.json.in >> %t/reference.output.json
-// RUN: %clang -extract-api --product-name=TypedefChain -target arm64-apple-macosx \
-// RUN: -x objective-c-header %t/input.h -o %t/output.json | FileCheck -allow-empty %s
+// RUN: %clang_cc1 -extract-api --product-name=TypedefChain -triple arm64-apple-macosx \
+// RUN:   -x c-header %t/input.h -o %t/output.json -verify
 
 // 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
 typedef struct { } MyStruct;
 typedef MyStruct MyStructStruct;
 typedef MyStructStruct MyStructStructStruct;
+typedef enum { Case } MyEnum;
+typedef MyEnum MyEnumEnum;
+typedef MyEnumEnum MyEnumEnumEnum;
+// expected-no-diagnostics
 
 //--- reference.output.json.in
 {
@@ -43,8 +44,110 @@
   "vendor": "apple"
 }
   },
-  "relationships": [],
+  "relationships": [
+{
+  "kind": "memberOf",
+  "source": "c:@EA@MyEnum@Case",
+  "target": "c:@EA@MyEnum",
+  "targetFallback": "MyEnum"
+}
+  ],
   "symbols": [
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "keyword",
+  "spelling": "typedef"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "keyword",
+  "spelling": "enum"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "MyEnum"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "c",
+"precise": "c:@EA@MyEnum"
+  },
+  "kind": {
+"displayName": "Enumeration",
+"identifier": "c.enum"
+  },
+  "location": {
+"position": {
+  "character": 9,
+  "line": 4
+},
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"navigator": [
+  {
+"kind": "identifier",
+"spelling": "MyEnum"
+  }
+],
+"title": "MyEnum"
+  },
+  "pathComponents": [
+"MyEnum"
+  ]
+},
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "identifier",
+  "spelling": "Case"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "c",
+"precise": "c:@EA@MyEnum@Case"
+  },
+  "kind": {
+"displayName": "Enumeration Case",
+"identifier": "c.enum.case"
+  },
+  "location": {
+"position": {
+  "character": 16,
+  "line": 4
+},
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"navigator": [
+  {
+"kind": "identifier",
+"spelling": "Case"
+  }
+],
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "Case"
+  }
+],
+"title": "Case"
+  },
+  "pathComponents": [
+"MyEnum",
+"Case"
+  ]
+},
 {
   "accessLevel": "public",
   "declarationFragments": [
@@ -70,12 +173,12 @@
 }
   ],
   "identifier": {
-"interfaceLanguage": "objective-c",
+"interfaceLanguage": "c",
 "precise": "c:@SA@MyStruct"
   },
   "kind": {
 "displayName": "Structure",
-"identifier": "objective-c.struct"
+"identifier": "c.struct"
   },
   "location": {
 "position": {
@@ -123,12 +226,12 @@
 }
   ],
   "identifier": {
-"interfaceLanguage": "objective-c",
+"interfaceLanguage": "c",
 "precise": "c:input.h@T@MyStructStruct"
   },
   "kind": {
 "displayName": "Type Alias",
-"identifier": "objective-c.typealias"
+"identifier": "c.typealias"
   },
   "location": {
 "position": {
@@ -183,12 +286,12 @@
 }
   ],
   "identifier": {
-"interfaceLanguage": 

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

[Daniel Grumberg via Phabricator via cfe-commits]

dang 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()) {

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).


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

[Daniel Grumberg via Phabricator via cfe-commits]

dang 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()) {

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.


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

[Daniel Grumberg via Phabricator via cfe-commits]

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

Anonymous enums that are typedef'd should take on the name of the typedef.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140010

Files:
  clang/lib/ExtractAPI/ExtractAPIVisitor.cpp
  clang/test/ExtractAPI/typedef_anonymous_record.c

Index: clang/test/ExtractAPI/typedef_anonymous_record.c
===
--- clang/test/ExtractAPI/typedef_anonymous_record.c
+++ clang/test/ExtractAPI/typedef_anonymous_record.c
@@ -2,21 +2,22 @@
 // RUN: split-file %s %t
 // RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
 // RUN: %t/reference.output.json.in >> %t/reference.output.json
-// RUN: %clang -extract-api --product-name=TypedefChain -target arm64-apple-macosx \
-// RUN: -x objective-c-header %t/input.h -o %t/output.json | FileCheck -allow-empty %s
+// RUN: %clang_cc1 -extract-api --product-name=TypedefChain -triple arm64-apple-macosx \
+// RUN:   -x c-header %t/input.h -o %t/output.json -verify
 
 // 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
 typedef struct { } MyStruct;
 typedef MyStruct MyStructStruct;
 typedef MyStructStruct MyStructStructStruct;
+typedef enum { Case } MyEnum;
+typedef MyEnum MyEnumEnum;
+typedef MyEnumEnum MyEnumEnumEnum;
+// expected-no-diagnostics
 
 //--- reference.output.json.in
 {
@@ -43,8 +44,110 @@
   "vendor": "apple"
 }
   },
-  "relationships": [],
+  "relationships": [
+{
+  "kind": "memberOf",
+  "source": "c:@EA@MyEnum@Case",
+  "target": "c:@EA@MyEnum",
+  "targetFallback": "MyEnum"
+}
+  ],
   "symbols": [
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "keyword",
+  "spelling": "typedef"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "keyword",
+  "spelling": "enum"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "MyEnum"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "c",
+"precise": "c:@EA@MyEnum"
+  },
+  "kind": {
+"displayName": "Enumeration",
+"identifier": "c.enum"
+  },
+  "location": {
+"position": {
+  "character": 9,
+  "line": 4
+},
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"navigator": [
+  {
+"kind": "identifier",
+"spelling": "MyEnum"
+  }
+],
+"title": "MyEnum"
+  },
+  "pathComponents": [
+"MyEnum"
+  ]
+},
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "identifier",
+  "spelling": "Case"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "c",
+"precise": "c:@EA@MyEnum@Case"
+  },
+  "kind": {
+"displayName": "Enumeration Case",
+"identifier": "c.enum.case"
+  },
+  "location": {
+"position": {
+  "character": 16,
+  "line": 4
+},
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"navigator": [
+  {
+"kind": "identifier",
+"spelling": "Case"
+  }
+],
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "Case"
+  }
+],
+"title": "Case"
+  },
+  "pathComponents": [
+"MyEnum",
+"Case"
+  ]
+},
 {
   "accessLevel": "public",
   "declarationFragments": [
@@ -70,12 +173,12 @@
 }
   ],
   "identifier": {
-"interfaceLanguage": "objective-c",
+"interfaceLanguage": "c",
 "precise": "c:@SA@MyStruct"
   },
   "kind": {
 "displayName": "Structure",
-"identifier": "objective-c.struct"
+"identifier": "c.struct"
   },
   "location": {
 "position": {
@@ -123,12 +226,12 @@
 }
   ],
   "identifier": {
-"interfaceLanguage": "objective-c",
+"interfaceLanguage": "c",
 "precise": "c:input.h@T@MyStructStruct"
   },
   "kind": {
 "displayName": "Type Alias",
-"identifier": "objective-c.typealias"
+"identifier": "c.typealias"
   },
   "location": {
 "position": {
@@ -183,12 +286,12 @@
 }
   ],
   

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

[Daniel Grumberg via Phabricator via cfe-commits]

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



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

zixuw wrote:
> 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
Currently yes, the Decl is available as the APISet is not kept around beyond 
the lifetime of the AST. Nonetheless I agree that it is used in an unusual 
manner, to help traverse the AST in libclang as opposed to truly encoding 
information about the available APIs. I think if we introduce new clients of 
APISet we would need to add an extension mechanism to the visitation mechanism 
to enable use cases like this.


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] D139115: [clang][ExtractAPI] Add support for single symbol SGF

[Daniel Grumberg via Phabricator via cfe-commits]

dang marked 6 inline comments as done.
dang added inline comments.



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

zixuw wrote:
> What's the reason for refactoring out instance vs. class property? Looks like 
> this should be a separated patch
It was needed to simplify the implementation of 
`SymbolGraphSerializer::serializeSingleRecord`


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

[Daniel Grumberg via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG671709f0e7d4: [clang][ExtractAPI] Add targetFallback to 
relationships in symbol graph (authored by dang).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136455

Files:
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
  clang/test/ExtractAPI/anonymous_record_no_typedef.c
  clang/test/ExtractAPI/enum.c
  clang/test/ExtractAPI/objc_category.m
  clang/test/ExtractAPI/objc_interface.m
  clang/test/ExtractAPI/objc_property.m
  clang/test/ExtractAPI/objc_protocol.m
  clang/test/ExtractAPI/struct.c
  clang/test/ExtractAPI/underscored.c

Index: clang/test/ExtractAPI/underscored.c
===
--- clang/test/ExtractAPI/underscored.c
+++ clang/test/ExtractAPI/underscored.c
@@ -65,7 +65,8 @@
 {
   "kind": "memberOf",
   "source": "c:@S@ExposedRecord@FI@a",
-  "target": "c:@S@ExposedRecord"
+  "target": "c:@S@ExposedRecord",
+  "targetFallback": "ExposedRecord"
 }
   ],
   "symbols": [
Index: clang/test/ExtractAPI/struct.c
===
--- clang/test/ExtractAPI/struct.c
+++ clang/test/ExtractAPI/struct.c
@@ -52,22 +52,26 @@
 {
   "kind": "memberOf",
   "source": "c:@S@Color@FI@Red",
-  "target": "c:@S@Color"
+  "target": "c:@S@Color",
+  "targetFallback": "Color"
 },
 {
   "kind": "memberOf",
   "source": "c:@S@Color@FI@Green",
-  "target": "c:@S@Color"
+  "target": "c:@S@Color",
+  "targetFallback": "Color"
 },
 {
   "kind": "memberOf",
   "source": "c:@S@Color@FI@Blue",
-  "target": "c:@S@Color"
+  "target": "c:@S@Color",
+  "targetFallback": "Color"
 },
 {
   "kind": "memberOf",
   "source": "c:@S@Color@FI@Alpha",
-  "target": "c:@S@Color"
+  "target": "c:@S@Color",
+  "targetFallback": "Color"
 }
   ],
   "symbols": [
Index: clang/test/ExtractAPI/objc_protocol.m
===
--- clang/test/ExtractAPI/objc_protocol.m
+++ clang/test/ExtractAPI/objc_protocol.m
@@ -49,7 +49,8 @@
 {
   "kind": "conformsTo",
   "source": "c:objc(pl)AnotherProtocol",
-  "target": "c:objc(pl)Protocol"
+  "target": "c:objc(pl)Protocol",
+  "targetFallback": "Protocol"
 }
   ],
   "symbols": [
Index: clang/test/ExtractAPI/objc_property.m
===
--- clang/test/ExtractAPI/objc_property.m
+++ clang/test/ExtractAPI/objc_property.m
@@ -55,37 +55,44 @@
 {
   "kind": "memberOf",
   "source": "c:objc(cs)Interface(cpy)myInterfaceTypeProp",
-  "target": "c:objc(cs)Interface"
+  "target": "c:objc(cs)Interface",
+  "targetFallback": "Interface"
 },
 {
   "kind": "memberOf",
   "source": "c:objc(cs)Interface(py)myInterfaceInstanceProp",
-  "target": "c:objc(cs)Interface"
+  "target": "c:objc(cs)Interface",
+  "targetFallback": "Interface"
 },
 {
   "kind": "memberOf",
   "source": "c:objc(cs)Interface(cpy)myCategoryTypeProp",
-  "target": "c:objc(cs)Interface"
+  "target": "c:objc(cs)Interface",
+  "targetFallback": "Interface"
 },
 {
   "kind": "memberOf",
   "source": "c:objc(cs)Interface(py)myCategoryInstanceProp",
-  "target": "c:objc(cs)Interface"
+  "target": "c:objc(cs)Interface",
+  "targetFallback": "Interface"
 },
 {
   "kind": "conformsTo",
   "source": "c:objc(cs)Interface",
-  "target": "c:objc(pl)Protocol"
+  "target": "c:objc(pl)Protocol",
+  "targetFallback": "Protocol"
 },
 {
   "kind": "memberOf",
   "source": "c:objc(pl)Protocol(cpy)myProtocolTypeProp",
-  "target": "c:objc(pl)Protocol"
+  "target": "c:objc(pl)Protocol",
+  "targetFallback": "Protocol"
 },
 {
   "kind": "memberOf",
   "source": "c:objc(pl)Protocol(py)myProtocolInstanceProp",
-  "target": "c:objc(pl)Protocol"
+  "target": "c:objc(pl)Protocol",
+  "targetFallback": "Protocol"
 }
   ],
   "symbols": [
Index: clang/test/ExtractAPI/objc_interface.m
===
--- clang/test/ExtractAPI/objc_interface.m
+++ clang/test/ExtractAPI/objc_interface.m
@@ -57,37 +57,44 @@
 {
   "kind": "memberOf",
   "source": "c:objc(cs)Super(cm)getWithProperty:",
-  "target": "c:objc(cs)Super"
+  "target": "c:objc(cs)Super",
+  "targetFallback": "Super"
 },
 {
   "kind": "memberOf",
   "source": "c:objc(cs)Super(im)setProperty:andOtherThing:",
-  "target": "c:objc(cs)Super"
+  "target": "c:objc(cs)Super",
+  "targetFallback": "Super"
 },
 {
   "kind": "memberOf",
   "source": 

[PATCH] D136450: [clang][ExtractAPI] Allow users to specify a list of symbols to ignore

[Daniel Grumberg via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG791fe26d7581: [clang][ExtractAPI] Allow users to specify a 
list of symbols to ignore (authored by dang).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136450

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/ExtractAPI/APIIgnoresList.h
  clang/include/clang/ExtractAPI/FrontendActions.h
  clang/include/clang/ExtractAPI/Serialization/SerializerBase.h
  clang/include/clang/ExtractAPI/Serialization/SymbolGraphSerializer.h
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/ExtractAPI/APIIgnoresList.cpp
  clang/lib/ExtractAPI/CMakeLists.txt
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
  clang/test/Driver/extract-api-unknown-ignore-diag.h
  clang/test/ExtractAPI/ignored-symbols.c

Index: clang/test/ExtractAPI/ignored-symbols.c
===
--- /dev/null
+++ clang/test/ExtractAPI/ignored-symbols.c
@@ -0,0 +1,27 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -extract-api -triple arm64-apple-macosx \
+// RUN:   --extract-api-ignores=%t/ignores-list\
+// RUN:   -x c-header %t/input.h -verify -o - | FileCheck %t/input.h
+
+//--- input.h
+#define IGNORED_1 1
+#define IGNORED_2 2
+#define IGNORED_3 3
+#define IGNORED_4 4
+typedef int Ignored;
+typedef float NonIgnored;
+
+// CHECK-NOT: IGNORED_1
+// CHECK-NOT: IGNORED_2
+// CHECK-NOT: IGNORED_3
+// CHECK: NonIgnored
+
+// expected-no-diagnostics
+
+//--- ignores-list
+Ignored
+IGNORED_4
+IGNORED_3   
+IGNORED_2
+IGNORED_1
Index: clang/test/Driver/extract-api-unknown-ignore-diag.h
===
--- /dev/null
+++ clang/test/Driver/extract-api-unknown-ignore-diag.h
@@ -0,0 +1,6 @@
+// RUN: rm -rf %t
+// RUN: not %clang -target x86_64-unknown-unknown -extract-api --extract-api-ignores=does-not-exist %s 2>&1 | FileCheck %s
+
+// CHECK: fatal error: file 'does-not-exist' specified by '--extract-api-ignores=' not found
+
+void dummy_function(void);
Index: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
===
--- clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
+++ clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
@@ -14,6 +14,7 @@
 #include "clang/ExtractAPI/Serialization/SymbolGraphSerializer.h"
 #include "clang/Basic/Version.h"
 #include "clang/ExtractAPI/API.h"
+#include "clang/ExtractAPI/APIIgnoresList.h"
 #include "clang/ExtractAPI/DeclarationFragments.h"
 #include "llvm/Support/JSON.h"
 #include "llvm/Support/Path.h"
@@ -480,6 +481,10 @@
 }
 
 bool SymbolGraphSerializer::shouldSkip(const APIRecord ) const {
+  // Skip explicitly ignored symbols.
+  if (IgnoresList.shouldIgnore(Record.Name))
+return true;
+
   // Skip unconditionally unavailable symbols
   if (Record.Availabilities.isUnconditionallyUnavailable())
 return true;
Index: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
===
--- clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
+++ clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
@@ -20,10 +20,12 @@
 #include "clang/AST/ParentMapContext.h"
 #include "clang/AST/RawCommentList.h"
 #include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Basic/DiagnosticFrontend.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/ExtractAPI/API.h"
+#include "clang/ExtractAPI/APIIgnoresList.h"
 #include "clang/ExtractAPI/AvailabilityInfo.h"
 #include "clang/ExtractAPI/DeclarationFragments.h"
 #include "clang/ExtractAPI/FrontendActions.h"
@@ -38,6 +40,7 @@
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
@@ -858,6 +861,18 @@
   Policy.AnonymousTagLocations = false;
   CI.getASTContext().setPrintingPolicy(Policy);
 
+  if (!CI.getFrontendOpts().ExtractAPIIgnoresFile.empty()) {
+llvm::handleAllErrors(
+APIIgnoresList::create(CI.getFrontendOpts().ExtractAPIIgnoresFile,
+   CI.getFileManager())
+.moveInto(IgnoresList),
+[](const IgnoresFileNotFound ) {
+  CI.getDiagnostics().Report(
+  diag::err_extract_api_ignores_file_not_found)
+  << Err.Path;
+});
+  }
+
   return std::make_unique(CI.getASTContext(),
   std::move(LCF), *API);
 }
@@ -926,7 +941,7 @@
   // Setup a SymbolGraphSerializer to write out collected API 

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

[Daniel Grumberg via Phabricator via cfe-commits]

dang added a comment.

However, I am not sure it's worthwhile and we might run into edge cases doing 
it this way with explicitly ignored symbols.


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] D136455: [clang][ExtractAPI] Add targetFallback to relationships in symbol graph

[Daniel Grumberg via Phabricator via cfe-commits]

dang added a comment.

In D136455#3879849 , @zixuw wrote:

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

I think it can be done, since we ignore symbols that don't come from the 
headers provided on the command line, we could just check if the target symbol 
is something that is known to the APISet.


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] D136455: [clang][ExtractAPI] Add targetFallback to relationships in symbol graph

[Daniel Grumberg via Phabricator via cfe-commits]

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

Adds a 'targetFallback' field to relationships in symbol graph that
contains the plain name of the relationship target. This is useful for
clients when the relationship target symbol is not available.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136455

Files:
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
  clang/test/ExtractAPI/anonymous_record_no_typedef.c
  clang/test/ExtractAPI/enum.c
  clang/test/ExtractAPI/objc_category.m
  clang/test/ExtractAPI/objc_interface.m
  clang/test/ExtractAPI/objc_property.m
  clang/test/ExtractAPI/objc_protocol.m
  clang/test/ExtractAPI/struct.c
  clang/test/ExtractAPI/underscored.c

Index: clang/test/ExtractAPI/underscored.c
===
--- clang/test/ExtractAPI/underscored.c
+++ clang/test/ExtractAPI/underscored.c
@@ -65,7 +65,8 @@
 {
   "kind": "memberOf",
   "source": "c:@S@ExposedRecord@FI@a",
-  "target": "c:@S@ExposedRecord"
+  "target": "c:@S@ExposedRecord",
+  "targetFallback": "ExposedRecord"
 }
   ],
   "symbols": [
Index: clang/test/ExtractAPI/struct.c
===
--- clang/test/ExtractAPI/struct.c
+++ clang/test/ExtractAPI/struct.c
@@ -52,22 +52,26 @@
 {
   "kind": "memberOf",
   "source": "c:@S@Color@FI@Red",
-  "target": "c:@S@Color"
+  "target": "c:@S@Color",
+  "targetFallback": "Color"
 },
 {
   "kind": "memberOf",
   "source": "c:@S@Color@FI@Green",
-  "target": "c:@S@Color"
+  "target": "c:@S@Color",
+  "targetFallback": "Color"
 },
 {
   "kind": "memberOf",
   "source": "c:@S@Color@FI@Blue",
-  "target": "c:@S@Color"
+  "target": "c:@S@Color",
+  "targetFallback": "Color"
 },
 {
   "kind": "memberOf",
   "source": "c:@S@Color@FI@Alpha",
-  "target": "c:@S@Color"
+  "target": "c:@S@Color",
+  "targetFallback": "Color"
 }
   ],
   "symbols": [
Index: clang/test/ExtractAPI/objc_protocol.m
===
--- clang/test/ExtractAPI/objc_protocol.m
+++ clang/test/ExtractAPI/objc_protocol.m
@@ -49,7 +49,8 @@
 {
   "kind": "conformsTo",
   "source": "c:objc(pl)AnotherProtocol",
-  "target": "c:objc(pl)Protocol"
+  "target": "c:objc(pl)Protocol",
+  "targetFallback": "Protocol"
 }
   ],
   "symbols": [
Index: clang/test/ExtractAPI/objc_property.m
===
--- clang/test/ExtractAPI/objc_property.m
+++ clang/test/ExtractAPI/objc_property.m
@@ -55,37 +55,44 @@
 {
   "kind": "memberOf",
   "source": "c:objc(cs)Interface(cpy)myInterfaceTypeProp",
-  "target": "c:objc(cs)Interface"
+  "target": "c:objc(cs)Interface",
+  "targetFallback": "Interface"
 },
 {
   "kind": "memberOf",
   "source": "c:objc(cs)Interface(py)myInterfaceInstanceProp",
-  "target": "c:objc(cs)Interface"
+  "target": "c:objc(cs)Interface",
+  "targetFallback": "Interface"
 },
 {
   "kind": "memberOf",
   "source": "c:objc(cs)Interface(cpy)myCategoryTypeProp",
-  "target": "c:objc(cs)Interface"
+  "target": "c:objc(cs)Interface",
+  "targetFallback": "Interface"
 },
 {
   "kind": "memberOf",
   "source": "c:objc(cs)Interface(py)myCategoryInstanceProp",
-  "target": "c:objc(cs)Interface"
+  "target": "c:objc(cs)Interface",
+  "targetFallback": "Interface"
 },
 {
   "kind": "conformsTo",
   "source": "c:objc(cs)Interface",
-  "target": "c:objc(pl)Protocol"
+  "target": "c:objc(pl)Protocol",
+  "targetFallback": "Protocol"
 },
 {
   "kind": "memberOf",
   "source": "c:objc(pl)Protocol(cpy)myProtocolTypeProp",
-  "target": "c:objc(pl)Protocol"
+  "target": "c:objc(pl)Protocol",
+  "targetFallback": "Protocol"
 },
 {
   "kind": "memberOf",
   "source": "c:objc(pl)Protocol(py)myProtocolInstanceProp",
-  "target": "c:objc(pl)Protocol"
+  "target": "c:objc(pl)Protocol",
+  "targetFallback": "Protocol"
 }
   ],
   "symbols": [
Index: clang/test/ExtractAPI/objc_interface.m
===
--- clang/test/ExtractAPI/objc_interface.m
+++ clang/test/ExtractAPI/objc_interface.m
@@ -57,37 +57,44 @@
 {
   "kind": "memberOf",
   "source": "c:objc(cs)Super(cm)getWithProperty:",
-  "target": "c:objc(cs)Super"
+  "target": "c:objc(cs)Super",
+  "targetFallback": "Super"
 },
 {
   "kind": "memberOf",
   "source": 

[PATCH] D136450: [clang][ExtractAPI] Allow users to specify a list of symbols to ignore

[Daniel Grumberg via Phabricator via cfe-commits]

dang created this revision.
dang added reviewers: zixuw, QuietMisdreavus.
Herald added a reviewer: ributzka.
Herald added a project: All.
dang requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

Adds a `--extract-api-ignores=` command line option that allows users to
provide a file containing a new line separated list of symbols to
unconditionally ignore when extracting API information.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136450

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/ExtractAPI/APIIgnoresList.h
  clang/include/clang/ExtractAPI/FrontendActions.h
  clang/include/clang/ExtractAPI/Serialization/SerializerBase.h
  clang/include/clang/ExtractAPI/Serialization/SymbolGraphSerializer.h
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/ExtractAPI/APIIgnoresList.cpp
  clang/lib/ExtractAPI/CMakeLists.txt
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
  clang/test/Driver/extract-api-unknown-ignore-diag.h
  clang/test/ExtractAPI/ignored-symbols.c

Index: clang/test/ExtractAPI/ignored-symbols.c
===
--- /dev/null
+++ clang/test/ExtractAPI/ignored-symbols.c
@@ -0,0 +1,27 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -extract-api -triple arm64-apple-macosx \
+// RUN:   --extract-api-ignores=%t/ignores-list\
+// RUN:   -x c-header %t/input.h -verify -o - | FileCheck %t/input.h
+
+//--- input.h
+#define IGNORED_1 1
+#define IGNORED_2 2
+#define IGNORED_3 3
+#define IGNORED_4 4
+typedef int Ignored;
+typedef float NonIgnored;
+
+// CHECK-NOT: IGNORED_1
+// CHECK-NOT: IGNORED_2
+// CHECK-NOT: IGNORED_3
+// CHECK: NonIgnored
+
+// expected-no-diagnostics
+
+//--- ignores-list
+Ignored
+IGNORED_4
+IGNORED_3   
+IGNORED_2
+IGNORED_1
Index: clang/test/Driver/extract-api-unknown-ignore-diag.h
===
--- /dev/null
+++ clang/test/Driver/extract-api-unknown-ignore-diag.h
@@ -0,0 +1,6 @@
+// RUN: rm -rf %t
+// RUN: not %clang -target x86_64-unknown-unknown -extract-api --extract-api-ignores=does-not-exist %s 2>&1 | FileCheck %s
+
+// CHECK: fatal error: file 'does-not-exist' specified by '--extract-api-ignores=' not found
+
+void dummy_function(void);
Index: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
===
--- clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
+++ clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
@@ -14,6 +14,7 @@
 #include "clang/ExtractAPI/Serialization/SymbolGraphSerializer.h"
 #include "clang/Basic/Version.h"
 #include "clang/ExtractAPI/API.h"
+#include "clang/ExtractAPI/APIIgnoresList.h"
 #include "clang/ExtractAPI/DeclarationFragments.h"
 #include "llvm/Support/JSON.h"
 #include "llvm/Support/Path.h"
@@ -480,6 +481,10 @@
 }
 
 bool SymbolGraphSerializer::shouldSkip(const APIRecord ) const {
+  // Skip explicitly ignored symbols.
+  if (IgnoresList.shouldIgnore(Record.Name))
+return true;
+
   // Skip unconditionally unavailable symbols
   if (Record.Availabilities.isUnconditionallyUnavailable())
 return true;
Index: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
===
--- clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
+++ clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
@@ -20,10 +20,12 @@
 #include "clang/AST/ParentMapContext.h"
 #include "clang/AST/RawCommentList.h"
 #include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Basic/DiagnosticFrontend.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/ExtractAPI/API.h"
+#include "clang/ExtractAPI/APIIgnoresList.h"
 #include "clang/ExtractAPI/AvailabilityInfo.h"
 #include "clang/ExtractAPI/DeclarationFragments.h"
 #include "clang/ExtractAPI/FrontendActions.h"
@@ -38,6 +40,7 @@
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
@@ -858,6 +861,18 @@
   Policy.AnonymousTagLocations = false;
   CI.getASTContext().setPrintingPolicy(Policy);
 
+  if (!CI.getFrontendOpts().ExtractAPIIgnoresFile.empty()) {
+llvm::handleAllErrors(
+APIIgnoresList::create(CI.getFrontendOpts().ExtractAPIIgnoresFile,
+   CI.getFileManager())
+.moveInto(IgnoresList),
+[](const IgnoresFileNotFound ) {
+  CI.getDiagnostics().Report(
+  diag::err_extract_api_ignores_file_not_found)
+  << Err.Path;
+});
+  }
+
   return 

[PATCH] D135804: [clang][ExtractAPI] Ignore fully anonymous RecordDecls

[Daniel Grumberg via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb6da16ffb9d5: [clang][ExtractAPI] Ignore fully anonymous 
RecordDecls (authored by dang).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135804

Files:
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/test/ExtractAPI/anonymous_record_no_typedef.c

Index: clang/test/ExtractAPI/anonymous_record_no_typedef.c
===
--- /dev/null
+++ clang/test/ExtractAPI/anonymous_record_no_typedef.c
@@ -0,0 +1,396 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
+// RUN: %t/reference.output.json.in >> %t/reference.output.json
+// RUN: %clang_cc1 -extract-api -triple arm64-apple-macosx \
+// RUN:   -x c-header %t/input.h -o %t/output.json -verify
+
+// 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
+
+//--- input.h
+/// A Vehicle
+struct Vehicle {
+/// The type of vehicle.
+enum {
+Bicycle,
+Car
+} type;
+
+/// The information about the vehicle.
+struct {
+int wheels;
+char *name;
+} information;
+};
+// expected-no-diagnostics
+
+//--- 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:@S@Vehicle@E@input.h@64@Bicycle",
+  "target": "c:@S@Vehicle@E@input.h@64"
+},
+{
+  "kind": "memberOf",
+  "source": "c:@S@Vehicle@E@input.h@64@Car",
+  "target": "c:@S@Vehicle@E@input.h@64"
+},
+{
+  "kind": "memberOf",
+  "source": "c:@S@Vehicle@FI@type",
+  "target": "c:@S@Vehicle"
+},
+{
+  "kind": "memberOf",
+  "source": "c:@S@Vehicle@FI@information",
+  "target": "c:@S@Vehicle"
+}
+  ],
+  "symbols": [
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "keyword",
+  "spelling": "enum"
+},
+{
+  "kind": "text",
+  "spelling": ": "
+},
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:i",
+  "spelling": "unsigned int"
+}
+  ],
+  "docComment": {
+"lines": [
+  {
+"range": {
+  "end": {
+"character": 29,
+"line": 3
+  },
+  "start": {
+"character": 9,
+"line": 3
+  }
+},
+"text": "The type of vehicle."
+  }
+]
+  },
+  "identifier": {
+"interfaceLanguage": "c",
+"precise": "c:@S@Vehicle@E@input.h@64"
+  },
+  "kind": {
+"displayName": "Enumeration",
+"identifier": "c.enum"
+  },
+  "location": {
+"position": {
+  "character": 5,
+  "line": 4
+},
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"navigator": [
+  {
+"kind": "identifier",
+"spelling": "Vehicle::(anonymous)"
+  }
+],
+"title": "Vehicle::(anonymous)"
+  },
+  "pathComponents": [
+"Vehicle::(anonymous)"
+  ]
+},
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "identifier",
+  "spelling": "Bicycle"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "c",
+"precise": "c:@S@Vehicle@E@input.h@64@Bicycle"
+  },
+  "kind": {
+"displayName": "Enumeration Case",
+"identifier": "c.enum.case"
+  },
+  "location": {
+"position": {
+  "character": 9,
+  "line": 5
+},
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"navigator": [
+  {
+"kind": "identifier",
+"spelling": "Bicycle"
+  }
+],
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "Bicycle"
+  }
+],
+"title": "Bicycle"
+  },
+  "pathComponents": [
+"Vehicle::(anonymous)",
+"Bicycle"
+  ]
+},
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  

[PATCH] D135804: [clang][ExtractAPI] Ignore fully anonymous RecordDecls

[Daniel Grumberg via Phabricator via cfe-commits]

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

ExtractAPI was emitting a separate symbol for anonymous record declaration
that define the type of a member of another record declaration. Now
ExtractAPI ignores these declarations and just records the existence of
the actual member.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135804

Files:
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/test/ExtractAPI/anonymous_record_no_typedef.c

Index: clang/test/ExtractAPI/anonymous_record_no_typedef.c
===
--- /dev/null
+++ clang/test/ExtractAPI/anonymous_record_no_typedef.c
@@ -0,0 +1,396 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
+// RUN: %t/reference.output.json.in >> %t/reference.output.json
+// RUN: %clang_cc1 -extract-api -triple arm64-apple-macosx \
+// RUN:   -x c-header %t/input.h -o %t/output.json -verify
+
+// 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
+
+//--- input.h
+/// A Vehicle
+struct Vehicle {
+/// The type of vehicle.
+enum {
+Bicycle,
+Car
+} type;
+
+/// The information about the vehicle.
+struct {
+int wheels;
+char *name;
+} information;
+};
+// expected-no-diagnostics
+
+//--- 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:@S@Vehicle@E@input.h@64@Bicycle",
+  "target": "c:@S@Vehicle@E@input.h@64"
+},
+{
+  "kind": "memberOf",
+  "source": "c:@S@Vehicle@E@input.h@64@Car",
+  "target": "c:@S@Vehicle@E@input.h@64"
+},
+{
+  "kind": "memberOf",
+  "source": "c:@S@Vehicle@FI@type",
+  "target": "c:@S@Vehicle"
+},
+{
+  "kind": "memberOf",
+  "source": "c:@S@Vehicle@FI@information",
+  "target": "c:@S@Vehicle"
+}
+  ],
+  "symbols": [
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "keyword",
+  "spelling": "enum"
+},
+{
+  "kind": "text",
+  "spelling": ": "
+},
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:i",
+  "spelling": "unsigned int"
+}
+  ],
+  "docComment": {
+"lines": [
+  {
+"range": {
+  "end": {
+"character": 29,
+"line": 3
+  },
+  "start": {
+"character": 9,
+"line": 3
+  }
+},
+"text": "The type of vehicle."
+  }
+]
+  },
+  "identifier": {
+"interfaceLanguage": "c",
+"precise": "c:@S@Vehicle@E@input.h@64"
+  },
+  "kind": {
+"displayName": "Enumeration",
+"identifier": "c.enum"
+  },
+  "location": {
+"position": {
+  "character": 5,
+  "line": 4
+},
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"navigator": [
+  {
+"kind": "identifier",
+"spelling": "Vehicle::(anonymous)"
+  }
+],
+"title": "Vehicle::(anonymous)"
+  },
+  "pathComponents": [
+"Vehicle::(anonymous)"
+  ]
+},
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "identifier",
+  "spelling": "Bicycle"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "c",
+"precise": "c:@S@Vehicle@E@input.h@64@Bicycle"
+  },
+  "kind": {
+"displayName": "Enumeration Case",
+"identifier": "c.enum.case"
+  },
+  "location": {
+"position": {
+  "character": 9,
+  "line": 5
+},
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"navigator": [
+  {
+"kind": "identifier",
+"spelling": "Bicycle"
+  }
+],
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "Bicycle"
+  }
+],
+"title": 

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

[Daniel Grumberg via Phabricator via cfe-commits]

dang accepted this revision.
dang added a comment.

In D134813#3838069 , @zixuw wrote:

> 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.

As far as I know this should affect downstream consumers like Swift-DocC. I am 
happy with these changes.


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

[Daniel Grumberg via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG57c9780d60b1: [clang][ExtractAPI] Record availability 
information on all platforms (authored by dang).

Changed prior to commit:
  https://reviews.llvm.org/D130918?vs=449228=454111#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130918

Files:
  clang/include/clang/ExtractAPI/API.h
  clang/include/clang/ExtractAPI/AvailabilityInfo.h
  clang/lib/ExtractAPI/API.cpp
  clang/lib/ExtractAPI/AvailabilityInfo.cpp
  clang/lib/ExtractAPI/CMakeLists.txt
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
  clang/test/ExtractAPI/availability.c

Index: clang/test/ExtractAPI/availability.c
===
--- /dev/null
+++ clang/test/ExtractAPI/availability.c
@@ -0,0 +1,459 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
+// RUN: %t/reference.output.json.in >> %t/reference.output.json
+// RUN: %clang_cc1 -extract-api --product-name=Availability -triple arm64-apple-macosx -x c-header %t/input.h -o %t/output.json -verify
+
+// 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
+void a(void);
+
+void b(void) __attribute__((availability(macos, introduced=12.0)));
+
+void c(void) __attribute__((availability(macos, introduced=11.0, deprecated=12.0, obsoleted=20.0)));
+
+void d(void) __attribute__((availability(macos, introduced=11.0, deprecated=12.0, obsoleted=20.0))) __attribute__((availability(ios, introduced=13.0)));
+
+void e(void) __attribute__((deprecated)) __attribute__((availability(macos, introduced=11.0)));
+
+void f(void) __attribute__((unavailable)) __attribute__((availability(macos, introduced=11.0)));
+
+void d(void) __attribute__((availability(tvos, introduced=15.0)));
+///expected-no-diagnostics
+
+//--- reference.output.json.in
+{
+  "metadata": {
+"formatVersion": {
+  "major": 0,
+  "minor": 5,
+  "patch": 3
+},
+"generator": "?"
+  },
+  "module": {
+"name": "Availability",
+"platform": {
+  "architecture": "arm64",
+  "operatingSystem": {
+"minimumVersion": {
+  "major": 11,
+  "minor": 0,
+  "patch": 0
+},
+"name": "macosx"
+  },
+  "vendor": "apple"
+}
+  },
+  "relationships": [],
+  "symbols": [
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:v",
+  "spelling": "void"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "a"
+},
+{
+  "kind": "text",
+  "spelling": "()"
+}
+  ],
+  "functionSignature": {
+"returns": [
+  {
+"kind": "typeIdentifier",
+"preciseIdentifier": "c:v",
+"spelling": "void"
+  }
+]
+  },
+  "identifier": {
+"interfaceLanguage": "c",
+"precise": "c:@F@a"
+  },
+  "kind": {
+"displayName": "Function",
+"identifier": "c.func"
+  },
+  "location": {
+"position": {
+  "character": 6,
+  "line": 1
+},
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"navigator": [
+  {
+"kind": "identifier",
+"spelling": "a"
+  }
+],
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "a"
+  }
+],
+"title": "a"
+  },
+  "pathComponents": [
+"a"
+  ]
+},
+{
+  "accessLevel": "public",
+  "availability": [
+{
+  "domain": "macos",
+  "introducedVersion": {
+"major": 12,
+"minor": 0,
+"patch": 0
+  }
+}
+  ],
+  "declarationFragments": [
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:v",
+  "spelling": "void"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "b"
+},
+{
+  "kind": "text",
+  "spelling": "()"
+}
+  ],
+  "functionSignature": {
+"returns": [
+  {
+"kind": "typeIdentifier",
+"preciseIdentifier": "c:v",
+"spelling": "void"
+  }
+]
+  },
+  

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

[Daniel Grumberg via Phabricator via cfe-commits]

dang updated this revision to Diff 449228.
dang added a comment.

Remove doc comments in test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130918

Files:
  clang/include/clang/ExtractAPI/API.h
  clang/include/clang/ExtractAPI/AvailabilityInfo.h
  clang/lib/ExtractAPI/API.cpp
  clang/lib/ExtractAPI/AvailabilityInfo.cpp
  clang/lib/ExtractAPI/CMakeLists.txt
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
  clang/test/ExtractAPI/availability.c

Index: clang/test/ExtractAPI/availability.c
===
--- /dev/null
+++ clang/test/ExtractAPI/availability.c
@@ -0,0 +1,459 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
+// RUN: %t/reference.output.json.in >> %t/reference.output.json
+// RUN: %clang_cc1 -extract-api --product-name=Availability -triple arm64-apple-macosx -x c-header %t/input.h -o %t/output.json -verify
+
+// 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
+void a(void);
+
+void b(void) __attribute__((availability(macos, introduced=12.0)));
+
+void c(void) __attribute__((availability(macos, introduced=11.0, deprecated=12.0, obsoleted=20.0)));
+
+void d(void) __attribute__((availability(macos, introduced=11.0, deprecated=12.0, obsoleted=20.0))) __attribute__((availability(ios, introduced=13.0)));
+
+void e(void) __attribute__((deprecated)) __attribute__((availability(macos, introduced=11.0)));
+
+void f(void) __attribute__((unavailable)) __attribute__((availability(macos, introduced=11.0)));
+
+void d(void) __attribute__((availability(tvos, introduced=15.0)));
+///expected-no-diagnostics
+
+//--- reference.output.json.in
+{
+  "metadata": {
+"formatVersion": {
+  "major": 0,
+  "minor": 5,
+  "patch": 3
+},
+"generator": "?"
+  },
+  "module": {
+"name": "Availability",
+"platform": {
+  "architecture": "arm64",
+  "operatingSystem": {
+"minimumVersion": {
+  "major": 11,
+  "minor": 0,
+  "patch": 0
+},
+"name": "macosx"
+  },
+  "vendor": "apple"
+}
+  },
+  "relationships": [],
+  "symbols": [
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:v",
+  "spelling": "void"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "a"
+},
+{
+  "kind": "text",
+  "spelling": "()"
+}
+  ],
+  "functionSignature": {
+"returns": [
+  {
+"kind": "typeIdentifier",
+"preciseIdentifier": "c:v",
+"spelling": "void"
+  }
+]
+  },
+  "identifier": {
+"interfaceLanguage": "c",
+"precise": "c:@F@a"
+  },
+  "kind": {
+"displayName": "Function",
+"identifier": "c.func"
+  },
+  "location": {
+"position": {
+  "character": 6,
+  "line": 1
+},
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"navigator": [
+  {
+"kind": "identifier",
+"spelling": "a"
+  }
+],
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "a"
+  }
+],
+"title": "a"
+  },
+  "pathComponents": [
+"a"
+  ]
+},
+{
+  "accessLevel": "public",
+  "availability": [
+{
+  "domain": "macos",
+  "introducedVersion": {
+"major": 12,
+"minor": 0,
+"patch": 0
+  }
+}
+  ],
+  "declarationFragments": [
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:v",
+  "spelling": "void"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "b"
+},
+{
+  "kind": "text",
+  "spelling": "()"
+}
+  ],
+  "functionSignature": {
+"returns": [
+  {
+"kind": "typeIdentifier",
+"preciseIdentifier": "c:v",
+"spelling": "void"
+  }
+]
+  },
+  "identifier": {
+"interfaceLanguage": "c",
+"precise": "c:@F@b"
+  },
+  "kind": {
+"displayName": "Function",
+"identifier": "c.func"
+  },

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

[Daniel Grumberg via Phabricator via cfe-commits]

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

Currently ExtractAPI only emits availability information for the
current platform. This makes it easy for clients to get all availability
information for a given symbol in one invocation as opposed to having to invoke
clang once per-platform and then merge the symbol-graphs.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130918

Files:
  clang/include/clang/ExtractAPI/API.h
  clang/include/clang/ExtractAPI/AvailabilityInfo.h
  clang/lib/ExtractAPI/API.cpp
  clang/lib/ExtractAPI/AvailabilityInfo.cpp
  clang/lib/ExtractAPI/CMakeLists.txt
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
  clang/test/ExtractAPI/availability.c

Index: clang/test/ExtractAPI/availability.c
===
--- /dev/null
+++ clang/test/ExtractAPI/availability.c
@@ -0,0 +1,551 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
+// RUN: %t/reference.output.json.in >> %t/reference.output.json
+// RUN: %clang_cc1 -extract-api --product-name=Availability -triple arm64-apple-macosx -x c-header %t/input.h -o %t/output.json -verify
+
+// 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
+/// Documentation for a.
+void a(void);
+
+/// Documentation for b.
+void b(void) __attribute__((availability(macos, introduced=12.0)));
+
+/// Documentation for c.
+void c(void) __attribute__((availability(macos, introduced=11.0, deprecated=12.0, obsoleted=20.0)));
+
+/// Documentation for d.
+void d(void) __attribute__((availability(macos, introduced=11.0, deprecated=12.0, obsoleted=20.0))) __attribute__((availability(ios, introduced=13.0)));
+
+/// Documentation for e.
+void e(void) __attribute__((deprecated)) __attribute__((availability(macos, introduced=11.0)));
+
+/// Documentation for f.
+void f(void) __attribute__((unavailable)) __attribute__((availability(macos, introduced=11.0)));
+
+// Later declaration for d that adds availability for a new domain.
+void d(void) __attribute__((availability(tvos, introduced=15.0)));
+///expected-no-diagnostics
+
+//--- reference.output.json.in
+{
+  "metadata": {
+"formatVersion": {
+  "major": 0,
+  "minor": 5,
+  "patch": 3
+},
+"generator": "?"
+  },
+  "module": {
+"name": "Availability",
+"platform": {
+  "architecture": "arm64",
+  "operatingSystem": {
+"minimumVersion": {
+  "major": 11,
+  "minor": 0,
+  "patch": 0
+},
+"name": "macosx"
+  },
+  "vendor": "apple"
+}
+  },
+  "relationships": [],
+  "symbols": [
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:v",
+  "spelling": "void"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "a"
+},
+{
+  "kind": "text",
+  "spelling": "()"
+}
+  ],
+  "docComment": {
+"lines": [
+  {
+"range": {
+  "end": {
+"character": 25,
+"line": 1
+  },
+  "start": {
+"character": 5,
+"line": 1
+  }
+},
+"text": "Documentation for a."
+  }
+]
+  },
+  "functionSignature": {
+"returns": [
+  {
+"kind": "typeIdentifier",
+"preciseIdentifier": "c:v",
+"spelling": "void"
+  }
+]
+  },
+  "identifier": {
+"interfaceLanguage": "c",
+"precise": "c:@F@a"
+  },
+  "kind": {
+"displayName": "Function",
+"identifier": "c.func"
+  },
+  "location": {
+"position": {
+  "character": 6,
+  "line": 2
+},
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"navigator": [
+  {
+"kind": "identifier",
+"spelling": "a"
+  }
+],
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "a"
+  }
+],
+"title": "a"
+  },
+  "pathComponents": [
+"a"
+  ]
+},
+{
+  "accessLevel": "public",
+  "availability": [
+{
+ 

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

[Daniel Grumberg 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 rGd3fc779e4295: [clang][ExtractAPI] Ensure that class 
properties have a kind of Type Property (authored by dang).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130581

Files:
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
  clang/test/ExtractAPI/objc_property.m

Index: clang/test/ExtractAPI/objc_property.m
===
--- /dev/null
+++ clang/test/ExtractAPI/objc_property.m
@@ -0,0 +1,745 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
+// RUN: %t/reference.output.json.in >> %t/reference.output.json
+// RUN: %clang_cc1 -extract-api -triple arm64-apple-macosx -x objective-c-header %t/input.h -o %t/output.json -verify
+
+// 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
+
+//--- input.h
+@protocol Protocol
+@property(class) int myProtocolTypeProp;
+@property int myProtocolInstanceProp;
+@end
+
+@interface Interface
+@property(class) int myInterfaceTypeProp;
+@property int myInterfaceInstanceProp;
+@end
+
+@interface Interface (Category) 
+@property(class) int myCategoryTypeProp;
+@property int myCategoryInstanceProp;
+@end
+// expected-no-diagnostics
+
+//--- 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(cpy)myInterfaceTypeProp",
+  "target": "c:objc(cs)Interface"
+},
+{
+  "kind": "memberOf",
+  "source": "c:objc(cs)Interface(py)myInterfaceInstanceProp",
+  "target": "c:objc(cs)Interface"
+},
+{
+  "kind": "memberOf",
+  "source": "c:objc(cs)Interface(cpy)myCategoryTypeProp",
+  "target": "c:objc(cs)Interface"
+},
+{
+  "kind": "memberOf",
+  "source": "c:objc(cs)Interface(py)myCategoryInstanceProp",
+  "target": "c:objc(cs)Interface"
+},
+{
+  "kind": "conformsTo",
+  "source": "c:objc(cs)Interface",
+  "target": "c:objc(pl)Protocol"
+},
+{
+  "kind": "memberOf",
+  "source": "c:objc(pl)Protocol(cpy)myProtocolTypeProp",
+  "target": "c:objc(pl)Protocol"
+},
+{
+  "kind": "memberOf",
+  "source": "c:objc(pl)Protocol(py)myProtocolInstanceProp",
+  "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": 6
+},
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"navigator": [
+  {
+"kind": "identifier",
+"spelling": "Interface"
+  }
+],
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "Interface"
+  }
+],
+"title": "Interface"
+  },
+  "pathComponents": [
+"Interface"
+  ]
+},
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "keyword",
+  "spelling": "@property"
+},
+{
+  "kind": "text",
+  "spelling": " ("
+},
+{
+  "kind": "keyword",
+  "spelling": "class"
+},
+{
+  "kind": "text",
+  "spelling": ", "
+},
+{
+  "kind": "keyword",
+  "spelling": "atomic"
+},
+{
+  "kind": "text",
+  "spelling": ", "
+},
+{
+  "kind": "keyword",
+  "spelling": "assign"
+},
+{
+  "kind": "text",
+  "spelling": ", "
+},
+{
+  

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

[Daniel Grumberg 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 rG7f0387de4c60: [clang][ExtractAPI] Add a space between type 
and name in property declaration… (authored by dang).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130583

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
@@ -462,6 +462,10 @@
   "preciseIdentifier": "c:i",
   "spelling": "unsigned int"
 },
+{
+  "kind": "text",
+  "spelling": " "
+},
 {
   "kind": "identifier",
   "spelling": "Property"
Index: clang/test/ExtractAPI/objc_category.m
===
--- clang/test/ExtractAPI/objc_category.m
+++ clang/test/ExtractAPI/objc_category.m
@@ -317,6 +317,10 @@
   "preciseIdentifier": "c:I",
   "spelling": "int"
 },
+{
+  "kind": "text",
+  "spelling": " "
+},
 {
   "kind": "identifier",
   "spelling": "Property"
Index: clang/lib/ExtractAPI/DeclarationFragments.cpp
===
--- clang/lib/ExtractAPI/DeclarationFragments.cpp
+++ clang/lib/ExtractAPI/DeclarationFragments.cpp
@@ -692,6 +692,7 @@
   return Fragments.appendSpace()
   .append(getFragmentsForType(Property->getType(),
   Property->getASTContext(), After))
+  .appendSpace()
   .append(Property->getName(),
   DeclarationFragments::FragmentKind::Identifier)
   .append(std::move(After));


Index: clang/test/ExtractAPI/objc_interface.m
===
--- clang/test/ExtractAPI/objc_interface.m
+++ clang/test/ExtractAPI/objc_interface.m
@@ -462,6 +462,10 @@
   "preciseIdentifier": "c:i",
   "spelling": "unsigned int"
 },
+{
+  "kind": "text",
+  "spelling": " "
+},
 {
   "kind": "identifier",
   "spelling": "Property"
Index: clang/test/ExtractAPI/objc_category.m
===
--- clang/test/ExtractAPI/objc_category.m
+++ clang/test/ExtractAPI/objc_category.m
@@ -317,6 +317,10 @@
   "preciseIdentifier": "c:I",
   "spelling": "int"
 },
+{
+  "kind": "text",
+  "spelling": " "
+},
 {
   "kind": "identifier",
   "spelling": "Property"
Index: clang/lib/ExtractAPI/DeclarationFragments.cpp
===
--- clang/lib/ExtractAPI/DeclarationFragments.cpp
+++ clang/lib/ExtractAPI/DeclarationFragments.cpp
@@ -692,6 +692,7 @@
   return Fragments.appendSpace()
   .append(getFragmentsForType(Property->getType(),
   Property->getASTContext(), After))
+  .appendSpace()
   .append(Property->getName(),
   DeclarationFragments::FragmentKind::Identifier)
   .append(std::move(After));
___
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

[Daniel Grumberg via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130583

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
@@ -462,6 +462,10 @@
   "preciseIdentifier": "c:i",
   "spelling": "unsigned int"
 },
+{
+  "kind": "text",
+  "spelling": " "
+},
 {
   "kind": "identifier",
   "spelling": "Property"
Index: clang/test/ExtractAPI/objc_category.m
===
--- clang/test/ExtractAPI/objc_category.m
+++ clang/test/ExtractAPI/objc_category.m
@@ -317,6 +317,10 @@
   "preciseIdentifier": "c:I",
   "spelling": "int"
 },
+{
+  "kind": "text",
+  "spelling": " "
+},
 {
   "kind": "identifier",
   "spelling": "Property"
Index: clang/lib/ExtractAPI/DeclarationFragments.cpp
===
--- clang/lib/ExtractAPI/DeclarationFragments.cpp
+++ clang/lib/ExtractAPI/DeclarationFragments.cpp
@@ -692,6 +692,7 @@
   return Fragments.appendSpace()
   .append(getFragmentsForType(Property->getType(),
   Property->getASTContext(), After))
+  .appendSpace()
   .append(Property->getName(),
   DeclarationFragments::FragmentKind::Identifier)
   .append(std::move(After));


Index: clang/test/ExtractAPI/objc_interface.m
===
--- clang/test/ExtractAPI/objc_interface.m
+++ clang/test/ExtractAPI/objc_interface.m
@@ -462,6 +462,10 @@
   "preciseIdentifier": "c:i",
   "spelling": "unsigned int"
 },
+{
+  "kind": "text",
+  "spelling": " "
+},
 {
   "kind": "identifier",
   "spelling": "Property"
Index: clang/test/ExtractAPI/objc_category.m
===
--- clang/test/ExtractAPI/objc_category.m
+++ clang/test/ExtractAPI/objc_category.m
@@ -317,6 +317,10 @@
   "preciseIdentifier": "c:I",
   "spelling": "int"
 },
+{
+  "kind": "text",
+  "spelling": " "
+},
 {
   "kind": "identifier",
   "spelling": "Property"
Index: clang/lib/ExtractAPI/DeclarationFragments.cpp
===
--- clang/lib/ExtractAPI/DeclarationFragments.cpp
+++ clang/lib/ExtractAPI/DeclarationFragments.cpp
@@ -692,6 +692,7 @@
   return Fragments.appendSpace()
   .append(getFragmentsForType(Property->getType(),
   Property->getASTContext(), After))
+  .appendSpace()
   .append(Property->getName(),
   DeclarationFragments::FragmentKind::Identifier)
   .append(std::move(After));
___
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"

[Daniel Grumberg via Phabricator via cfe-commits]

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

Generated symbol graphs should distinguish between type properties and instance
properties.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130581

Files:
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
  clang/test/ExtractAPI/objc_property.m

Index: clang/test/ExtractAPI/objc_property.m
===
--- /dev/null
+++ clang/test/ExtractAPI/objc_property.m
@@ -0,0 +1,745 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
+// RUN: %t/reference.output.json.in >> %t/reference.output.json
+// RUN: %clang_cc1 -extract-api -triple arm64-apple-macosx -x objective-c-header %t/input.h -o %t/output.json -verify
+
+// 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
+
+//--- input.h
+@protocol Protocol
+@property(class) int myProtocolTypeProp;
+@property int myProtocolInstanceProp;
+@end
+
+@interface Interface
+@property(class) int myInterfaceTypeProp;
+@property int myInterfaceInstanceProp;
+@end
+
+@interface Interface (Category) 
+@property(class) int myCategoryTypeProp;
+@property int myCategoryInstanceProp;
+@end
+// expected-no-diagnostics
+
+//--- 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(cpy)myInterfaceTypeProp",
+  "target": "c:objc(cs)Interface"
+},
+{
+  "kind": "memberOf",
+  "source": "c:objc(cs)Interface(py)myInterfaceInstanceProp",
+  "target": "c:objc(cs)Interface"
+},
+{
+  "kind": "memberOf",
+  "source": "c:objc(cs)Interface(cpy)myCategoryTypeProp",
+  "target": "c:objc(cs)Interface"
+},
+{
+  "kind": "memberOf",
+  "source": "c:objc(cs)Interface(py)myCategoryInstanceProp",
+  "target": "c:objc(cs)Interface"
+},
+{
+  "kind": "conformsTo",
+  "source": "c:objc(cs)Interface",
+  "target": "c:objc(pl)Protocol"
+},
+{
+  "kind": "memberOf",
+  "source": "c:objc(pl)Protocol(cpy)myProtocolTypeProp",
+  "target": "c:objc(pl)Protocol"
+},
+{
+  "kind": "memberOf",
+  "source": "c:objc(pl)Protocol(py)myProtocolInstanceProp",
+  "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": 6
+},
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"navigator": [
+  {
+"kind": "identifier",
+"spelling": "Interface"
+  }
+],
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "Interface"
+  }
+],
+"title": "Interface"
+  },
+  "pathComponents": [
+"Interface"
+  ]
+},
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "keyword",
+  "spelling": "@property"
+},
+{
+  "kind": "text",
+  "spelling": " ("
+},
+{
+  "kind": "keyword",
+  "spelling": "class"
+},
+{
+  "kind": "text",
+  "spelling": ", "
+},
+{
+  "kind": "keyword",
+  "spelling": "atomic"
+},
+{
+  "kind": "text",
+  "spelling": ", "
+},
+{
+  "kind": "keyword",
+  "spelling": "assign"
+},
+{
+  "kind": "text",
+  "spelling": ", "
+},
+{

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

[Daniel Grumberg 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 rG504736cedff3: [clang][extract-api] Dont emit symbols 
prefixed with an underscore (authored by dang).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125678

Files:
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
  clang/test/ExtractAPI/underscored.c

Index: clang/test/ExtractAPI/underscored.c
===
--- /dev/null
+++ clang/test/ExtractAPI/underscored.c
@@ -0,0 +1,396 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
+// RUN: %t/reference.output.json.in >> %t/reference.output.json
+// RUN: %clang_cc1 -extract-api -triple arm64-apple-macosx \
+// RUN:   -x c-header %t/input.h -o %t/output.json -verify
+
+// 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
+
+//--- input.h
+// expected-no-diagnostics
+
+// Global record
+int _HiddenGlobal;
+int exposed_global;
+
+// Record type
+struct _HiddenRecord {
+  int a;
+};
+
+struct ExposedRecord {
+  int a;
+};
+
+// Typedef
+typedef struct {} _HiddenTypedef;
+typedef int ExposedTypedef;
+typedef _HiddenTypedef ExposedTypedefToHidden;
+
+// Macros
+#define _HIDDEN_MACRO 5
+#define EXPOSED_MACRO 5
+
+// Symbols that start with '_' should not appear in the reference output
+//--- 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:@S@ExposedRecord@FI@a",
+  "target": "c:@S@ExposedRecord"
+}
+  ],
+  "symbols": [
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:I",
+  "spelling": "int"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "exposed_global"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "c",
+"precise": "c:@exposed_global"
+  },
+  "kind": {
+"displayName": "Global Variable",
+"identifier": "c.var"
+  },
+  "location": {
+"position": {
+  "character": 5,
+  "line": 5
+},
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"navigator": [
+  {
+"kind": "identifier",
+"spelling": "exposed_global"
+  }
+],
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "exposed_global"
+  }
+],
+"title": "exposed_global"
+  },
+  "pathComponents": [
+"exposed_global"
+  ]
+},
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "keyword",
+  "spelling": "struct"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "ExposedRecord"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "c",
+"precise": "c:@S@ExposedRecord"
+  },
+  "kind": {
+"displayName": "Structure",
+"identifier": "c.struct"
+  },
+  "location": {
+"position": {
+  "character": 8,
+  "line": 12
+},
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"navigator": [
+  {
+"kind": "identifier",
+"spelling": "ExposedRecord"
+  }
+],
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "ExposedRecord"
+  }
+],
+"title": "ExposedRecord"
+  },
+  "pathComponents": [
+"ExposedRecord"
+  ]
+},
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:I",
+  "spelling": "int"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "a"
+}
+  ],
+  "identifier": {
+ 

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

[Daniel Grumberg via Phabricator via cfe-commits]

dang added a comment.

In D125678#3517174 , @zixuw wrote:

> 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.

Yeah this always unfortunately happens can't expect clang format to seamlessly 
accept to format both C and JSON. Also formatting matters less in tests.


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] D125678: [clang][extract-api] Don't emit symbols prefixed with an underscore

[Daniel Grumberg via Phabricator via cfe-commits]

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

These symbols are understood to not be used for client API consumption
by convention so they should not appear in the generated symbol graph.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125678

Files:
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
  clang/test/ExtractAPI/underscored.c

Index: clang/test/ExtractAPI/underscored.c
===
--- /dev/null
+++ clang/test/ExtractAPI/underscored.c
@@ -0,0 +1,396 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
+// RUN: %t/reference.output.json.in >> %t/reference.output.json
+// RUN: %clang_cc1 -extract-api -triple arm64-apple-macosx \
+// RUN:   -x c-header %t/input.h -o %t/output.json -verify
+
+// 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
+
+//--- input.h
+// expected-no-diagnostics
+
+// Global record
+int _HiddenGlobal;
+int exposed_global;
+
+// Record type
+struct _HiddenRecord {
+  int a;
+};
+
+struct ExposedRecord {
+  int a;
+};
+
+// Typedef
+typedef struct {} _HiddenTypedef;
+typedef int ExposedTypedef;
+typedef _HiddenTypedef ExposedTypedefToHidden;
+
+// Macros
+#define _HIDDEN_MACRO 5
+#define EXPOSED_MACRO 5
+
+// Symbols that start with '_' should not appear in the reference output
+//--- 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:@S@ExposedRecord@FI@a",
+  "target": "c:@S@ExposedRecord"
+}
+  ],
+  "symbols": [
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:I",
+  "spelling": "int"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "exposed_global"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "c",
+"precise": "c:@exposed_global"
+  },
+  "kind": {
+"displayName": "Global Variable",
+"identifier": "c.var"
+  },
+  "location": {
+"position": {
+  "character": 5,
+  "line": 5
+},
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"navigator": [
+  {
+"kind": "identifier",
+"spelling": "exposed_global"
+  }
+],
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "exposed_global"
+  }
+],
+"title": "exposed_global"
+  },
+  "pathComponents": [
+"exposed_global"
+  ]
+},
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "keyword",
+  "spelling": "struct"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "ExposedRecord"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "c",
+"precise": "c:@S@ExposedRecord"
+  },
+  "kind": {
+"displayName": "Structure",
+"identifier": "c.struct"
+  },
+  "location": {
+"position": {
+  "character": 8,
+  "line": 12
+},
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"navigator": [
+  {
+"kind": "identifier",
+"spelling": "ExposedRecord"
+  }
+],
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "ExposedRecord"
+  }
+],
+"title": "ExposedRecord"
+  },
+  "pathComponents": [
+"ExposedRecord"
+  ]
+},
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:I",
+  "spelling": "int"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "a"

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

[Daniel Grumberg via Phabricator via cfe-commits]

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

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?


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] D123831: [clang][extract-api] Use relative includes

[Daniel Grumberg via Phabricator via cfe-commits]

dang added a comment.

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


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

[Daniel Grumberg via Phabricator via cfe-commits]

dang accepted this revision.
dang added a comment.

LGTM


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

[Daniel Grumberg via Phabricator via cfe-commits]

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

Minor comment LGTM otherwise




Comment at: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:203
+// Try to reduce the include name the same way we tried to include it.
+if (auto IncludeName = getRelativeIncludeName(CI, FileName))
+  if (llvm::find(KnownFiles, *IncludeName) != KnownFiles.end()) {

Should we be tracking if the include was quoted or not? In principle, that 
could mean a different redirect in header search and therefore potentially 
mapping to different content.


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] D124634: ExtractAPI: Use %clang_cc1 and -verify in enum.c

[Daniel Grumberg via Phabricator via cfe-commits]

dang accepted this revision.
dang 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/D124634/new/

https://reviews.llvm.org/D124634

___
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

[Daniel Grumberg via Phabricator via cfe-commits]

dang added inline comments.



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

zixuw wrote:
> 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.
Two things:
- If we want an exhaustive search, then it would make sense to do what would 
actually happen in a compilation. Iterate forward and find all matches, then 
iterate forwards again with each of the matches from the previous step until we 
find all terminal matches and then heuristically pick the "best one" probably 
the shortest one.
- The "correct" way of including a header would certainly be `#include 
` for a Darwin framework. Nonetheless if the search paths and 
headermaps setup means that `#include "Header.h"` or `#include ` is a 
possible way of getting to the same file I see no harm in doing that. As long 
as shortening a given file results in deterministic results it should work 
fine. I think it would be a user error if shortening a file path to say 
`` meant that including it would lead to a completely different file 
(that is with different declarations/definitions).



Comment at: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:908
+if (auto RelativeName = getRelativeIncludeName(CI, FilePath)) {
+  HeaderContents += " <";
+  HeaderContents += *RelativeName;

I think `getRelativeIncludeName` is slightly wrong. It doesn't seem like it 
accounts for matches specifically made with `-iquote` arguments. My 
understanding is that we should record that information at that point and 
include the file using the appropriate form `#include "RelativeName"` or 
`#include `. It would probably be best if 
`getRelativeIncludeName` included the quote type in the string directly.



Comment at: clang/test/ExtractAPI/known_files_only_hmap.c:1
+// XFAIL: *
 // RUN: rm -rf %t

I think you can safely delete this test.


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

[Daniel Grumberg via Phabricator via cfe-commits]

dang added a comment.

In D123831#3459368 , @ributzka wrote:

> In D123831#3458774 , @dang wrote:
>
>> In D123831#3455048 , @cishida 
>> wrote:
>>
 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?
>>>
>>> I think, in most cases, this shouldn't matter because if the header path 
>>> input doesn't match the location stored in the header map, they should 
>>> still have the same source content. The same should be true with header 
>>> search resolution with modules & vfsoverlay
>>
>> Agreed, I think it would be classified as a user error to remap to different 
>> source content via headermap.
>
> This is happening today in Xcode. Headers that are mapped from the DSTROOT 
> back to the SRCROOT can be different, because they are not simply copied. 
> Xcode also runs unifdef on them.

What do you mean? The headers in the DSTROOT don't have the ifdef? Either way, 
this shouldn't matter because the declarations we are processing would be the 
same across both versions of the header?


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

[Daniel Grumberg via Phabricator via cfe-commits]

dang added a comment.

In D123831#3455048 , @cishida wrote:

>> 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?
>
> I think, in most cases, this shouldn't matter because if the header path 
> input doesn't match the location stored in the header map, they should still 
> have the same source content. The same should be true with header search 
> resolution with modules & vfsoverlay

Agreed, I think it would be classified as a user error to remap to different 
source content via headermap.


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

[Daniel Grumberg via Phabricator via cfe-commits]

dang added inline comments.



Comment at: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:65
+  // Matches framework include patterns
+  const llvm::Regex Rule("/(.+)\\.framework/(.+)?Headers/(.+)");
+  StringRef WorkingDir = CI.getFileSystemOpts().WorkingDir;

This does rely on the path separator being "/". If stick with this regex, do we 
need to convert all paths to POSIX format? I think the best thing to do is to 
iterate through the given path components and match for just "(.+)\\.framework" 
to match just the framework name and the use the base name of the file directly 
instead of matching it via the regex as well...



Comment at: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:185-186
+ CI.getPreprocessor().getHeaderSearchInfo().search_dir_range()) {
+  if (!DL.isHeaderMap())
+continue;
+

Not sure that just accounting for the header map case is the correct thing to 
do here. Ideally we would like to know what the include string was, e.g. 
 and match that with how we included one of the 
original files. This would account for all remapping functionality.


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