[fidl][cpp] Make stubs and proxies always accept generated ordinals.
This allows the stubs and proxies to be resilient to older prebuilts
using explicit ordinals. We can revert the semantic parts of this
change when everything is using generated ordinals.
This also has a small number of formatting improvements to the generated
code.
FIDL-425
Test: Test added to fidl unittests.
Change-Id: Ic646d9940c6b0276092cd74a27ec376c7417836b
diff --git a/go/src/fidl/compiler/backend/cpp/ir/ir.go b/go/src/fidl/compiler/backend/cpp/ir/ir.go
index 93bf8f9..a54eaa6 100644
--- a/go/src/fidl/compiler/backend/cpp/ir/ir.go
+++ b/go/src/fidl/compiler/backend/cpp/ir/ir.go
@@ -154,6 +154,8 @@
types.Attributes
Ordinal types.Ordinal
OrdinalName string
+ GenOrdinal types.Ordinal
+ GenOrdinalName string
Name string
HasRequest bool
Request []Parameter
@@ -601,6 +603,8 @@
v.Attributes,
v.Ordinal,
fmt.Sprintf("k%s_%s_Ordinal", r.Name, v.Name),
+ v.GenOrdinal,
+ fmt.Sprintf("k%s_%s_GenOrdinal", r.Name, v.Name),
name,
v.HasRequest,
c.compileParameterArray(v.Request),
diff --git a/go/src/fidl/compiler/backend/cpp/ir/ir_test.go b/go/src/fidl/compiler/backend/cpp/ir/ir_test.go
index 189e61f..e1c144c 100644
--- a/go/src/fidl/compiler/backend/cpp/ir/ir_test.go
+++ b/go/src/fidl/compiler/backend/cpp/ir/ir_test.go
@@ -30,6 +30,7 @@
Methods: []types.Method{
{
Ordinal: types.Ordinal(1),
+ GenOrdinal: types.Ordinal(314159),
Name: types.Identifier("First"),
HasRequest: true,
Request: []types.Parameter{
@@ -42,6 +43,7 @@
},
{
Ordinal: types.Ordinal(2),
+ GenOrdinal: types.Ordinal(271828),
Name: types.Identifier("Second"),
HasRequest: true,
Request: []types.Parameter{
@@ -82,14 +84,16 @@
SyncProxyName: "Test_SyncProxy",
Methods: []Method{
{
- Ordinal: 1,
- OrdinalName: "kTest_First_Ordinal",
- Name: "First",
- HasRequest: true,
+ Ordinal: 1,
+ OrdinalName: "kTest_First_Ordinal",
+ GenOrdinal: 314159,
+ GenOrdinalName: "kTest_First_GenOrdinal",
+ Name: "First",
+ HasRequest: true,
Request: []Parameter{
{
Type: Type{
- Decl: "int16_t",
+ Decl: "int16_t",
LLDecl: "int16_t",
},
Name: "Value",
@@ -109,10 +113,12 @@
ResponderType: "Test_First_Responder",
},
{
- Ordinal: 2,
- OrdinalName: "kTest_Second_Ordinal",
- Name: "Second",
- HasRequest: true,
+ Ordinal: 2,
+ OrdinalName: "kTest_Second_Ordinal",
+ GenOrdinal: 271828,
+ GenOrdinalName: "kTest_Second_GenOrdinal",
+ Name: "Second",
+ HasRequest: true,
Request: []Parameter{
{
Type: Type{
diff --git a/go/src/fidl/compiler/backend/cpp/templates/interface.tmpl.go b/go/src/fidl/compiler/backend/cpp/templates/interface.tmpl.go
index b52208c..92e3031 100644
--- a/go/src/fidl/compiler/backend/cpp/templates/interface.tmpl.go
+++ b/go/src/fidl/compiler/backend/cpp/templates/interface.tmpl.go
@@ -176,7 +176,10 @@
{{- define "InterfaceDefinition" }}
namespace {
-{{ range .Methods }}
+{{- range .Methods }}
+ {{ if ne .GenOrdinal .Ordinal }}
+constexpr uint32_t {{ .GenOrdinalName }} = {{ .GenOrdinal }}u;
+ {{- end }}
constexpr uint32_t {{ .OrdinalName }} = {{ .Ordinal }}u;
{{- if .HasRequest }}
extern "C" const fidl_type_t {{ .RequestTypeName }};
@@ -200,7 +203,7 @@
{{ .ProxyName }}::{{ .ProxyName }}(::fidl::internal::ProxyController* controller)
: controller_(controller) {
- (void) controller_;
+ (void)controller_;
}
{{ .ProxyName }}::~{{ .ProxyName }}() = default;
@@ -211,6 +214,9 @@
{{- range .Methods }}
{{- if not .HasRequest }}
{{- if .HasResponse }}
+ {{- if ne .GenOrdinal .Ordinal }}
+ case {{ .GenOrdinalName }}:
+ {{- end }}
case {{ .OrdinalName }}: {
if (!{{ .Name }}) {
status = ZX_OK;
@@ -252,7 +258,7 @@
namespace {
class {{ .ResponseHandlerType }} : public ::fidl::internal::MessageHandler {
- public:
+ public:
{{ .ResponseHandlerType }}({{ $.Name }}::{{ .CallbackType }} callback)
: callback_(std::move(callback)) {
ZX_DEBUG_ASSERT_MSG(callback_,
@@ -280,7 +286,7 @@
return ZX_OK;
}
- private:
+ private:
{{ $.Name }}::{{ .CallbackType }} callback_;
{{ .ResponseHandlerType }}(const {{ .ResponseHandlerType }}&) = delete;
@@ -288,7 +294,6 @@
};
} // namespace
-
{{- end }}
void {{ $.ProxyName }}::{{ template "RequestMethodSignature" . }} {
::fidl::Encoder _encoder({{ .OrdinalName }});
@@ -308,7 +313,7 @@
{{- end }}
{{ .StubName }}::{{ .StubName }}({{ .ClassName }}* impl) : impl_(impl) {
- (void) impl_;
+ (void)impl_;
}
{{ .StubName }}::~{{ .StubName }}() = default;
@@ -320,15 +325,15 @@
class {{ .ResponderType }} {
public:
- {{ .ResponderType }}(::fidl::internal::PendingResponse response)
- : response_(std::move(response)) {}
+ {{ .ResponderType }}(::fidl::internal::PendingResponse response, uint32_t ordinal)
+ : response_(std::move(response)), ordinal_(ordinal) {}
void operator()({{ template "Params" .Response }}) {
- ::fidl::Encoder _encoder({{ .OrdinalName }});
+ ::fidl::Encoder _encoder(ordinal_);
{{- if .Response }}
- _encoder.Alloc({{ .ResponseSize }} - sizeof(fidl_message_header_t));
+ _encoder.Alloc({{ .ResponseSize }} - sizeof(fidl_message_header_t));
{{- range .Response }}
- ::fidl::Encode(&_encoder, &{{ .Name }}, {{ .Offset }});
+ ::fidl::Encode(&_encoder, &{{ .Name }}, {{ .Offset }});
{{- end }}
{{- end }}
response_.Send(&{{ .ResponseTypeName }}, _encoder.GetMessage());
@@ -336,6 +341,7 @@
private:
::fidl::internal::PendingResponse response_;
+ uint32_t ordinal_;
};
{{- end }}
{{- end }}
@@ -347,9 +353,13 @@
::fidl::Message message,
::fidl::internal::PendingResponse response) {
zx_status_t status = ZX_OK;
- switch (message.ordinal()) {
+ uint32_t ordinal = message.ordinal();
+ switch (ordinal) {
{{- range .Methods }}
{{- if .HasRequest }}
+ {{- if ne .GenOrdinal .Ordinal }}
+ case {{ .GenOrdinalName }}:
+ {{- end }}
case {{ .OrdinalName }}: {
const char* error_msg = nullptr;
status = message.Decode(&{{ .RequestTypeName }}, &error_msg);
@@ -368,7 +378,7 @@
{{- if $index }}, {{ end }}std::move(arg{{ $index }})
{{- end -}}
{{- if .HasResponse -}}
- {{- if .Request }}, {{ end -}}{{ .ResponderType }}(std::move(response))
+ {{- if .Request }}, {{ end -}}{{ .ResponderType }}(std::move(response), ordinal)
{{- end -}}
);
break;
@@ -401,7 +411,7 @@
{{- end }}
{{ .SyncProxyName }}::{{ .SyncProxyName }}(::zx::channel channel)
- : proxy_(::std::move(channel)) {}
+ : proxy_(::std::move(channel)) {}
{{ .SyncProxyName }}::~{{ .SyncProxyName }}() = default;
diff --git a/go/src/fidl/compiler/backend/types/types.go b/go/src/fidl/compiler/backend/types/types.go
index 1e9600f..3fed949 100644
--- a/go/src/fidl/compiler/backend/types/types.go
+++ b/go/src/fidl/compiler/backend/types/types.go
@@ -421,6 +421,7 @@
type Method struct {
Attributes
Ordinal Ordinal `json:"ordinal"`
+ GenOrdinal Ordinal `json:"generated_ordinal"`
Name Identifier `json:"name"`
HasRequest bool `json:"has_request"`
Request []Parameter `json:"maybe_request,omitempty"`
diff --git a/go/src/fidl/compiler/backend/typestest/doc_comments.fidl.json b/go/src/fidl/compiler/backend/typestest/doc_comments.fidl.json
index c6e8bfc..5a3b1c4 100644
--- a/go/src/fidl/compiler/backend/typestest/doc_comments.fidl.json
+++ b/go/src/fidl/compiler/backend/typestest/doc_comments.fidl.json
@@ -37,6 +37,7 @@
"methods": [
{
"ordinal": 1,
+ "generated_ordinal": 1061382220,
"name": "Method",
"has_request": true,
"maybe_attributes": [
diff --git a/go/src/fidl/compiler/backend/typestest/doc_comments.fidl.json.cc.golden b/go/src/fidl/compiler/backend/typestest/doc_comments.fidl.json.cc.golden
index d693230..fb5eb0f 100644
--- a/go/src/fidl/compiler/backend/typestest/doc_comments.fidl.json.cc.golden
+++ b/go/src/fidl/compiler/backend/typestest/doc_comments.fidl.json.cc.golden
@@ -7,8 +7,8 @@
namespace name {
namespace {
-
-
+
+constexpr uint32_t kInterface_Method_GenOrdinal = 1061382220u;
constexpr uint32_t kInterface_Method_Ordinal = 1u;
extern "C" const fidl_type_t test_name_InterfaceMethodRequestTable;
@@ -22,7 +22,7 @@
Interface_Proxy::Interface_Proxy(::fidl::internal::ProxyController* controller)
: controller_(controller) {
- (void) controller_;
+ (void)controller_;
}
Interface_Proxy::~Interface_Proxy() = default;
@@ -45,7 +45,7 @@
}
Interface_Stub::Interface_Stub(Interface_clazz* impl) : impl_(impl) {
- (void) impl_;
+ (void)impl_;
}
Interface_Stub::~Interface_Stub() = default;
@@ -58,7 +58,9 @@
::fidl::Message message,
::fidl::internal::PendingResponse response) {
zx_status_t status = ZX_OK;
- switch (message.ordinal()) {
+ uint32_t ordinal = message.ordinal();
+ switch (ordinal) {
+ case kInterface_Method_GenOrdinal:
case kInterface_Method_Ordinal: {
const char* error_msg = nullptr;
status = message.Decode(&test_name_InterfaceMethodRequestTable, &error_msg);
@@ -78,7 +80,7 @@
}
Interface_SyncProxy::Interface_SyncProxy(::zx::channel channel)
- : proxy_(::std::move(channel)) {}
+ : proxy_(::std::move(channel)) {}
Interface_SyncProxy::~Interface_SyncProxy() = default;
zx_status_t Interface_SyncProxy::Method() {
diff --git a/public/lib/fidl/cpp/interface_ptr_unittest.cc b/public/lib/fidl/cpp/interface_ptr_unittest.cc
index dc78086..fcf7b8d 100644
--- a/public/lib/fidl/cpp/interface_ptr_unittest.cc
+++ b/public/lib/fidl/cpp/interface_ptr_unittest.cc
@@ -271,5 +271,58 @@
EXPECT_EQ(1u, grobs.size());
}
+TEST(InterfacePtr, InterfaceCanHandleGeneratedOrdinal) {
+ // This tests that the server bindings can respond just as well to the
+ // generated ordinal as they can to the explicitly specified ordinal. It
+ // works by intercepting a message sent by the interface ptr client, changing
+ // the ordinal, forwarding it to the server, and then sending it back. If all
+ // goes well, no one should be any the wiser.
+
+ test::AsyncLoopForTest loop;
+
+ zx::channel client, h2;
+ EXPECT_EQ(ZX_OK, zx::channel::create(0, &client, &h2));
+ fidl::test::frobinator::FrobinatorPtr ptr;
+ int error_count = 0;
+ ptr.set_error_handler([&error_count](zx_status_t status) {
+ EXPECT_EQ(ZX_ERR_INVALID_ARGS, status);
+ ++error_count;
+ });
+ EXPECT_EQ(ZX_OK, ptr.Bind(std::move(client)));
+
+ int reply_count = 0;
+ ptr->Grob("one", [&reply_count](StringPtr value) {
+ ++reply_count;
+ EXPECT_FALSE(value.is_null());
+ EXPECT_EQ("response", *value);
+ });
+
+ loop.RunUntilIdle();
+
+ fidl::MessageBuffer buffer;
+ fidl::Message message = buffer.CreateEmptyMessage();
+ EXPECT_EQ(ZX_OK, message.Read(h2.get(), 0));
+
+ // Cribbed from generated .cc file.
+ constexpr uint32_t kFrobinator_Grob_GenOrdinal = 1499796418u;
+ fidl_message_header_t *header =
+ reinterpret_cast<fidl_message_header_t *>(buffer.bytes());
+ header->ordinal = kFrobinator_Grob_GenOrdinal;
+
+ test::FrobinatorImpl impl;
+ zx::channel h3, server;
+ EXPECT_EQ(ZX_OK, zx::channel::create(0, &h3, &server));
+ Binding<fidl::test::frobinator::Frobinator> binding(&impl, std::move(server),
+ loop.dispatcher());
+ fidl::MessageBuffer response_buffer;
+ fidl::Message response = response_buffer.CreateEmptyMessage();
+ EXPECT_EQ(ZX_OK, message.Write(h3.get(), 0));
+ loop.RunUntilIdle();
+ EXPECT_EQ(ZX_OK, response.Read(h3.get(), 0));
+ EXPECT_EQ(ZX_OK, response.Write(h2.get(), 0));
+ EXPECT_EQ(ZX_OK, ptr.WaitForResponse());
+ EXPECT_EQ(1, reply_count);
+}
+
} // namespace
} // namespace fidl
