hidl-gen: understand doc comments
Doc comments were used by hidl-doc in order to
generate documentation. However, because hidl-doc
tokenizes the entire file including the comments
some of the information is lost. Rather than refactor
hidl-doc, we are going to use 3rd party doc generation
tools in order to generate docs from the output of
hidl-gen.
This has a couple of benefits:
- people automatically get the documentation if they are using
an IDE which pulls the documentation
- hidl-doc/hidl-gen won't get out of sync
- documentation will be closer to actual usage
This will require ABI-safe changes to some HAL interfaces.
Bug: 78135149
Test: manually inspect hidl-gen output.
Test: (sanity) run_all_host_tests.sh
Test: (sanity) run_all_device_tests.sh
Change-Id: I9a09ed48e2e3834fab7e032e29c48f466510e51f
diff --git a/Android.bp b/Android.bp
index d1e9c5a..dd51705 100644
--- a/Android.bp
+++ b/Android.bp
@@ -78,6 +78,7 @@
"CompoundType.cpp",
"ConstantExpression.cpp",
"DeathRecipientType.cpp",
+ "DocComment.cpp",
"EnumType.cpp",
"HandleType.cpp",
"HidlTypeAssertion.cpp",
diff --git a/CompoundType.cpp b/CompoundType.cpp
index fc3c27d..87f0f14 100644
--- a/CompoundType.cpp
+++ b/CompoundType.cpp
@@ -386,6 +386,7 @@
if (containsPointer()) {
for (const auto &field : *mFields) {
+ field->emitDocComment(out);
out << field->type().getCppStackType()
<< " "
<< field->name()
@@ -577,7 +578,9 @@
Scope::emitJavaTypeDeclarations(out, false /* atTopLevel */);
- for (const auto &field : *mFields) {
+ for (const auto& field : *mFields) {
+ field->emitDocComment(out);
+
out << "public ";
field->type().emitJavaFieldInitializer(out, field->name());
diff --git a/DocComment.cpp b/DocComment.cpp
new file mode 100644
index 0000000..91bc676
--- /dev/null
+++ b/DocComment.cpp
@@ -0,0 +1,71 @@
+/*
+ * Copyright (C) 2018 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "DocComment.h"
+
+#include <hidl-util/StringHelper.h>
+
+#include <cctype>
+#include <sstream>
+
+namespace android {
+
+DocComment::DocComment(const std::string& comment) {
+ std::vector<std::string> lines;
+ StringHelper::SplitString(comment, '\n', &lines);
+
+ bool foundFirstLine = false;
+
+ std::ostringstream is;
+ for (size_t l = 0; l < lines.size(); l++) {
+ const std::string& line = lines[l];
+
+ // Delete prefixes like " * ", " *", or " ".
+ size_t idx = 0;
+ for (; idx < line.size() && isspace(line[idx]); idx++)
+ ;
+ if (idx < line.size() && line[idx] == '*') idx++;
+ if (idx < line.size() && line[idx] == ' ') idx++;
+
+ if (idx < line.size()) {
+ foundFirstLine = true;
+ }
+
+ if (!foundFirstLine) continue;
+
+ is << line.substr(idx);
+
+ if (l + 1 < lines.size()) {
+ is << "\n";
+ }
+ }
+
+ mComment = is.str();
+}
+
+void DocComment::merge(const DocComment* comment) {
+ mComment = mComment + "\n\n" + comment->mComment;
+}
+
+void DocComment::emit(Formatter& out) const {
+ out << "/**\n";
+ out.setLinePrefix(" * ");
+ out << mComment;
+ out.unsetLinePrefix();
+ out << " */\n";
+}
+
+} // namespace android
diff --git a/DocComment.h b/DocComment.h
new file mode 100644
index 0000000..e0677eb
--- /dev/null
+++ b/DocComment.h
@@ -0,0 +1,52 @@
+/*
+ * Copyright (C) 2018 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef DOC_COMMENT_H_
+
+#define DOC_COMMENT_H_
+
+#include <hidl-util/Formatter.h>
+
+#include <string>
+
+namespace android {
+
+struct DocComment {
+ DocComment(const std::string& comment);
+
+ void merge(const DocComment* comment);
+
+ void emit(Formatter& out) const;
+
+ private:
+ std::string mComment;
+};
+
+struct DocCommentable {
+ void setDocComment(const DocComment* docComment) { mDocComment = docComment; }
+ void emitDocComment(Formatter& out) const {
+ if (mDocComment != nullptr) {
+ mDocComment->emit(out);
+ }
+ }
+
+ private:
+ const DocComment* mDocComment = nullptr;
+};
+
+} // namespace android
+
+#endif // DOC_COMMENT_H_
diff --git a/EnumType.cpp b/EnumType.cpp
index 1984693..ead64ba 100644
--- a/EnumType.cpp
+++ b/EnumType.cpp
@@ -254,6 +254,8 @@
const auto &type = *it;
for (const auto &entry : type->values()) {
+ entry->emitDocComment(out);
+
out << entry->name();
std::string value = entry->cppValue(scalarType->getKind());
@@ -472,6 +474,8 @@
const auto &type = *it;
for (const auto &entry : type->values()) {
+ entry->emitDocComment(out);
+
out << "public static final "
<< typeName
<< " "
diff --git a/EnumType.h b/EnumType.h
index d219506..eef4eb2 100644
--- a/EnumType.h
+++ b/EnumType.h
@@ -137,7 +137,7 @@
DISALLOW_COPY_AND_ASSIGN(EnumType);
};
-struct EnumValue : public LocalIdentifier {
+struct EnumValue : public LocalIdentifier, DocCommentable {
EnumValue(const char* name, ConstantExpression* value, const Location& location);
std::string name() const;
diff --git a/Method.h b/Method.h
index 291ea34..711f087 100644
--- a/Method.h
+++ b/Method.h
@@ -28,6 +28,7 @@
#include <unordered_set>
#include <vector>
+#include "DocComment.h"
#include "Location.h"
#include "Reference.h"
@@ -50,7 +51,7 @@
using MethodImpl = std::map<MethodImplType, std::function<void(Formatter &)>>;
-struct Method {
+struct Method : DocCommentable {
Method(const char* name, std::vector<NamedReference<Type>*>* args,
std::vector<NamedReference<Type>*>* results, bool oneway,
std::vector<Annotation*>* annotations, const Location& location);
diff --git a/Reference.h b/Reference.h
index 4b03c15..7ddc889 100644
--- a/Reference.h
+++ b/Reference.h
@@ -21,6 +21,7 @@
#include <android-base/logging.h>
#include <hidl-util/FQName.h>
+#include "DocComment.h"
#include "Location.h"
namespace android {
@@ -120,7 +121,7 @@
};
template <class T>
-struct NamedReference : public Reference<T> {
+struct NamedReference : public Reference<T>, DocCommentable {
NamedReference(const std::string& name, const Reference<T>& reference, const Location& location)
: Reference<T>(reference, location), mName(name) {}
diff --git a/Scope.cpp b/Scope.cpp
index f2cd80c..bff097f 100644
--- a/Scope.cpp
+++ b/Scope.cpp
@@ -160,6 +160,7 @@
}
for (const Type* type : mTypes) {
+ type->emitDocComment(out);
type->emitTypeDeclarations(out);
}
}
@@ -188,6 +189,7 @@
}
for (const Type* type : mTypes) {
+ type->emitDocComment(out);
type->emitJavaTypeDeclarations(out, atTopLevel);
}
}
diff --git a/Type.h b/Type.h
index b1b97f4..a0ed5bc 100644
--- a/Type.h
+++ b/Type.h
@@ -26,6 +26,7 @@
#include <unordered_set>
#include <vector>
+#include "DocComment.h"
#include "Reference.h"
namespace android {
@@ -36,7 +37,7 @@
struct ScalarType;
struct Scope;
-struct Type {
+struct Type : DocCommentable {
Type(Scope* parent);
virtual ~Type();
diff --git a/c2hal/test/simple.h b/c2hal/test/simple.h
index e3d7f3e..701ebe4 100644
--- a/c2hal/test/simple.h
+++ b/c2hal/test/simple.h
@@ -32,7 +32,7 @@
/* Simple example */
typedef struct simple_t {
- /**
+ /*
* Common methods of the simple device.
*/
struct hw_device_t common;
@@ -67,7 +67,7 @@
} simple_location_t;
-/** convenience API for coloring */
+/* convenience API for coloring */
static inline int showColor(const struct hw_module_t* module,
struct simple_t** device) {
diff --git a/c2hal/test/test.h b/c2hal/test/test.h
index b1a0058..3a83b0c 100644
--- a/c2hal/test/test.h
+++ b/c2hal/test/test.h
@@ -34,7 +34,6 @@
get rid of magic numbers */
/* test */
-/** test */
/* test **/
/* test / ** ** / test */
/* test //// ***** test /****/
@@ -91,7 +90,6 @@
#undef ONE
-/*****************************************************************************/
typedef void (*no_arg_fun)(void);
typedef int (*other_fun)(int j);
@@ -110,7 +108,7 @@
int (*global_fun_2)(struct framebuffer_device_t* dev, int enable);
typedef struct framebuffer_device_t {
- /**
+ /*
* Common methods of the framebuffer device.
*/
struct hw_device_t common;
@@ -324,7 +322,7 @@
float c;
} that_t;
-/**
+/*
* return the frame size (number of bytes per sample) of an output stream.
*/
static inline size_t audio_stream_out_frame_size(const struct audio_stream_out *s)
diff --git a/generateCpp.cpp b/generateCpp.cpp
index 93bfef4..4fd3956 100644
--- a/generateCpp.cpp
+++ b/generateCpp.cpp
@@ -288,6 +288,8 @@
method->dumpAnnotations(out);
+ method->emitDocComment(out);
+
if (elidedReturn) {
out << "virtual ::android::hardware::Return<";
out << elidedReturn->type().getCppResultType() << "> ";
diff --git a/generateJava.cpp b/generateJava.cpp
index 531211d..bdbe25a 100644
--- a/generateJava.cpp
+++ b/generateJava.cpp
@@ -237,6 +237,8 @@
out << "}\n\n";
}
+ method->emitDocComment(out);
+
if (returnsValue && !needsCallback) {
out << method->results()[0]->type().getJavaType();
} else {
diff --git a/hidl-gen_l.ll b/hidl-gen_l.ll
index 40c03b2..c94df01 100644
--- a/hidl-gen_l.ll
+++ b/hidl-gen_l.ll
@@ -35,6 +35,7 @@
#include "CompoundType.h"
#include "ConstantExpression.h"
#include "DeathRecipientType.h"
+#include "DocComment.h"
#include "EnumType.h"
#include "HandleType.h"
#include "MemoryType.h"
@@ -54,6 +55,8 @@
using namespace android;
using token = yy::parser::token;
+static std::string gCurrentComment;
+
#define SCALAR_TYPE(kind) \
{ \
yylval->type = new ScalarType(ScalarType::kind, *scope); \
@@ -81,13 +84,24 @@
%option bison-locations
%x COMMENT_STATE
+%x DOC_COMMENT_STATE
%%
-"/*" { BEGIN(COMMENT_STATE); }
-<COMMENT_STATE>"*/" { BEGIN(INITIAL); }
-<COMMENT_STATE>[\n] { yylloc->lines(); }
-<COMMENT_STATE>. { }
+"/**" { gCurrentComment.clear(); BEGIN(DOC_COMMENT_STATE); }
+<DOC_COMMENT_STATE>"*/" {
+ BEGIN(INITIAL);
+ yylval->docComment = new DocComment(gCurrentComment);
+ return token::DOC_COMMENT;
+ }
+<DOC_COMMENT_STATE>[^*\n]* { gCurrentComment += yytext; }
+<DOC_COMMENT_STATE>[\n] { gCurrentComment += yytext; yylloc->lines(); }
+<DOC_COMMENT_STATE>[*] { gCurrentComment += yytext; }
+
+"/*" { BEGIN(COMMENT_STATE); }
+<COMMENT_STATE>"*/" { BEGIN(INITIAL); }
+<COMMENT_STATE>[\n] { yylloc->lines(); }
+<COMMENT_STATE>. { }
"//"[^\r\n]* { /* skip C++ style comment */ }
diff --git a/hidl-gen_y.yy b/hidl-gen_y.yy
index 005aa59..da45823 100644
--- a/hidl-gen_y.yy
+++ b/hidl-gen_y.yy
@@ -21,6 +21,7 @@
#include "ArrayType.h"
#include "CompoundType.h"
#include "ConstantExpression.h"
+#include "DocComment.h"
#include "EnumType.h"
#include "Interface.h"
#include "Location.h"
@@ -214,6 +215,10 @@
%expect-rr 0
%error-verbose
+%debug
+
+%token<docComment> DOC_COMMENT "doc comment"
+
%token<void> ENUM "keyword `enum`"
%token<void> EXTENDS "keyword `extends`"
%token<str> FQNAME "fully-qualified name"
@@ -257,6 +262,8 @@
/* Precedence level 3, RTL; but we have to use %left here */
%left UNARY_MINUS UNARY_PLUS '!' '~'
+%type<docComment> doc_comments
+
%type<str> error_stmt error
%type<str> package
%type<fqName> fqname
@@ -267,18 +274,19 @@
%type<referenceToType> array_type_base
%type<arrayType> array_type
%type<referenceToType> opt_extends
-%type<type> type_declaration type_declaration_body interface_declaration typedef_declaration
+%type<type> type_declaration commentable_type_declaration type_declaration_body
+%type<type> interface_declaration typedef_declaration
%type<type> named_struct_or_union_declaration named_enum_declaration
%type<type> compound_declaration annotated_compound_declaration
-%type<field> field_declaration
+%type<field> field_declaration commentable_field_declaration
%type<fields> field_declarations struct_or_union_body
%type<constantExpression> const_expr
-%type<enumValue> enum_value
+%type<enumValue> enum_value commentable_enum_value
%type<enumValues> enum_values enum_declaration_body
%type<typedVars> typed_vars
%type<typedVar> typed_var
-%type<method> method_declaration
+%type<method> method_declaration commentable_method_declaration
%type<compoundStyle> struct_or_union_keyword
%type<stringVec> annotation_string_values annotation_string_value
%type<constExprVec> annotation_const_expr_values annotation_const_expr_value
@@ -312,14 +320,24 @@
android::AnnotationParamVector *annotationParams;
android::Annotation *annotation;
std::vector<android::Annotation *> *annotations;
+ android::DocComment* docComment;
}
%%
program
- : package
- imports
- type_declarations
+ // Don't care if license header is a doc comment or not
+ : DOC_COMMENT package imports type_declarations
+ | package imports type_declarations
+ ;
+
+doc_comments
+ : DOC_COMMENT { $$ = $1; }
+ | doc_comments DOC_COMMENT
+ {
+ $1->merge($2);
+ $$ = $1;
+ }
;
valid_identifier
@@ -549,7 +567,7 @@
interface_declarations
: /* empty */
- | interface_declarations type_declaration
+ | interface_declarations commentable_type_declaration
{
CHECK((*scope)->isInterface());
@@ -562,7 +580,7 @@
YYERROR;
}
}
- | interface_declarations method_declaration
+ | interface_declarations commentable_method_declaration
{
CHECK((*scope)->isInterface());
@@ -590,7 +608,16 @@
type_declarations
: /* empty */
| error_stmt
- | type_declarations type_declaration
+ | type_declarations commentable_type_declaration
+ ;
+
+commentable_type_declaration
+ : doc_comments type_declaration
+ {
+ $2->setDocComment($1);
+ $$ = $2;
+ }
+ | type_declaration { $$ = $1; }
;
type_declaration
@@ -750,6 +777,17 @@
}
;
+commentable_method_declaration
+ : doc_comments method_declaration
+ {
+ if ($2 != nullptr) $2->setDocComment($1);
+ $$ = $2;
+ }
+ | method_declaration
+ {
+ $$ = $1;
+ }
+
method_declaration
: error_stmt { $$ = nullptr; }
| opt_annotations valid_identifier '(' typed_vars ')' require_semicolon
@@ -860,7 +898,7 @@
field_declarations
: /* empty */ { $$ = new std::vector<NamedReference<Type>*>; }
- | field_declarations field_declaration
+ | field_declarations commentable_field_declaration
{
$$ = $1;
@@ -871,6 +909,14 @@
}
;
+commentable_field_declaration
+ : doc_comments field_declaration
+ {
+ if ($2 != nullptr) $2->setDocComment($1);
+ $$ = $2;
+ }
+ | field_declaration { $$ = $1; }
+
field_declaration
: error_stmt { $$ = nullptr; }
| type_or_inplace_compound_declaration valid_identifier require_semicolon
@@ -959,6 +1005,15 @@
: '{' enum_values opt_comma '}' { $$ = $2; }
;
+commentable_enum_value
+ : doc_comments enum_value
+ {
+ $2->setDocComment($1);
+ $$ = $2;
+ }
+ | enum_value { $$ = $1; }
+ ;
+
enum_value
: valid_identifier
{
@@ -973,24 +1028,24 @@
enum_values
: /* empty */
{ /* do nothing */ }
- | enum_value
+ | commentable_enum_value
{
CHECK((*scope)->isEnum());
static_cast<EnumType *>(*scope)->addValue($1);
}
- | enum_values ',' enum_value
+ | enum_values ',' commentable_enum_value
{
CHECK((*scope)->isEnum());
static_cast<EnumType *>(*scope)->addValue($3);
}
- | error ',' enum_value
+ | error ',' commentable_enum_value
{
ast->addSyntaxError();
CHECK((*scope)->isEnum());
static_cast<EnumType *>(*scope)->addValue($3);
}
- | enum_values ',' error ',' enum_value
+ | enum_values ',' error ',' commentable_enum_value
{
ast->addSyntaxError();
diff --git a/utils/Formatter.cpp b/utils/Formatter.cpp
index b5876dc..338cb17 100644
--- a/utils/Formatter.cpp
+++ b/utils/Formatter.cpp
@@ -123,8 +123,8 @@
if (pos == std::string::npos) {
if (mAtStartOfLine) {
- fprintf(mFile, "%s", mLinePrefix.c_str());
fprintf(mFile, "%*s", (int)(mSpacesPerIndent * mIndentDepth), "");
+ fprintf(mFile, "%s", mLinePrefix.c_str());
mAtStartOfLine = false;
}
@@ -132,17 +132,16 @@
break;
}
+ if (mAtStartOfLine && (pos > start || !mLinePrefix.empty())) {
+ fprintf(mFile, "%*s", (int)(mSpacesPerIndent * mIndentDepth), "");
+ fprintf(mFile, "%s", mLinePrefix.c_str());
+ }
+
if (pos == start) {
fprintf(mFile, "\n");
mAtStartOfLine = true;
} else if (pos > start) {
- if (mAtStartOfLine) {
- fprintf(mFile, "%s", mLinePrefix.c_str());
- fprintf(mFile, "%*s", (int)(mSpacesPerIndent * mIndentDepth), "");
- }
-
output(out.substr(start, pos - start + 1));
-
mAtStartOfLine = true;
}
