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<CXXMethodRecord> : 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 <typename FunctionT>
+ 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<CXXDestructorDecl>(Method))
+ // TODO: add '~'
+ Name = cast<CXXRecordDecl>(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
