From 76fb7dee43ab6b259ccb4a9f363db866b582c726 Mon Sep 17 00:00:00 2001 From: topisani Date: Wed, 20 Dec 2017 18:15:31 +0100 Subject: [PATCH 1/3] Parse C++17 nested namespace declarations This is maybe not the most correct way to do it, but i dont think it misparses any valid code. --- src/libclang/namespace_parser.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/libclang/namespace_parser.cpp b/src/libclang/namespace_parser.cpp index c5d1cd9..e7df48f 100644 --- a/src/libclang/namespace_parser.cpp +++ b/src/libclang/namespace_parser.cpp @@ -18,13 +18,16 @@ namespace { detail::cxtokenizer tokenizer(context.tu, context.file, cur); detail::cxtoken_stream stream(tokenizer, cur); - // [inline] namespace [] { + // [inline] namespace|:: [] [{] auto is_inline = false; if (skip_if(stream, "inline")) is_inline = true; - skip(stream, "namespace"); + // C++17 nested namespace declarations + if (!detail::skip_if(stream, "namespace")) + skip(stream, "::"); + auto attributes = parse_attributes(stream); // { @@ -37,7 +40,9 @@ namespace auto other_attributes = parse_attributes(stream); attributes.insert(attributes.end(), other_attributes.begin(), other_attributes.end()); - skip(stream, "{"); + // C++17 nested namespace declarations + if (!detail::skip_if(stream, "::")) + skip(stream, "{"); auto result = cpp_namespace::builder(name.c_str(), is_inline); result.get().add_attribute(attributes); From 037619964f1cf3345deaf4460f381194feb421be Mon Sep 17 00:00:00 2001 From: topisani Date: Fri, 22 Dec 2017 04:09:48 +0100 Subject: [PATCH 2/3] Comments and (failing) tests --- src/libclang/namespace_parser.cpp | 7 +++++-- test/cpp_namespace.cpp | 23 +++++++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/libclang/namespace_parser.cpp b/src/libclang/namespace_parser.cpp index e7df48f..7938ad4 100644 --- a/src/libclang/namespace_parser.cpp +++ b/src/libclang/namespace_parser.cpp @@ -24,7 +24,9 @@ namespace if (skip_if(stream, "inline")) is_inline = true; - // C++17 nested namespace declarations + // C++17 nested namespace declarations get one cursor per nested name. + // The first cursor starts with the `namespace` keyword, and the + // following start with the `::` separator. Either way, it is skipped. if (!detail::skip_if(stream, "namespace")) skip(stream, "::"); @@ -40,7 +42,8 @@ namespace auto other_attributes = parse_attributes(stream); attributes.insert(attributes.end(), other_attributes.begin(), other_attributes.end()); - // C++17 nested namespace declarations + // If the next token is not `::`, there are no more nested namespace + // names, and we expect to see an opening brace. if (!detail::skip_if(stream, "::")) skip(stream, "{"); diff --git a/test/cpp_namespace.cpp b/test/cpp_namespace.cpp index 4b08ba7..20f9833 100644 --- a/test/cpp_namespace.cpp +++ b/test/cpp_namespace.cpp @@ -33,6 +33,16 @@ namespace c /// namespace { /// } namespace {} + +/// namespace e{ +/// namespace f{ +/// } +/// } +namespace e::f {} + +/// \entity e::f +/// namespace f{ +/// } )"; auto file = parse({}, "cpp_namespace.cpp", code); @@ -69,6 +79,19 @@ namespace {} REQUIRE(!ns.is_inline()); REQUIRE(no_children == 0u); } + else if (ns.name() == "e") + { + REQUIRE(!ns.is_anonymous()); + REQUIRE(!ns.is_inline()); + REQUIRE(no_children == 1u); + } + else if (ns.name() == "f") + { + REQUIRE(!ns.is_anonymous()); + check_parent(ns, "e", "e::f"); + REQUIRE(!ns.is_inline()); + REQUIRE(no_children == 0u); + } else REQUIRE(false); }); From 755f251da9e8798f7d037035bb879afeafbeccbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonathan=20M=C3=BCller?= Date: Fri, 22 Dec 2017 10:54:37 +0100 Subject: [PATCH 3/3] Steal comment of nested namespace --- include/cppast/cpp_namespace.hpp | 21 +++++++++++------- src/libclang/class_parser.cpp | 2 +- src/libclang/friend_parser.cpp | 4 ++-- src/libclang/language_linkage_parser.cpp | 2 +- src/libclang/libclang_parser.cpp | 2 +- src/libclang/namespace_parser.cpp | 27 ++++++++++++++++++------ src/libclang/parse_functions.cpp | 7 +++--- src/libclang/parse_functions.hpp | 5 +++-- src/libclang/template_parser.cpp | 2 +- test/cpp_namespace.cpp | 15 ++++++------- 10 files changed, 52 insertions(+), 35 deletions(-) diff --git a/include/cppast/cpp_namespace.hpp b/include/cppast/cpp_namespace.hpp index ee419f8..5d18274 100644 --- a/include/cppast/cpp_namespace.hpp +++ b/include/cppast/cpp_namespace.hpp @@ -23,9 +23,9 @@ namespace cppast class builder { public: - /// \effects Sets the namespace name and whether it is inline. - explicit builder(std::string name, bool is_inline) - : namespace_(new cpp_namespace(std::move(name), is_inline)) + /// \effects Sets the namespace name and whether it is inline and nested. + explicit builder(std::string name, bool is_inline, bool is_nested) + : namespace_(new cpp_namespace(std::move(name), is_inline, is_nested)) { } @@ -60,6 +60,12 @@ namespace cppast return inline_; } + /// \returns Whether or not the namespace is part of a C++17 nested namespace. + bool is_nested() const noexcept + { + return nested_; + } + /// \returns Whether or not the namespace is anonymous. bool is_anonymous() const noexcept { @@ -67,8 +73,8 @@ namespace cppast } private: - cpp_namespace(std::string name, bool is_inline) - : cpp_entity(std::move(name)), inline_(is_inline) + cpp_namespace(std::string name, bool is_inline, bool is_nested) + : cpp_entity(std::move(name)), inline_(is_inline), nested_(is_nested) { } @@ -80,6 +86,7 @@ namespace cppast } bool inline_; + bool nested_; }; /// \exclude @@ -184,9 +191,7 @@ namespace cppast } private: - cpp_using_declaration(cpp_entity_ref target) : cpp_entity(""), target_(std::move(target)) - { - } + cpp_using_declaration(cpp_entity_ref target) : cpp_entity(""), target_(std::move(target)) {} cpp_entity_kind do_get_entity_kind() const noexcept override; diff --git a/src/libclang/class_parser.cpp b/src/libclang/class_parser.cpp index fde0a95..c020de5 100644 --- a/src/libclang/class_parser.cpp +++ b/src/libclang/class_parser.cpp @@ -167,7 +167,7 @@ std::unique_ptr detail::parse_cpp_class(const detail::parse_context& == CXCursor_UnexposedAttr) // I have no idea what this is, but happens on Windows // other children due to templates and stuff return; - else if (auto entity = parse_entity(context, child)) + else if (auto entity = parse_entity(context, &builder.get(), child)) builder.add_child(std::move(entity)); }); } diff --git a/src/libclang/friend_parser.cpp b/src/libclang/friend_parser.cpp index 9649fa5..72583f7 100644 --- a/src/libclang/friend_parser.cpp +++ b/src/libclang/friend_parser.cpp @@ -54,7 +54,7 @@ std::unique_ptr detail::parse_cpp_friend(const detail::parse_context // for some reason libclang gives a type ref here // we actually need a class decl cursor, so parse the referenced one // this might be a definition, so give friend information to the parser - entity = parse_entity(context, referenced, cur); + entity = parse_entity(context, nullptr, referenced, cur); comment = type_safe::copy(entity->comment()).value_or(""); } } @@ -74,7 +74,7 @@ std::unique_ptr detail::parse_cpp_friend(const detail::parse_context } else if (clang_isDeclaration(kind)) { - entity = parse_entity(context, child, cur); + entity = parse_entity(context, nullptr, child, cur); if (entity) { // steal comment diff --git a/src/libclang/language_linkage_parser.cpp b/src/libclang/language_linkage_parser.cpp index 0477f21..d07078a 100644 --- a/src/libclang/language_linkage_parser.cpp +++ b/src/libclang/language_linkage_parser.cpp @@ -30,7 +30,7 @@ std::unique_ptr detail::try_parse_cpp_language_linkage(const parse_c auto builder = cpp_language_linkage::builder(name.c_str()); context.comments.match(builder.get(), cur); detail::visit_children(cur, [&](const CXCursor& child) { - auto entity = parse_entity(context, child); + auto entity = parse_entity(context, &builder.get(), child); if (entity) builder.add_child(std::move(entity)); }); diff --git a/src/libclang/libclang_parser.cpp b/src/libclang/libclang_parser.cpp index 14fb260..8b121e1 100644 --- a/src/libclang/libclang_parser.cpp +++ b/src/libclang/libclang_parser.cpp @@ -504,7 +504,7 @@ std::unique_ptr libclang_parser::do_parse(const cpp_entity_index& idx, macro_iter != preprocessed.macros.end() && macro_iter->line <= line; ++macro_iter) builder.add_child(std::move(macro_iter->macro)); - auto entity = detail::parse_entity(context, cur); + auto entity = detail::parse_entity(context, &builder.get(), cur); if (entity) builder.add_child(std::move(entity)); } diff --git a/src/libclang/namespace_parser.cpp b/src/libclang/namespace_parser.cpp index 7938ad4..0dc5636 100644 --- a/src/libclang/namespace_parser.cpp +++ b/src/libclang/namespace_parser.cpp @@ -27,15 +27,19 @@ namespace // C++17 nested namespace declarations get one cursor per nested name. // The first cursor starts with the `namespace` keyword, and the // following start with the `::` separator. Either way, it is skipped. + auto is_nested = false; if (!detail::skip_if(stream, "namespace")) - skip(stream, "::"); + { + is_nested = true; + skip(stream, "::"); + } auto attributes = parse_attributes(stream); // { // or when anonymous: { if (detail::skip_if(stream, "{")) - return cpp_namespace::builder("", is_inline); + return cpp_namespace::builder("", is_inline, false); auto& name = stream.get().value(); @@ -45,23 +49,32 @@ namespace // If the next token is not `::`, there are no more nested namespace // names, and we expect to see an opening brace. if (!detail::skip_if(stream, "::")) - skip(stream, "{"); + skip(stream, "{"); - auto result = cpp_namespace::builder(name.c_str(), is_inline); + auto result = cpp_namespace::builder(name.c_str(), is_inline, is_nested); result.get().add_attribute(attributes); return result; } } std::unique_ptr detail::parse_cpp_namespace(const detail::parse_context& context, - const CXCursor& cur) + cpp_entity& parent, const CXCursor& cur) { DEBUG_ASSERT(cur.kind == CXCursor_Namespace, detail::assert_handler{}); auto builder = make_ns_builder(context, cur); - context.comments.match(builder.get(), cur); + if (builder.get().is_nested()) + { + // steal comment from parent + DEBUG_ASSERT(parent.kind() == cpp_namespace::kind(), detail::assert_handler{}); + builder.get().set_comment(type_safe::copy(parent.comment())); + parent.set_comment(type_safe::nullopt); + } + else + context.comments.match(builder.get(), cur); + detail::visit_children(cur, [&](const CXCursor& cur) { - auto entity = parse_entity(context, cur); + auto entity = parse_entity(context, &builder.get(), cur); if (entity) builder.add_child(std::move(entity)); }); diff --git a/src/libclang/parse_functions.cpp b/src/libclang/parse_functions.cpp index a4f5a60..b035b81 100644 --- a/src/libclang/parse_functions.cpp +++ b/src/libclang/parse_functions.cpp @@ -112,8 +112,8 @@ namespace } std::unique_ptr detail::parse_entity(const detail::parse_context& context, - const CXCursor& cur, - const CXCursor& parent_cur) try + cpp_entity* parent, const CXCursor& cur, + const CXCursor& parent_cur) try { if (context.logger->is_verbose()) { @@ -138,7 +138,8 @@ std::unique_ptr detail::parse_entity(const detail::parse_context& co break; case CXCursor_Namespace: - return parse_cpp_namespace(context, cur); + DEBUG_ASSERT(parent, detail::assert_handler{}); + return parse_cpp_namespace(context, *parent, cur); case CXCursor_NamespaceAlias: return parse_cpp_namespace_alias(context, cur); case CXCursor_UsingDirective: diff --git a/src/libclang/parse_functions.hpp b/src/libclang/parse_functions.hpp index da732cd..866d6c8 100644 --- a/src/libclang/parse_functions.hpp +++ b/src/libclang/parse_functions.hpp @@ -112,7 +112,7 @@ namespace cppast const parse_context& context, const CXCursor& cur); std::unique_ptr parse_cpp_namespace(const parse_context& context, - const CXCursor& cur); + cpp_entity& parent, const CXCursor& cur); std::unique_ptr parse_cpp_namespace_alias(const parse_context& context, const CXCursor& cur); std::unique_ptr parse_cpp_using_directive(const parse_context& context, @@ -162,9 +162,10 @@ namespace cppast std::unique_ptr parse_cpp_static_assert(const parse_context& context, const CXCursor& cur); + // parent: used for nested namespace, doesn't matter otherwise // parent_cur: used when parsing templates or friends std::unique_ptr parse_entity( - const parse_context& context, const CXCursor& cur, + const parse_context& context, cpp_entity* parent, const CXCursor& cur, const CXCursor& parent_cur = clang_getNullCursor()); } } // namespace cppast::detail diff --git a/src/libclang/template_parser.cpp b/src/libclang/template_parser.cpp index 1ba1281..a38232a 100644 --- a/src/libclang/template_parser.cpp +++ b/src/libclang/template_parser.cpp @@ -35,7 +35,7 @@ namespace DEBUG_ASSERT(!clang_Cursor_isNull(result), detail::parse_error_handler{}, cur, "missing child of template"); - auto entity = detail::parse_entity(context, result, cur); + auto entity = detail::parse_entity(context, nullptr, result, cur); if (!entity) return type_safe::nullopt; DEBUG_ASSERT(p(entity->kind()), detail::parse_error_handler{}, cur, diff --git a/test/cpp_namespace.cpp b/test/cpp_namespace.cpp index 20f9833..70fbdf6 100644 --- a/test/cpp_namespace.cpp +++ b/test/cpp_namespace.cpp @@ -34,15 +34,9 @@ namespace c /// } namespace {} -/// namespace e{ -/// namespace f{ -/// } -/// } -namespace e::f {} - -/// \entity e::f /// namespace f{ /// } +namespace e::f {} )"; auto file = parse({}, "cpp_namespace.cpp", code); @@ -84,6 +78,7 @@ namespace e::f {} REQUIRE(!ns.is_anonymous()); REQUIRE(!ns.is_inline()); REQUIRE(no_children == 1u); + return false; // don't have a comment } else if (ns.name() == "f") { @@ -94,8 +89,10 @@ namespace e::f {} } else REQUIRE(false); + + return true; }); - REQUIRE(count == 5u); + REQUIRE(count == 7u); } TEST_CASE("cpp_namespace_alias") @@ -126,7 +123,7 @@ namespace f = outer::c; )"; cpp_entity_index idx; - auto check_alias = [&](const cpp_namespace_alias& alias, const char* target_name, + auto check_alias = [&](const cpp_namespace_alias& alias, const char* target_name, const char* target_full_name, unsigned no) { auto& target = alias.target(); REQUIRE(target.name() == target_name);