wip: more error changes

Change-Id: Ib6766c69e5dac1e6a1daff1db909d20f36b81d2e
diff --git a/system/host/fidl/include/fidl/flat_ast.h b/system/host/fidl/include/fidl/flat_ast.h
index c4740ab..32e3ac6 100644
--- a/system/host/fidl/include/fidl/flat_ast.h
+++ b/system/host/fidl/include/fidl/flat_ast.h
@@ -22,6 +22,7 @@
 #include "error_reporter.h"
 #include "raw_ast.h"
 #include "type_shape.h"
+#include "virtual_source_file.h"
 
 namespace fidl {
 namespace flat {
@@ -702,7 +703,7 @@
     }
 
     std::vector<Member> members;
-    const bool anonymous;
+    bool anonymous;
     TypeShape typeshape;
     bool recursive = false;
 };
@@ -773,14 +774,10 @@
         Method(std::unique_ptr<raw::AttributeList> attributes,
                std::unique_ptr<raw::Ordinal> ordinal, SourceLocation name,
                Struct* maybe_request,
-               Struct* maybe_response,
-               std::unique_ptr<Type> maybe_error)
+               Struct* maybe_response)
             : attributes(std::move(attributes)), ordinal(std::move(ordinal)), name(std::move(name)),
-              maybe_request(maybe_request), maybe_response(maybe_response), maybe_error(std::move(maybe_error)) {
+              maybe_request(maybe_request), maybe_response(maybe_response) {
             assert(this->maybe_request != nullptr || this->maybe_response != nullptr);
-            if (maybe_error) {
-                assert(this->maybe_request != nullptr && this->maybe_response != nullptr);
-            }
         }
 
         std::unique_ptr<raw::AttributeList> attributes;
@@ -788,7 +785,6 @@
         SourceLocation name;
         Struct* maybe_request;
         Struct* maybe_response;
-        std::unique_ptr<Type> maybe_error;
     };
 
     Interface(std::unique_ptr<raw::AttributeList> attributes, Name name,
@@ -1070,6 +1066,7 @@
                                        const raw::AttributeList* attributes);
 
     Name NextAnonymousName();
+    Name GeneratedName(const std::string& name);
 
     bool CompileCompoundIdentifier(const raw::CompoundIdentifier* compound_identifier,
                                    SourceLocation location, Name* out_name);
@@ -1093,6 +1090,8 @@
     bool ConsumeTableDeclaration(std::unique_ptr<raw::TableDeclaration> table_declaration);
     bool ConsumeUnionDeclaration(std::unique_ptr<raw::UnionDeclaration> union_declaration);
 
+    bool CreateMethodResponse(Struct* in_response, std::unique_ptr<raw::Type> error, Struct** out_response);
+
     bool TypeCanBeConst(const Type* type);
     const Type* TypeResolve(const Type* type);
     bool TypeIsConvertibleTo(const Type* from_type, const Type* to_type);
@@ -1197,6 +1196,9 @@
     Typespace* typespace_;
 
     uint32_t anon_counter_ = 0;
+
+    VirtualSourceFile generated_source_file_{"generated"};
+    std::map<std::string, StringView> generated_names_;
 };
 
 } // namespace flat
diff --git a/system/host/fidl/include/fidl/source_file.h b/system/host/fidl/include/fidl/source_file.h
index 8b9f9d5..41798a6 100644
--- a/system/host/fidl/include/fidl/source_file.h
+++ b/system/host/fidl/include/fidl/source_file.h
@@ -16,7 +16,7 @@
 class SourceFile {
 public:
     SourceFile(std::string filename, std::string data);
-    ~SourceFile();
+    virtual ~SourceFile();
 
     StringView filename() const { return filename_; }
     StringView data() const { return data_; }
@@ -28,7 +28,7 @@
         int column;
     };
 
-    StringView LineContaining(StringView view, Position* position_out) const;
+    virtual StringView LineContaining(StringView view, Position* position_out) const;
 
 private:
     std::string filename_;
diff --git a/system/host/fidl/include/fidl/virtual_source_file.h b/system/host/fidl/include/fidl/virtual_source_file.h
new file mode 100644
index 0000000..39a994f
--- /dev/null
+++ b/system/host/fidl/include/fidl/virtual_source_file.h
@@ -0,0 +1,31 @@
+// Copyright 2018 The Fuchsia Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef ZIRCON_SYSTEM_HOST_FIDL_INCLUDE_FIDL_VIRTUAL_SOURCE_FILE_H_
+#define ZIRCON_SYSTEM_HOST_FIDL_INCLUDE_FIDL_VIRTUAL_SOURCE_FILE_H_
+
+#include <string>
+#include <utility>
+#include <vector>
+
+#include "source_file.h"
+
+namespace fidl {
+
+class VirtualSourceFile : public SourceFile {
+public:
+    VirtualSourceFile(std::string filename) : SourceFile(filename, "") {}
+    virtual ~VirtualSourceFile() = default;
+
+    virtual StringView LineContaining(StringView view, Position* position_out) const;
+
+    StringView AddLine(const std::string& line);
+
+private:
+    std::vector<std::string> lines_;
+};
+
+} // namespace fidl
+
+#endif // ZIRCON_SYSTEM_HOST_FIDL_INCLUDE_FIDL_VIRTUAL_SOURCE_FILE_H_
diff --git a/system/host/fidl/lib/flat_ast.cpp b/system/host/fidl/lib/flat_ast.cpp
index 6fca0aa..8c41e3b 100644
--- a/system/host/fidl/lib/flat_ast.cpp
+++ b/system/host/fidl/lib/flat_ast.cpp
@@ -817,7 +817,14 @@
     std::ostringstream data;
     data << "SomeLongAnonymousPrefix";
     data << anon_counter_++;
-    return Name(this, StringView(data.str()));
+    return GeneratedName(data.str());
+}
+
+Name Library::GeneratedName(const std::string& name) {
+    if (generated_names_.find(name) == generated_names_.end()) {
+        generated_names_.emplace(name, generated_source_file_.AddLine(name));
+    }
+    return Name(this, SourceLocation(generated_names_[name], generated_source_file_));
 }
 
 bool Library::CompileCompoundIdentifier(const raw::CompoundIdentifier* compound_identifier,
@@ -1123,10 +1130,10 @@
                 return false;
         }
 
+        // TODO(ianloic): desugar errors here.
         std::unique_ptr<Type> maybe_error = nullptr;
         if (method->maybe_error != nullptr) {
-            auto location = method->maybe_error->location();
-            if (!ConsumeType(std::move(method->maybe_error), location, &maybe_error))
+            if (!CreateMethodResponse(maybe_response, std::move(method->maybe_error), &maybe_response))
                 return false;
         }
 
@@ -1134,7 +1141,7 @@
         methods.emplace_back(std::move(attributes),
                              std::move(ordinal_literal),
                              std::move(method_name), std::move(maybe_request),
-                             std::move(maybe_response), std::move(maybe_error));
+                             std::move(maybe_response));
     }
 
     interface_declarations_.push_back(
@@ -1143,6 +1150,48 @@
     return RegisterDecl(interface_declarations_.back().get());
 }
 
+bool Library::CreateMethodResponse(Struct* in_response, std::unique_ptr<raw::Type> raw_error, Struct** out_response) {
+    in_response->anonymous = false;
+
+    // Get the type of in_response.
+    auto in_response_type = std::make_unique<IdentifierType>(Name(in_response->name.library(), in_response->name.name_part()), types::Nullability::kNonnullable);
+    // Consume the error type.
+    auto location = raw_error->location();
+    std::unique_ptr<Type> error;
+    if (!ConsumeType(std::move(raw_error), location, &error))
+        return false;
+    // TODO(ianloic): validate that error is 32 bits.
+
+    // Make a union containing in_response and error.
+    auto response_name = GeneratedName("response");
+    auto error_name = GeneratedName("err");
+    std::vector<Union::Member> union_members;
+    union_members.emplace_back(std::move(in_response_type), response_name.source_location(), nullptr);
+    union_members.emplace_back(std::move(error), error_name.source_location(), nullptr);
+
+    auto union_name = NextAnonymousName();
+    // TODO(ianloic): create a [Result] attribute
+    union_declarations_.push_back(std::make_unique<Union>(nullptr /* attributes */, std::move(union_name),
+                                                          std::move(union_members)));
+    auto union_decl = union_declarations_.back().get();
+    if (!RegisterDecl(union_decl))
+        return false;
+    auto union_type = std::make_unique<IdentifierType>(Name(union_decl->name.library(), union_decl->name.name_part()), types::Nullability::kNonnullable);
+
+    // Make a struct just containing that union.
+    auto struct_name = NextAnonymousName();
+    std::vector<Struct::Member> struct_members;
+    auto result_name = GeneratedName("result");
+    struct_members.emplace_back(std::move(union_type), result_name.source_location(), nullptr, nullptr);
+    struct_declarations_.push_back(std::make_unique<Struct>(nullptr, std::move(struct_name), std::move(struct_members), true));
+    auto struct_decl = struct_declarations_.back().get();
+    if (!RegisterDecl(struct_decl))
+        return false;
+    *out_response = struct_decl;
+
+    return true;
+}
+
 bool Library::ConsumeParameterList(std::unique_ptr<raw::ParameterList> parameter_list,
                                    Struct** out_struct_decl) {
     std::vector<Struct::Member> members;
diff --git a/system/host/fidl/lib/virtual_source_file.cpp b/system/host/fidl/lib/virtual_source_file.cpp
new file mode 100644
index 0000000..382ac89
--- /dev/null
+++ b/system/host/fidl/lib/virtual_source_file.cpp
@@ -0,0 +1,32 @@
+// Copyright 2018 The Fuchsia Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "fidl/virtual_source_file.h"
+
+namespace fidl {
+
+StringView VirtualSourceFile::AddLine(const std::string& line) {
+    std::string& new_line = lines_.emplace_back(line);
+    
+    return StringView(new_line);
+}
+
+StringView VirtualSourceFile::LineContaining(StringView view, Position* position_out) const {
+    for (int i=0; i<lines_.size(); i++) {
+        const std::string& line = lines_[i];
+        const char* line_begin = &*line.cbegin();
+        const char* line_end = &*line.cend();
+        if (view.data() < line_begin || view.data() + view.size() > line_end)
+            continue;
+        if (position_out != nullptr) {
+            auto column = view.data() - line_begin;
+            assert(column < std::numeric_limits<int>::max());
+            *position_out = {i+1, (int)column};
+        }
+        return StringView(line);
+    }
+    return StringView();
+}
+
+}  // namespace fidl
\ No newline at end of file
diff --git a/system/host/fidl/rules.mk b/system/host/fidl/rules.mk
index c649a59..0bcd772 100644
--- a/system/host/fidl/rules.mk
+++ b/system/host/fidl/rules.mk
@@ -30,6 +30,7 @@
     $(LOCAL_DIR)/lib/source_manager.cpp \
     $(LOCAL_DIR)/lib/tables_generator.cpp \
     $(LOCAL_DIR)/lib/tree_visitor.cpp \
+    $(LOCAL_DIR)/lib/virtual_source_file.cpp \
     $(BUILDGEN_DIR)/lib/json_schema.cpp \
 
 $(BUILDGEN_DIR)/lib/json_schema.cpp: $(LOCAL_DIR)/schema.json
diff --git a/system/utest/fidl-compiler/errors_tests.cpp b/system/utest/fidl-compiler/errors_tests.cpp
index 202a46f..28b125b 100644
--- a/system/utest/fidl-compiler/errors_tests.cpp
+++ b/system/utest/fidl-compiler/errors_tests.cpp
@@ -13,20 +13,39 @@
 
     TestLibrary library(R"FIDL(
 library example;
+
 interface Example {
-    Method() -> (int32 flub) error int32;
+    Method() -> (string foo) error int32;    
 };
+
 )FIDL");
+
     ASSERT_TRUE(library.Compile());
 
     auto methods = &library.LookupInterface("Example")->methods;
     ASSERT_EQ(methods->size(), 1);
     auto method = &methods->at(0);
+    auto response = method->maybe_response;
+    ASSERT_NOT_NULL(response);
+    ASSERT_EQ(response->members.size(), 1);
+    auto response_member = &response->members.at(0);
+    ASSERT_EQ(response_member->type->kind, fidl::flat::Type::Kind::kIdentifier);
+    auto result_identifier = static_cast<fidl::flat::IdentifierType*>(response_member->type.get());
+    auto result_name = &result_identifier->name;
+    auto result_union = library.LookupUnion(result_name->name_part());
+    ASSERT_NOT_NULL(result_union);
+    ASSERT_EQ(result_union->members.size(), 2);
+
+    auto error_member = &result_union->members.at(1);
+    std::string error_member_name = error_member->name.data();
+    ASSERT_STR_EQ(error_member_name.c_str(), "err");
+    /*
     auto error_type = method->maybe_error.get();
     ASSERT_NE(error_type, nullptr);
     ASSERT_EQ(error_type->kind, fidl::flat::Type::Kind::kPrimitive);
     auto primitive_type = static_cast<fidl::flat::PrimitiveType*>(error_type);
     ASSERT_EQ(primitive_type->subtype, fidl::types::PrimitiveSubtype::kInt32);
+    */
 
     END_TEST;
 }
diff --git a/system/utest/fidl-compiler/json_generator_tests.cpp b/system/utest/fidl-compiler/json_generator_tests.cpp
index 6c21c26..cef5658 100644
--- a/system/utest/fidl-compiler/json_generator_tests.cpp
+++ b/system/utest/fidl-compiler/json_generator_tests.cpp
@@ -622,6 +622,139 @@
 
     END_TEST;
 }
+
+bool json_generator_test_error() {
+    BEGIN_TEST;
+
+    for (int i = 0; i < /*kRepeatTestCount*/ 1; i++) {
+        EXPECT_TRUE(checkJSONGenerator(R"FIDL(
+library fidl.test.json;
+
+interface Example {
+   foo(string s) -> (int64 y) error uint32;
+};
+
+)FIDL",
+                                       R"JSON({
+  "version": "0.0.1",
+  "name": "fidl.test.json",
+  "library_dependencies": [],
+  "const_declarations": [],
+  "enum_declarations": [],
+  "interface_declarations": [
+    {
+      "name": "fidl.test.json/Example",
+      "methods": [
+        {
+          "ordinal": 1369693400,
+          "name": "foo",
+          "has_request": true,
+          "maybe_request": [
+            {
+              "type": {
+                "kind": "string",
+                "nullable": false
+              },
+              "name": "s",
+              "size": 16,
+              "alignment": 8,
+              "offset": 16,
+              "max_handles": 0
+            }
+          ],
+          "maybe_request_size": 32,
+          "maybe_request_alignment": 8,
+          "has_response": true,
+          "maybe_response": [
+            {
+              "type": {
+                "kind": "identifier",
+                "identifier": "fidl.test.json/SomeLongAnonymousPrefix2",
+                "nullable": false
+              },
+              "name": "result",
+              "size": 16,
+              "alignment": 8,
+              "offset": 16,
+              "max_handles": 0
+            }
+          ],
+          "maybe_response_size": 32,
+          "maybe_response_alignment": 8
+        }
+      ]
+    }
+  ],
+  "struct_declarations": [
+    {
+      "name": "fidl.test.json/SomeLongAnonymousPrefix1",
+      "anonymous": false,
+      "members": [
+        {
+          "type": {
+            "kind": "primitive",
+            "subtype": "int64"
+          },
+          "name": "y",
+          "size": 8,
+          "alignment": 8,
+          "offset": 0,
+          "max_handles": 0
+        }
+      ],
+      "size": 8,
+      "alignment": 8,
+      "max_handles": 0
+    }
+  ],
+  "table_declarations": [],
+  "union_declarations": [
+    {
+      "name": "fidl.test.json/SomeLongAnonymousPrefix2",
+      "members": [
+        {
+          "type": {
+            "kind": "identifier",
+            "identifier": "fidl.test.json/SomeLongAnonymousPrefix1",
+            "nullable": false
+          },
+          "name": "response",
+          "size": 8,
+          "alignment": 8,
+          "offset": 8
+        },
+        {
+          "type": {
+            "kind": "primitive",
+            "subtype": "uint32"
+          },
+          "name": "err",
+          "size": 4,
+          "alignment": 4,
+          "offset": 8
+        }
+      ],
+      "size": 16,
+      "alignment": 8,
+      "max_handles": 0
+    }
+  ],
+  "declaration_order": [
+    "fidl.test.json/SomeLongAnonymousPrefix1",
+    "fidl.test.json/SomeLongAnonymousPrefix2",
+    "fidl.test.json/Example"
+  ],
+  "declarations": {
+    "fidl.test.json/Example": "interface",
+    "fidl.test.json/SomeLongAnonymousPrefix1": "struct",
+    "fidl.test.json/SomeLongAnonymousPrefix2": "union"
+  }
+})JSON"));
+    }
+
+    END_TEST;
+}
+
 } // namespace
 
 BEGIN_TEST_CASE(json_generator_tests);
@@ -630,4 +763,5 @@
 RUN_TEST(json_generator_test_table);
 RUN_TEST(json_generator_test_union);
 RUN_TEST(json_generator_test_inheritance);
+RUN_TEST(json_generator_test_error);
 END_TEST_CASE(json_generator_tests);