dang added inline comments.
================
Comment at: clang/include/clang/ExtractAPI/API.h:34
+/// Documentation comment is a vector of RawComment::CommentLine.
+///
----------------
You should use the actual name of the type or "This"
================
Comment at: clang/include/clang/ExtractAPI/API.h:37
+/// Each line represents one line of striped documentation comment,
+/// with source range information. This could simplify calculating the source
+/// location of a character in the doc comment for pointing back to the source
----------------
If it wouldn't make things simpler we wouldn't have it. IMHO it is better to
use definitive terms in documentation. here it would "This simplifies
calculating [...]"
================
Comment at: clang/include/clang/ExtractAPI/API.h:103
+/// GlobalRecord holds information of a global variable or a function.
struct GlobalRecord : APIRecord {
----------------
Nit: instead of "of a" maybe use "associated with"
================
Comment at: clang/include/clang/ExtractAPI/API.h:123
+private:
+ virtual void anchor();
};
----------------
This is new so I don't think it belongs in this patch.
================
Comment at: clang/include/clang/ExtractAPI/API.h:129
public:
- APISet(const llvm::Triple &Target, const LangOptions &LangOpts)
- : Target(Target), LangOpts(LangOpts) {}
-
- const llvm::Triple &getTarget() const { return Target; }
- const LangOptions &getLangOpts() const { return LangOpts; }
-
+ /// Create and add a GlobalRecord of kind \p Kind into the API set.
GlobalRecord *addGlobal(GVKind Kind, StringRef Name, StringRef USR,
----------------
why is stuff reordered in the patch?
================
Comment at: clang/include/clang/ExtractAPI/DeclarationFragments.h:108
+ ///
+ /// A helper function to quicking move the fragments in another
+ /// DeclarationFragments and concatenate.
----------------
Not sure an explanation of the process is needed. Anyway "quicking" is not a
word ;) I would suggest "Other is moved from and can not be used after a call
to this method".
================
Comment at: clang/include/clang/ExtractAPI/Serialization/SerializerBase.h:33
+/// The base interface of JSON serializers for API information.
+class APIJSONSerializer {
+public:
----------------
If we are going for a base class, I am not sure that the base should be JSON
specific.
================
Comment at:
clang/include/clang/ExtractAPI/Serialization/SymbolGraphSerializer.h:34
+class SymbolGraphSerializer : public APIJSONSerializer {
+ virtual void anchor();
+
----------------
Not sure why we need this.
================
Comment at: clang/lib/ExtractAPI/API.cpp:34
if (Result.second) {
+ // Create the record if not already exist.
auto Record = APIRecordUniquePtr<GlobalRecord>(new (Allocator)
GlobalRecord{
----------------
"Create the record if it does not already exist."
================
Comment at: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:195
+ // the Symbol Graph format.
+ SymbolGraphSerializer SGSerializer(Visitor.getAPI());
+ SGSerializer.serialize(*OS);
----------------
The kind of serializer should be made configurable somehow.
================
Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:26
-namespace {
+namespace { // Helper functions.
----------------
This is unnecessary imho.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122160/new/
https://reviews.llvm.org/D122160
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
