From 7c30ba7c464e44dc38d262ecc43885141bfb6e5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonathan=20M=C3=BCller?= Date: Tue, 10 Oct 2017 17:07:41 +0200 Subject: [PATCH] [breaking] Ensure matching line numbers after preprocessing Unmatched documentation comments are not just a string, but also contain the line number as well. --- include/cppast/cpp_file.hpp | 28 +++++++++++++++-------- src/libclang/libclang_parser.cpp | 2 +- src/libclang/preprocessor.cpp | 39 ++++++++++++++++++++++++++++---- test/cpp_preprocessor.cpp | 39 +++++++++++++++++++++++++++++++- 4 files changed, 91 insertions(+), 17 deletions(-) diff --git a/include/cppast/cpp_file.hpp b/include/cppast/cpp_file.hpp index b45ebec..942e4f5 100644 --- a/include/cppast/cpp_file.hpp +++ b/include/cppast/cpp_file.hpp @@ -13,6 +13,18 @@ namespace cppast { + /// An unmatched documentation comment. + struct cpp_doc_comment + { + std::string content; + unsigned line; + + cpp_doc_comment(std::string content, unsigned line) + : content(std::move(content)), line(line) + { + } + }; + /// A [cppast::cpp_entity]() modelling a file. /// /// This is the top-level entity of the AST. @@ -26,9 +38,7 @@ namespace cppast { public: /// \effects Sets the file name. - explicit builder(std::string name) : file_(new cpp_file(std::move(name))) - { - } + explicit builder(std::string name) : file_(new cpp_file(std::move(name))) {} /// \effects Adds an entity. void add_child(std::unique_ptr child) noexcept @@ -37,9 +47,9 @@ namespace cppast } /// \effects Adds an unmatched documentation comment. - void add_unmatched_comment(std::string str) + void add_unmatched_comment(cpp_doc_comment comment) { - file_->comments_.push_back(std::move(str)); + file_->comments_.push_back(std::move(comment)); } /// \returns The not yet finished file. @@ -62,20 +72,18 @@ namespace cppast }; /// \returns The unmatched documentation comments. - type_safe::array_ref unmatched_comments() const noexcept + type_safe::array_ref unmatched_comments() const noexcept { return type_safe::ref(comments_.data(), comments_.size()); } private: - cpp_file(std::string name) : cpp_entity(std::move(name)) - { - } + cpp_file(std::string name) : cpp_entity(std::move(name)) {} /// \returns [cpp_entity_type::file_t](). cpp_entity_kind do_get_entity_kind() const noexcept override; - std::vector comments_; + std::vector comments_; }; /// \exclude diff --git a/src/libclang/libclang_parser.cpp b/src/libclang/libclang_parser.cpp index 4fcae1e..996b8ec 100644 --- a/src/libclang/libclang_parser.cpp +++ b/src/libclang/libclang_parser.cpp @@ -510,7 +510,7 @@ std::unique_ptr libclang_parser::do_parse(const cpp_entity_index& idx, for (auto& c : preprocessed.comments) { if (!c.comment.empty()) - builder.add_unmatched_comment(std::move(c.comment)); + builder.add_unmatched_comment(cpp_doc_comment(std::move(c.comment), c.line)); } if (context.error) diff --git a/src/libclang/preprocessor.cpp b/src/libclang/preprocessor.cpp index 45c3d52..3de001c 100644 --- a/src/libclang/preprocessor.cpp +++ b/src/libclang/preprocessor.cpp @@ -191,6 +191,16 @@ namespace { } + void undo_write() + { + if (write_ == true && !result_->empty()) + { + if (result_->back() == '\n') + --cur_line_; + result_->pop_back(); + } + } + void write_str(std::string str) { if (write_ == false) @@ -363,7 +373,7 @@ namespace break; } } - p.skip(); + skip(p, "\n"); return result; } @@ -734,16 +744,32 @@ detail::preprocessor_output detail::preprocess(const libclang_compile_config& co switch (lm.value().flag) { case linemarker::line_directive: - break; // ignore - // no need to handle it, preprocessed output doesn't need to match line numbers precisely + if (file_depth == 0u && lm.value().line > p.cur_line()) + { + // write the necessary newlines + p.write_str(std::string(lm.value().line - p.cur_line(), '\n')); + DEBUG_ASSERT(p.cur_line() == lm.value().line, detail::assert_handler{}); + } + else if (file_depth == 0u) + // current line may be one higher than the expected line, + // because the include directives inserted by -dI have an additional newline + // that is not yet removed + DEBUG_ASSERT(p.cur_line() - lm.value().line <= 1u, detail::assert_handler{}); + break; case linemarker::enter_new: if (file_depth == 0u && lm.value().file.front() != '<') { // this file is directly included by the given file - // write include with full path - // note: don't build include here, do it when an #include is encountered + // and it is not a fake file like builtin or command line + + // remove the trailing newline from the include that brought us here + DEBUG_ASSERT(p.was_newl(), detail::assert_handler{}); + p.undo_write(); + + // and write include with full path p.write_str("#include \"" + lm.value().file + "\"\n"); + // note: don't build include here, do it when an #include is encountered } ++file_depth; @@ -755,6 +781,9 @@ detail::preprocessor_output detail::preprocess(const libclang_compile_config& co if (file_depth == 0u) { DEBUG_ASSERT(lm.value().file == path, detail::assert_handler{}); + // difference is 1 if coming from an included file (because newline of include is already written) + // difference is 0 if coming from a builtin file (because no include has been written) + DEBUG_ASSERT(p.cur_line() - lm.value().line <= 1u, detail::assert_handler{}); p.enable_write(); } break; diff --git a/test/cpp_preprocessor.cpp b/test/cpp_preprocessor.cpp index d2db720..316906c 100644 --- a/test/cpp_preprocessor.cpp +++ b/test/cpp_preprocessor.cpp @@ -136,6 +136,43 @@ TEST_CASE("cpp_include_directive", "[!hide][clang4]") REQUIRE(count == 1u); } +TEST_CASE("preprocessor line numbers") +{ + auto code = R"(/// 1 + +#include + +/// 5 + +#include + +int foo; + +/// 11 + +#define foo \ + \ + \ + int \ + main() + +/// 19 + +foo {} + +/// 23 + +#include + +/// 27 +)"; + + auto file = parse({}, "preprocessor_line_numbers.cpp", code); + for (auto& comment : file->unmatched_comments()) + REQUIRE(comment.line == std::stoi(comment.content)); + REQUIRE((file->unmatched_comments().size() == 6u)); +} + TEST_CASE("comment matching") { auto code = R"( @@ -205,6 +242,6 @@ void j(); }); for (auto& comment : file->unmatched_comments()) - REQUIRE(comment == "u"); + REQUIRE(comment.content == "u"); REQUIRE((file->unmatched_comments().size() == 3u)); }