diff --git a/.gitignore b/.gitignore index 7c094ba619a00ee9e851239c214a85c6147553d7..cd5fbb355bf9bc4611f25f010a92454d7dae5088 100644 --- a/.gitignore +++ b/.gitignore @@ -21,6 +21,7 @@ compile_commands.json *_LOCAL_* *_BASE_* *.sublime-* +json_generator_tests_*.txt /benchmarks/ /infra/.recipe_deps /out* diff --git a/system/host/fidl/include/fidl/flat_ast.h b/system/host/fidl/include/fidl/flat_ast.h index ea2bad456975a3d8c952dcf53485863c53ae0355..3349e5f307c5b9e755e2b47a245225f4a62ab864 100644 --- a/system/host/fidl/include/fidl/flat_ast.h +++ b/system/host/fidl/include/fidl/flat_ast.h @@ -51,7 +51,7 @@ struct Name { } return name_.data() == other.name_.data(); } - bool operator!=(const Name& other) const { return name_.data() != other.name_.data(); } + bool operator!=(const Name& other) const { return !operator==(other); } bool operator<(const Name& other) const { if (LibraryName(library_, ".") != LibraryName(other.library_, ".")) { @@ -483,6 +483,7 @@ public: // Lookup a library by its |library_name|. bool Lookup(const std::vector<StringView>& library_name, Library** out_library) const; + private: std::map<std::vector<StringView>, std::unique_ptr<Library>> all_libraries_; }; @@ -493,7 +494,7 @@ public: // will be referenced by its name, and may also be optionally be referenced // by an alias. bool Register(StringView filename, Library* dep_library, - const std::unique_ptr<raw::Identifier>& maybe_alias); + const std::unique_ptr<raw::Identifier>& maybe_alias); // Lookup a dependent library by |filename| and |name|. bool Lookup(StringView filename, const std::vector<StringView>& name, diff --git a/system/host/fidl/lib/flat_ast.cpp b/system/host/fidl/lib/flat_ast.cpp index f36ccd4d7eaf2562793db52a536b8cf32684893f..6a1c9ccc3b9d6a58dcf8d17a902c5a5926266b20 100644 --- a/system/host/fidl/lib/flat_ast.cpp +++ b/system/host/fidl/lib/flat_ast.cpp @@ -292,7 +292,7 @@ bool Libraries::Insert(std::unique_ptr<Library> library) { } bool Libraries::Lookup(const std::vector<StringView>& library_name, - Library** out_library) const { + Library** out_library) const { auto iter = all_libraries_.find(library_name); if (iter == all_libraries_.end()) { return false; @@ -1109,11 +1109,21 @@ bool Library::DeclDependencies(Decl* decl, std::set<Decl*>* out_edges) { return true; } +namespace { +// To compare two Decl's in the same library, it suffices to compare the unqualified names of the Decl's. +struct CmpDeclInLibrary { + bool operator()(const Decl* a, const Decl* b) const { + assert(a->name != b->name || a == b); + return a->name < b->name; + } +}; +} // namespace + bool Library::SortDeclarations() { // |degree| is the number of undeclared dependencies for each decl. - std::map<Decl*, uint32_t> degrees; + std::map<Decl*, uint32_t, CmpDeclInLibrary> degrees; // |inverse_dependencies| records the decls that depend on each decl. - std::map<Decl*, std::vector<Decl*>> inverse_dependencies; + std::map<Decl*, std::vector<Decl*>, CmpDeclInLibrary> inverse_dependencies; for (auto& name_and_decl : declarations_) { Decl* decl = name_and_decl.second; degrees[decl] = 0u; diff --git a/system/utest/fidl-compiler/json_generator_tests.cpp b/system/utest/fidl-compiler/json_generator_tests.cpp index fb5af64d6c1dcbc8d2df40b73a4b55059f1de78e..8254964ccde21418bd106a37d581bc841986a00d 100644 --- a/system/utest/fidl-compiler/json_generator_tests.cpp +++ b/system/utest/fidl-compiler/json_generator_tests.cpp @@ -16,13 +16,20 @@ namespace { -static inline void trim(std::string &s) { +// We repeat each test in a loop in order to catch situations where memory layout +// determines what JSON is produced (this is often manifested due to using a std::map<Foo*,...> +// in compiler source code). +static const int kRepeatTestCount = 100; + +static inline void trim(std::string& s) { s.erase(s.begin(), std::find_if(s.begin(), s.end(), [](int ch) { - return !std::isspace(ch) && ch != '\n'; - })); + return !std::isspace(ch) && ch != '\n'; + })); s.erase(std::find_if(s.rbegin(), s.rend(), [](int ch) { - return !std::isspace(ch) && ch != '\n'; - }).base(), s.end()); + return !std::isspace(ch) && ch != '\n'; + }) + .base(), + s.end()); } bool checkJSONGenerator(std::string raw_source_code, std::string expected_json) { @@ -57,7 +64,8 @@ bool checkJSONGenerator(std::string raw_source_code, std::string expected_json) bool json_generator_test_simple() { BEGIN_TEST; - EXPECT_TRUE(checkJSONGenerator(R"FIDL( + for (int i = 0; i < kRepeatTestCount; i++) { + EXPECT_TRUE(checkJSONGenerator(R"FIDL( library fidl.test.json; struct Simple { @@ -65,7 +73,8 @@ struct Simple { bool f2; }; -)FIDL", R"JSON( +)FIDL", + R"JSON( { "version": "0.0.1", "name": "fidl.test.json", @@ -114,6 +123,7 @@ struct Simple { } } )JSON")); + } END_TEST; } @@ -121,7 +131,8 @@ struct Simple { bool json_generator_test_union() { BEGIN_TEST; - EXPECT_TRUE(checkJSONGenerator(R"FIDL( + for (int i = 0; i < kRepeatTestCount; i++) { + EXPECT_TRUE(checkJSONGenerator(R"FIDL( library fidl.test.json; struct Pizza { @@ -137,7 +148,8 @@ union PizzaOrPasta { Pasta pasta; }; -)FIDL", R"JSON( +)FIDL", + R"JSON( { "version": "0.0.1", "name": "fidl.test.json", @@ -224,8 +236,8 @@ union PizzaOrPasta { } ], "declaration_order": [ - "fidl.test.json/Pasta", "fidl.test.json/Pizza", + "fidl.test.json/Pasta", "fidl.test.json/PizzaOrPasta" ], "declarations": { @@ -235,6 +247,7 @@ union PizzaOrPasta { } } )JSON")); + } END_TEST; }