From 5c5ddf165cb32c1139ed00e9168cd4d536861976 Mon Sep 17 00:00:00 2001 From: alandefreitas Date: Fri, 17 Jan 2025 11:46:25 -0300 Subject: [PATCH 1/3] support locations relative to search paths #feat fix #798 --- include/mrdocs/Metadata/Source.hpp | 6 - src/lib/AST/ASTVisitor.cpp | 170 +++++++++++------------------ src/lib/AST/ASTVisitor.hpp | 16 ++- src/lib/Metadata/Source.cpp | 1 - src/lib/Support/Path.cpp | 8 +- 5 files changed, 77 insertions(+), 124 deletions(-) diff --git a/include/mrdocs/Metadata/Source.hpp b/include/mrdocs/Metadata/Source.hpp index 2adeee27f0..65f9be68ea 100644 --- a/include/mrdocs/Metadata/Source.hpp +++ b/include/mrdocs/Metadata/Source.hpp @@ -61,10 +61,6 @@ struct MRDOCS_DECL */ unsigned LineNumber = 0; - /** The kind of file this is - */ - FileKind Kind = FileKind::Source; - /** Whether this location has documentation. */ bool Documented = false; @@ -75,12 +71,10 @@ struct MRDOCS_DECL std::string_view const filepath = {}, std::string_view const filename = {}, unsigned const line = 0, - FileKind const kind = FileKind::Source, bool const documented = false) : Path(filepath) , Filename(filename) , LineNumber(line) - , Kind(kind) , Documented(documented) { } diff --git a/src/lib/AST/ASTVisitor.cpp b/src/lib/AST/ASTVisitor.cpp index 6fafebaa41..da57272d5a 100644 --- a/src/lib/AST/ASTVisitor.cpp +++ b/src/lib/AST/ASTVisitor.cpp @@ -36,6 +36,7 @@ #include #include #include +#include #include #include #include @@ -43,75 +44,58 @@ namespace clang::mrdocs { -auto +ASTVisitor::FileInfo ASTVisitor::FileInfo::build( - std::span const> /* search_dirs */, + std::span const search_dirs, std::string_view const file_path, - std::string_view const sourceRoot) -> ASTVisitor::FileInfo { + std::string_view const sourceRoot) { FileInfo file_info; file_info.full_path = std::string(file_path); - file_info.short_path = file_info.full_path; + file_info.short_path = std::string(file_path); - std::ptrdiff_t best_length = 0; - auto check_dir = [&]( - std::string_view const dir_path, - FileKind const kind) + // Find the best match for the file path + for (auto const& search_dir : search_dirs) { - auto NI = llvm::sys::path::begin(file_path); - auto const NE = llvm::sys::path::end(file_path); - auto DI = llvm::sys::path::begin(dir_path); - auto const DE = llvm::sys::path::end(dir_path); - - for(; NI != NE; ++NI, ++DI) + if (files::startsWith(file_path, search_dir)) { - // reached the end of the directory path - if(DI == DE) + file_info.short_path.erase(0, search_dir.size()); + if (file_info.short_path.starts_with('/')) { - // update the best prefix length - if(std::ptrdiff_t length = - NI - llvm::sys::path::begin(file_path); - length > best_length) - { - best_length = length; - file_info.kind = kind; - return true; - } - break; + file_info.short_path.erase(0, 1); } - // separators always match - if(NI->size() == 1 && DI->size() == 1 && - llvm::sys::path::is_separator(NI->front()) && - llvm::sys::path::is_separator(DI->front())) - continue; - // components don't match - if(*NI != *DI) - break; + return file_info; } - return false; - }; - - bool const in_sourceRoot = check_dir( - sourceRoot, FileKind::Source); - - // only use a sourceRoot relative path if we - // don't find anything in the include search directories - // bool any_match = false; - // for (auto const& [dir_path, kind]: search_dirs) - // { - // any_match |= check_dir(dir_path, kind); - // } - - // KRYSTIAN TODO: if we don't find any matches, - // make the path relative to sourceRoot and return it + } - // override the file kind if - // the file was found in sourceRoot - if (in_sourceRoot) + // Fallback to sourceRoot + if (files::startsWith(file_path, sourceRoot)) { - file_info.kind = FileKind::Source; + file_info.short_path.erase(0, sourceRoot.size()); + if (file_info.short_path.starts_with('/')) + { + file_info.short_path.erase(0, 1); + } + return file_info; } - file_info.short_path.erase(0, best_length); + // Fallback to system search paths in PATH + std::optional const optEnvPathsStr = llvm::sys::Process::GetEnv("PATH"); + MRDOCS_CHECK_OR(optEnvPathsStr, file_info); + std::string const& envPathsStr = *optEnvPathsStr; + for (auto const envPaths = llvm::split(envPathsStr, llvm::sys::EnvPathSeparator); + auto envPath: envPaths) + { + auto normEnvPath = files::makePosixStyle(envPath); + if (files::startsWith(file_path, normEnvPath)) + { + file_info.short_path.erase(0, normEnvPath.size()); + if (file_info.short_path.starts_with('/')) + { + file_info.short_path.erase(0, 1); + } + return file_info; + } + } return file_info; } @@ -140,63 +124,37 @@ ASTVisitor( MRDOCS_ASSERT(context_.getTraversalScope() == std::vector{context_.getTranslationUnitDecl()}); - auto make_posix_absolute = [&](std::string_view const old_path) - { - llvm::SmallString<128> new_path(old_path); - if(! llvm::sys::path::is_absolute(new_path)) - { - // KRYSTIAN FIXME: use FileManager::makeAbsolutePath? - auto const& cwd = source_.getFileManager(). - getFileSystemOpts().WorkingDir; - // we can't normalize a relative path - // without a base directory - // MRDOCS_ASSERT(! cwd.empty()); - llvm::sys::fs::make_absolute(cwd, new_path); - } - // remove ./ and ../ - llvm::sys::path::remove_dots(new_path, true, llvm::sys::path::Style::posix); - // convert to posix style - llvm::sys::path::native(new_path, llvm::sys::path::Style::posix); - return std::string(new_path); - }; - + // Store the search directories + auto const& cwd = source_.getFileManager().getFileSystemOpts().WorkingDir; Preprocessor& PP = sema_.getPreprocessor(); HeaderSearch& HS = PP.getHeaderSearchInfo(); - std::vector> search_dirs; - search_dirs.reserve(HS.search_dir_size()); + search_dirs_.reserve(HS.search_dir_size()); // first, convert all the include search directories into POSIX style for (const DirectoryLookup& DL : HS.search_dir_range()) { OptionalDirectoryEntryRef DR = DL.getDirRef(); // only consider normal directories - if(! DL.isNormalDir() || ! DR) + if (!DL.isNormalDir() || !DR) + { continue; + } // store the normalized path - search_dirs.emplace_back( - make_posix_absolute(DR->getName()), - DL.isSystemHeaderDirectory() ? - FileKind::System : FileKind::Other); - } - - std::string const sourceRoot = make_posix_absolute(config_->sourceRoot); - auto cacheFileInfo = [&](const FileEntry* entry) - { - // "try" implies this may fail, so fallback to getName - // if an empty string is returned - std::string_view file_path = - entry->tryGetRealPathName(); - files_.emplace( - entry, - FileInfo::build( - search_dirs, - make_posix_absolute(file_path), - sourceRoot)); + auto normPath = files::makePosixStyle(files::makeAbsolute(DR->getName(), cwd)); + search_dirs_.push_back(std::move(normPath)); + } + + // Store preprocessed information about all file entries + std::string const sourceRoot = files::makePosixStyle(files::makeAbsolute(config_->sourceRoot, cwd)); + auto cacheFileInfo = [&](FileEntry const* entry) { + std::string_view const file_path = entry->tryGetRealPathName(); + MRDOCS_CHECK_OR(!file_path.empty()); + auto const normPath = files::makePosixStyle( + files::makeAbsolute(file_path, cwd)); + auto FI = FileInfo::build(search_dirs_, normPath, sourceRoot); + files_.emplace(entry, FI); }; - - // build the file info for the main file - cacheFileInfo(source_.getFileEntryForID(source_.getMainFileID())); - - // build the file info for all included files + FileEntry const* mainFileEntry = source_.getFileEntryForID(source_.getMainFileID()); + cacheFileInfo(mainFileEntry); for(const FileEntry* file : PP.getIncludedFiles()) { cacheFileInfo(file); @@ -829,9 +787,7 @@ populate( { return; } - I.DefLoc.emplace(file->full_path, - file->short_path, line, file->kind, - documented); + I.DefLoc.emplace(file->full_path, file->short_path, line, documented); } else { @@ -846,9 +802,7 @@ populate( { return; } - I.Loc.emplace_back(file->full_path, - file->short_path, line, file->kind, - documented); + I.Loc.emplace_back(file->full_path, file->short_path, line, documented); } } diff --git a/src/lib/AST/ASTVisitor.hpp b/src/lib/AST/ASTVisitor.hpp index 18da626a67..4783aa6a2a 100644 --- a/src/lib/AST/ASTVisitor.hpp +++ b/src/lib/AST/ASTVisitor.hpp @@ -79,6 +79,17 @@ class ASTVisitor // An unordered set of all extracted Info declarations InfoSet info_; + /* Preprocessed information about search directories + + This vector stores information about the search directories + used by the translation unit. + + Whenever we extract information about where a symbol is located, + we store the full path of the symbol and the path relative + to the search directory in this vector. + */ + std::vector search_dirs_; + /* Struct to hold pre-processed file information. This struct stores information about a file, including its full path, @@ -92,7 +103,7 @@ class ASTVisitor static FileInfo build( - std::span> search_dirs, + std::span search_dirs, std::string_view file_path, std::string_view sourceRoot); @@ -102,9 +113,6 @@ class ASTVisitor // The file path relative to a search directory. std::string short_path; - // The kind of the file. - FileKind kind = FileKind::Source; - // Whether this file passes the file filters std::optional passesFilters; }; diff --git a/src/lib/Metadata/Source.cpp b/src/lib/Metadata/Source.cpp index 1b77dccfa8..27ca99857e 100644 --- a/src/lib/Metadata/Source.cpp +++ b/src/lib/Metadata/Source.cpp @@ -42,7 +42,6 @@ tag_invoke( io.map("path", loc.Path); io.map("file", loc.Filename); io.map("line", loc.LineNumber); - io.map("kind", loc.Kind); io.map("documented", loc.Documented); } diff --git a/src/lib/Support/Path.cpp b/src/lib/Support/Path.cpp index 0dd40b2c96..00b028bd68 100644 --- a/src/lib/Support/Path.cpp +++ b/src/lib/Support/Path.cpp @@ -266,11 +266,9 @@ std::string makePosixStyle( std::string_view pathName) { - std::string result(pathName); - for(auto& c : result) - if(c == '\\') - c = '/'; - return result; + SmallPathString result(pathName); + llvm::sys::path::native(result, llvm::sys::path::Style::posix); + return std::string(result); } std::string From 4665e2f2ba89633e0ad80382cf2bdefc971512d7 Mon Sep 17 00:00:00 2001 From: alandefreitas Date: Fri, 17 Jan 2025 12:48:02 -0300 Subject: [PATCH 2/3] lazy FileInfo extraction Most symbols are filtered out regardless of the file location. This commit creates a FileInfo cache and only builds the FileInfo when necessary. The search paths are also lazily iterated to find the best matches. #perf --- include/mrdocs/Support/Path.hpp | 6 + src/lib/AST/ASTVisitor.cpp | 252 ++++++++++++++++---------------- src/lib/AST/ASTVisitor.hpp | 71 +++++---- src/lib/Support/Path.cpp | 21 ++- 4 files changed, 200 insertions(+), 150 deletions(-) diff --git a/include/mrdocs/Support/Path.hpp b/include/mrdocs/Support/Path.hpp index 878d9b6171..a4cf2447c7 100644 --- a/include/mrdocs/Support/Path.hpp +++ b/include/mrdocs/Support/Path.hpp @@ -241,6 +241,12 @@ std::string makePosixStyle( std::string_view pathName); +/** Check if the path is posix style. +*/ +MRDOCS_DECL +bool +isPosixStyle(std::string_view pathName); + /** Return the filename with a new or different extension. @param fileName The absolute or relative path diff --git a/src/lib/AST/ASTVisitor.cpp b/src/lib/AST/ASTVisitor.cpp index da57272d5a..551ee2c099 100644 --- a/src/lib/AST/ASTVisitor.cpp +++ b/src/lib/AST/ASTVisitor.cpp @@ -44,61 +44,6 @@ namespace clang::mrdocs { -ASTVisitor::FileInfo -ASTVisitor::FileInfo::build( - std::span const search_dirs, - std::string_view const file_path, - std::string_view const sourceRoot) { - FileInfo file_info; - file_info.full_path = std::string(file_path); - file_info.short_path = std::string(file_path); - - // Find the best match for the file path - for (auto const& search_dir : search_dirs) - { - if (files::startsWith(file_path, search_dir)) - { - file_info.short_path.erase(0, search_dir.size()); - if (file_info.short_path.starts_with('/')) - { - file_info.short_path.erase(0, 1); - } - return file_info; - } - } - - // Fallback to sourceRoot - if (files::startsWith(file_path, sourceRoot)) - { - file_info.short_path.erase(0, sourceRoot.size()); - if (file_info.short_path.starts_with('/')) - { - file_info.short_path.erase(0, 1); - } - return file_info; - } - - // Fallback to system search paths in PATH - std::optional const optEnvPathsStr = llvm::sys::Process::GetEnv("PATH"); - MRDOCS_CHECK_OR(optEnvPathsStr, file_info); - std::string const& envPathsStr = *optEnvPathsStr; - for (auto const envPaths = llvm::split(envPathsStr, llvm::sys::EnvPathSeparator); - auto envPath: envPaths) - { - auto normEnvPath = files::makePosixStyle(envPath); - if (files::startsWith(file_path, normEnvPath)) - { - file_info.short_path.erase(0, normEnvPath.size()); - if (file_info.short_path.starts_with('/')) - { - file_info.short_path.erase(0, 1); - } - return file_info; - } - } - return file_info; -} - ASTVisitor:: ASTVisitor( const ConfigImpl& config, @@ -123,42 +68,6 @@ ASTVisitor( // used somewhere MRDOCS_ASSERT(context_.getTraversalScope() == std::vector{context_.getTranslationUnitDecl()}); - - // Store the search directories - auto const& cwd = source_.getFileManager().getFileSystemOpts().WorkingDir; - Preprocessor& PP = sema_.getPreprocessor(); - HeaderSearch& HS = PP.getHeaderSearchInfo(); - search_dirs_.reserve(HS.search_dir_size()); - // first, convert all the include search directories into POSIX style - for (const DirectoryLookup& DL : HS.search_dir_range()) - { - OptionalDirectoryEntryRef DR = DL.getDirRef(); - // only consider normal directories - if (!DL.isNormalDir() || !DR) - { - continue; - } - // store the normalized path - auto normPath = files::makePosixStyle(files::makeAbsolute(DR->getName(), cwd)); - search_dirs_.push_back(std::move(normPath)); - } - - // Store preprocessed information about all file entries - std::string const sourceRoot = files::makePosixStyle(files::makeAbsolute(config_->sourceRoot, cwd)); - auto cacheFileInfo = [&](FileEntry const* entry) { - std::string_view const file_path = entry->tryGetRealPathName(); - MRDOCS_CHECK_OR(!file_path.empty()); - auto const normPath = files::makePosixStyle( - files::makeAbsolute(file_path, cwd)); - auto FI = FileInfo::build(search_dirs_, normPath, sourceRoot); - files_.emplace(entry, FI); - }; - FileEntry const* mainFileEntry = source_.getFileEntryForID(source_.getMainFileID()); - cacheFileInfo(mainFileEntry); - for(const FileEntry* file : PP.getIncludedFiles()) - { - cacheFileInfo(file); - } } void @@ -2514,8 +2423,8 @@ shouldExtract( // This is not a scoped promotion because // parents and members should also assume // the same base extraction mode. - if (checkFileFilters(D) && - checkSymbolFilters(D)) + if (checkSymbolFilters(D) && + checkFileFilters(D)) { mode_ = ExtractionMode::Regular; } @@ -2523,17 +2432,17 @@ shouldExtract( return true; } + // Check if this symbol should be extracted according + // to its qualified name. This checks if it matches + // the symbol patterns and if it's not excluded. + MRDOCS_CHECK_OR(checkSymbolFilters(D), false); + // Check if this symbol should be extracted according // to its location. This checks if it's in one of the // input directories, if it matches the file patterns, // and it's not in an excluded file. MRDOCS_CHECK_OR(checkFileFilters(D), false); - // Check if this symbol should be extracted according - // to its qualified name. This checks if it matches - // the symbol patterns and if it's not excluded. - MRDOCS_CHECK_OR(checkSymbolFilters(D), false); - return true; } @@ -2776,36 +2685,132 @@ ASTVisitor::FileInfo* ASTVisitor:: findFileInfo(clang::SourceLocation const loc) { - if (loc.isInvalid()) - { - return nullptr; - } - // KRYSTIAN FIXME: we really should not be - // calling getPresumedLoc this often, + MRDOCS_CHECK_OR(!loc.isInvalid(), nullptr); + + // KRYSTIAN FIXME: we really should not be calling getPresumedLoc this often, // it's quite expensive auto const presumed = source_.getPresumedLoc(loc, false); - if (presumed.isInvalid()) + MRDOCS_CHECK_OR(!presumed.isInvalid(), nullptr); + + const FileEntry* entry = source_.getFileEntryForID( presumed.getFileID()); + MRDOCS_CHECK_OR(entry, nullptr); + + // Find in the cache + if (auto const it = files_.find(entry); it != files_.end()) { - return nullptr; + return std::addressof(it->second); } - const FileEntry* file = - source_.getFileEntryForID( - presumed.getFileID()); - // KRYSTIAN NOTE: i have no idea under what - // circumstances the file entry would be null - if (!file) + + // Build FileInfo + auto const FI = buildFileInfo(entry); + MRDOCS_CHECK_OR(FI, nullptr); + auto [it, inserted] = files_.try_emplace(entry, std::move(*FI)); + return std::addressof(it->second); +} + +std::optional +ASTVisitor:: +buildFileInfo(FileEntry const* entry) +{ + std::string_view const file_path = entry->tryGetRealPathName(); + MRDOCS_CHECK_OR(!file_path.empty(), std::nullopt); + return buildFileInfo(file_path); +} + +ASTVisitor::FileInfo +ASTVisitor:: +buildFileInfo(std::string_view const file_path) +{ + FileInfo file_info; + file_info.full_path = file_path; + if (!files::isPosixStyle(file_info.full_path)) { - return nullptr; + file_info.full_path = files::makePosixStyle(file_info.full_path); } - // KRYSTIAN NOTE: i have no idea under what - // circumstances the file would not be in either - // the main file, or an included file - auto const it = files_.find(file); - if (it == files_.end()) + + // Attempts to get a relative path for the prefix + auto tryGetRelativePosixPath = [&file_info](std::string_view const prefix) + -> std::optional { - return nullptr; + if (files::startsWith(file_info.full_path, prefix)) + { + std::string_view res = file_info.full_path; + res.remove_prefix(prefix.size()); + if (res.starts_with('/')) + { + res.remove_prefix(1); + } + return res; + } + return std::nullopt; + }; + + auto tryGetRelativePath = [&tryGetRelativePosixPath](std::string_view const prefix) + -> std::optional + { + if (!files::isAbsolute(prefix)) + { + return std::nullopt; + } + if (files::isPosixStyle(prefix)) + { + // If already posix, we use the string view directly + // to avoid creating a new string for the check + return tryGetRelativePosixPath(prefix); + } + std::string const posixPrefix = files::makePosixStyle(prefix); + return tryGetRelativePosixPath(posixPrefix); + }; + + // Find the best match for the file path in the search directories + for (HeaderSearch& HS = sema_.getPreprocessor().getHeaderSearchInfo(); + DirectoryLookup const& DL : HS.search_dir_range()) + { + OptionalDirectoryEntryRef DR = DL.getDirRef(); + if (!DL.isNormalDir() || !DR) + { + // Only consider normal directories + continue; + } + StringRef searchDir = DR->getName(); + if (auto shortPath = tryGetRelativePath(searchDir)) + { + file_info.short_path = std::string(*shortPath); + return file_info; + } + } + + // Fallback to sourceRoot + if (files::isAbsolute(config_->sourceRoot)) + { + if (auto shortPath = tryGetRelativePath(config_->sourceRoot)) + { + file_info.short_path = std::string(*shortPath); + return file_info; + } } - return &it->second; + + // Fallback to system search paths in PATH + std::optional const optEnvPathsStr = llvm::sys::Process::GetEnv("PATH"); + MRDOCS_CHECK_OR(optEnvPathsStr, file_info); + std::string const& envPathsStr = *optEnvPathsStr; + for (auto const envPaths = llvm::split(envPathsStr, llvm::sys::EnvPathSeparator); + auto envPath: envPaths) + { + if (!files::isAbsolute(envPath)) + { + continue; + } + if (auto shortPath = tryGetRelativePath(envPath)) + { + file_info.short_path = std::string(*shortPath); + return file_info; + } + } + + // Fallback to the full path + file_info.short_path = file_info.full_path; + return file_info; } template InfoTy> @@ -2840,9 +2845,10 @@ ASTVisitor:: upsert(DeclType* D) { AccessSpecifier access = getAccess(D); - MRDOCS_CHECK_MSG( - shouldExtract(D, access), - "Symbol should not be extracted"); + if (!shouldExtract(D, access)) + { + return Unexpected(Error("Symbol should not be extracted")); + } SymbolID const id = generateID(D); MRDOCS_CHECK_MSG(id, "Failed to extract symbol ID"); diff --git a/src/lib/AST/ASTVisitor.hpp b/src/lib/AST/ASTVisitor.hpp index 4783aa6a2a..1d290b1267 100644 --- a/src/lib/AST/ASTVisitor.hpp +++ b/src/lib/AST/ASTVisitor.hpp @@ -79,17 +79,6 @@ class ASTVisitor // An unordered set of all extracted Info declarations InfoSet info_; - /* Preprocessed information about search directories - - This vector stores information about the search directories - used by the translation unit. - - Whenever we extract information about where a symbol is located, - we store the full path of the symbol and the path relative - to the search directory in this vector. - */ - std::vector search_dirs_; - /* Struct to hold pre-processed file information. This struct stores information about a file, including its full path, @@ -100,13 +89,6 @@ class ASTVisitor */ struct FileInfo { - static - FileInfo - build( - std::span search_dirs, - std::string_view file_path, - std::string_view sourceRoot); - // The full path of the file. std::string full_path; @@ -910,13 +892,9 @@ class ASTVisitor This function will return a pointer to a FileInfo object for a given Clang FileEntry object. - If the FileInfo object does not exist, the function - will construct a new FileInfo object and add it to - the files_ map. - - The map of files is created during the object - construction, and is populated with the FileInfo - for each file in the translation unit. + If the FileInfo object does not exist in the cache, + the function will build a new FileInfo object and + add it to the files_ map. @param file The Clang FileEntry object to get the FileInfo for. @@ -925,6 +903,49 @@ class ASTVisitor FileInfo* findFileInfo(clang::SourceLocation loc); + /* Build a FileInfo for a FileEntry + + This function will build a FileInfo object for a + given Clang FileEntry object. + + The function will extract the full path and short + path of the file, and store the information in the + FileInfo object. + + This function is used as an auxiliary function to + `findFileInfo` when the FileInfo object does not + exist in the cache. + + @param file The Clang FileEntry object to build the FileInfo for. + @return the FileInfo object. + + */ + std::optional + buildFileInfo(FileEntry const* entry); + + /* Build a FileInfo for a string path + + This function will build a FileInfo object for a + given file path. + + The function will extract the full path and short + path of the file, and store the information in the + FileInfo object. + + The search paths will be used to identify the + short path of the file relative to the search + directories. + + This function is used as an auxiliary function to + `buildFileInfo` once the file path has been extracted + from the FileEntry object. + + @param file_path The file path to build the FileInfo for. + @return the FileInfo object. + */ + FileInfo + buildFileInfo(std::string_view file_path); + /* Result of an upsert operation This struct is used to return the result of an diff --git a/src/lib/Support/Path.cpp b/src/lib/Support/Path.cpp index 00b028bd68..5c240f6452 100644 --- a/src/lib/Support/Path.cpp +++ b/src/lib/Support/Path.cpp @@ -263,14 +263,31 @@ makeAbsolute( } std::string -makePosixStyle( - std::string_view pathName) +makePosixStyle(std::string_view pathName) { SmallPathString result(pathName); llvm::sys::path::native(result, llvm::sys::path::Style::posix); return std::string(result); } +bool +isPosixStyle(std::string_view pathName) +{ + namespace path = llvm::sys::path; + + if(pathName.empty()) + { + return true; + } + llvm::StringRef separator = llvm::sys::path::get_separator(path::Style::windows); + if (pathName.find(separator) != llvm::StringRef::npos) + { + return false; + } + return true; +} + + std::string withExtension( std::string_view fileName, From 90fcf10a282ba37747176276ebb11014bf183af0 Mon Sep 17 00:00:00 2001 From: alandefreitas Date: Fri, 17 Jan 2025 16:28:59 -0300 Subject: [PATCH 3/3] support source paths in Location objects Location objects only contained the full path and the path relative to the search directories. This commit includes a new field for the source path, which contains the path relative to the `source-root`. #feat fix #798 --- CMakeLists.txt | 2 +- docs/modules/ROOT/pages/generators.adoc | 20 +- docs/mrdocs.schema.json | 2 +- include/mrdocs/Metadata/Source.hpp | 22 +- mrdocs.rnc | 4 +- .../common/partials/location/source.hbs | 8 +- src/lib/AST/ASTVisitor.cpp | 26 +- src/lib/AST/ASTVisitor.hpp | 3 + src/lib/Gen/xml/XMLWriter.cpp | 14 +- src/lib/Lib/ConfigOptions.json | 2 +- src/lib/Metadata/Reduce.cpp | 8 +- src/lib/Metadata/Source.cpp | 5 +- test-files/golden-tests/core/libcxx.xml | 2 +- test-files/golden-tests/core/utf-8.xml | 2 +- .../filters/file/include-self.xml | 2 +- .../filters/symbol-name/blacklist_0.xml | 10 +- .../symbol-name/excluded-namespace-alias.xml | 2 +- .../filters/symbol-name/extraction-mode.xml | 52 +- .../symbol-name/impl-defined-member.xml | 8 +- .../filters/symbol-name/whitelist_0.xml | 6 +- .../symbol-type/nested-private-template.xml | 6 +- test-files/golden-tests/javadoc/brief-1.xml | 4 +- test-files/golden-tests/javadoc/brief-2.xml | 14 +- test-files/golden-tests/javadoc/brief-3.xml | 8 +- test-files/golden-tests/javadoc/code.xml | 56 +- test-files/golden-tests/javadoc/commands.xml | 2 +- .../golden-tests/javadoc/duplicate-jdoc.xml | 12 +- test-files/golden-tests/javadoc/li.xml | 2 +- test-files/golden-tests/javadoc/par-1.xml | 8 +- test-files/golden-tests/javadoc/para-1.xml | 8 +- test-files/golden-tests/javadoc/para-2.xml | 2 +- test-files/golden-tests/javadoc/para-3.xml | 2 +- .../golden-tests/javadoc/param-direction.xml | 26 +- test-files/golden-tests/javadoc/param.xml | 8 +- test-files/golden-tests/javadoc/pre-post.xml | 2 +- test-files/golden-tests/javadoc/ref.xml | 104 +-- test-files/golden-tests/javadoc/styled.xml | 110 +-- test-files/golden-tests/javadoc/throw.xml | 2 +- .../golden-tests/metadata/alias-template.xml | 10 +- .../golden-tests/metadata/attributes_1.xml | 2 +- .../metadata/class-private-alias.xml | 4 +- .../metadata/class-template-partial-spec.xml | 8 +- .../metadata/class-template-spec.xml | 36 +- .../class-template-specializations-1.xml | 844 +++++++++--------- .../class-template-specializations-2.xml | 50 +- .../class-template-specializations-3.xml | 130 +-- .../golden-tests/metadata/class-template.xml | 46 +- test-files/golden-tests/metadata/concept.xml | 6 +- .../metadata/decay-to-primary.xml | 22 +- .../metadata/dependency-propagation.xml | 10 +- test-files/golden-tests/metadata/enum.xml | 24 +- .../metadata/explicit-conv-operator.xml | 16 +- .../golden-tests/metadata/explicit-ctor.xml | 40 +- .../metadata/explicit-deduct-guide.xml | 10 +- .../metadata/explicit-object-parameter.xml | 6 +- test-files/golden-tests/metadata/friend-1.xml | 6 +- test-files/golden-tests/metadata/friend-2.xml | 8 +- test-files/golden-tests/metadata/friend-3.xml | 14 +- test-files/golden-tests/metadata/friend-4.xml | 14 +- test-files/golden-tests/metadata/friend-5.xml | 14 +- test-files/golden-tests/metadata/friend-6.xml | 16 +- .../metadata/function-parm-decay.xml | 36 +- .../metadata/function-template-template.xml | 36 +- .../metadata/function-template.xml | 24 +- .../metadata/function-tparm-decay.xml | 36 +- .../implicit-instantiation-member-ref.xml | 22 +- .../golden-tests/metadata/local-class.xml | 4 +- test-files/golden-tests/metadata/mem-fn.xml | 76 +- .../metadata/namespace-alias-1.xml | 2 +- .../metadata/namespace-alias-2.xml | 4 +- .../metadata/namespace-alias-3.xml | 4 +- .../golden-tests/metadata/namespace.xml | 24 +- .../metadata/no_unique_address.xml | 8 +- test-files/golden-tests/metadata/noreturn.xml | 10 +- .../golden-tests/metadata/ns-variables.xml | 24 +- .../metadata/out-of-line-record-def.xml | 4 +- .../golden-tests/metadata/overloaded-op-1.xml | 4 +- .../golden-tests/metadata/overloaded-op-2.xml | 4 +- .../golden-tests/metadata/overloads.xml | 28 +- test-files/golden-tests/metadata/record-1.xml | 18 +- .../golden-tests/metadata/record-access.xml | 24 +- .../golden-tests/metadata/record-data.xml | 42 +- .../metadata/record-inheritance.xml | 34 +- .../golden-tests/metadata/requires-clause.xml | 16 +- test-files/golden-tests/metadata/sfinae.xml | 26 +- .../spec-mem-implicit-instantiation.xml | 70 +- .../metadata/static-data-def-constexpr.xml | 12 +- .../golden-tests/metadata/static-data-def.xml | 38 +- .../metadata/static-data-template.xml | 8 +- .../template-specialization-inheritance.xml | 28 +- .../golden-tests/metadata/type-resolution.xml | 100 +-- test-files/golden-tests/metadata/union.xml | 14 +- test-files/golden-tests/metadata/using-2.xml | 8 +- test-files/golden-tests/metadata/using-3.xml | 14 +- .../metadata/var-inline-constexpr.xml | 20 +- .../golden-tests/metadata/var-template.xml | 14 +- .../metadata/variadic-function.xml | 10 +- .../golden-tests/output/canonical_1.xml | 16 +- test-files/golden-tests/snippets/distance.xml | 2 +- test-files/golden-tests/snippets/is_prime.xml | 2 +- test-files/golden-tests/snippets/sqrt.xml | 2 +- .../golden-tests/snippets/terminate.xml | 2 +- .../templates/c_mct_expl_inline.xml | 10 +- .../templates/c_mct_expl_outside.xml | 10 +- .../templates/c_mft_expl_inline.xml | 6 +- .../templates/c_mft_expl_outside.xml | 6 +- test-files/golden-tests/templates/ct_expl.xml | 8 +- test-files/golden-tests/templates/ct_mc.xml | 6 +- .../templates/ct_mc_expl_outside.xml | 14 +- test-files/golden-tests/templates/ct_mct.xml | 6 +- .../templates/ct_mct_expl_inline.xml | 10 +- .../templates/ct_mct_expl_outside.xml | 14 +- test-files/golden-tests/templates/ct_mf.xml | 4 +- .../templates/ct_mf_expl_outside.xml | 10 +- test-files/golden-tests/templates/ct_mft.xml | 4 +- .../templates/ct_mft_expl_inline.xml | 6 +- .../templates/ct_mft_expl_outside.xml | 10 +- test-files/golden-tests/templates/ft_expl.xml | 4 +- 118 files changed, 1467 insertions(+), 1443 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 6a628b7e57..a31af70c02 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -440,7 +440,7 @@ if (MRDOCS_BUILD_TESTS) DEPENDS mrdocs.rnc) add_custom_target(mrdocs_rng ALL DEPENDS mrdocs.rng) - file(GLOB_RECURSE XML_SOURCES CONFIGURE_DEPENDS test-files/*.xml) + file(GLOB_RECURSE XML_SOURCES CONFIGURE_DEPENDS test-files/golden-tests/*.xml) add_test(NAME xml-lint COMMAND ${LIBXML2_XMLLINT_EXECUTABLE} --dropdtd --noout --relaxng ${CMAKE_CURRENT_BINARY_DIR}/mrdocs.rng ${XML_SOURCES} diff --git a/docs/modules/ROOT/pages/generators.adoc b/docs/modules/ROOT/pages/generators.adoc index 94f7110592..7d057bc98a 100644 --- a/docs/modules/ROOT/pages/generators.adoc +++ b/docs/modules/ROOT/pages/generators.adoc @@ -862,25 +862,25 @@ The location object has the following properties: |=== |Property |Type| Description -| `path` +| `fullPath` | `string` -| The path of the source file. +| The full path of the source file. -| `file` +| `shortPath` | `string` -| The filename of the source file. +| The path of the source file relative to the search directories. + +| `sourcePath` +| `string` +| The path of the source file relative to the `source-root`. | `line` | `integer` -| The line number of the symbol. - -| `kind` -| `string` -| The kind of file (e.g., `source`, `system`, `other`). +| The line number of the symbol at this location. | `documented` | `bool` -| Whether the symbol is documented. +| Whether the symbol is documented at this location. |=== [#tparam-fields] diff --git a/docs/mrdocs.schema.json b/docs/mrdocs.schema.json index 281d139942..17ff4f5e8d 100644 --- a/docs/mrdocs.schema.json +++ b/docs/mrdocs.schema.json @@ -225,7 +225,7 @@ }, "source-root": { "default": "", - "description": "Path to the root directory of the source code. This path is used as a default for input files and a base for relative paths formed from absolute paths.", + "description": "Path to the root directory of the source code. This path is used as a default for input files and a base for relative paths formed from absolute paths. This should typically be the root directory of the git project, as relative paths formed from it can be used to create links to these source files in the repository. Templates use the `base-url` option to create links to the source code.", "title": "Path to the root directory of the source code", "type": "string" }, diff --git a/include/mrdocs/Metadata/Source.hpp b/include/mrdocs/Metadata/Source.hpp index 65f9be68ea..b331958e1e 100644 --- a/include/mrdocs/Metadata/Source.hpp +++ b/include/mrdocs/Metadata/Source.hpp @@ -51,11 +51,15 @@ struct MRDOCS_DECL { /** The full file path */ - std::string Path; + std::string FullPath; - /** Name of the file + /** The file path relative to one of the search directories */ - std::string Filename; + std::string ShortPath; + + /** The file path relative to the source-root directory + */ + std::string SourcePath; /** Line number within the file */ @@ -68,12 +72,14 @@ struct MRDOCS_DECL //-------------------------------------------- Location( - std::string_view const filepath = {}, - std::string_view const filename = {}, + std::string_view const full_path = {}, + std::string_view const short_path = {}, + std::string_view const source_path = {}, unsigned const line = 0, bool const documented = false) - : Path(filepath) - , Filename(filename) + : FullPath(full_path) + , ShortPath(short_path) + , SourcePath(source_path) , LineNumber(line) , Documented(documented) { @@ -92,7 +98,7 @@ struct LocationEmptyPredicate constexpr bool operator()( Location const& loc) const noexcept { - return loc.Filename.empty(); + return loc.ShortPath.empty(); } }; diff --git a/mrdocs.rnc b/mrdocs.rnc index baa8e5974e..bf0aee0287 100644 --- a/mrdocs.rnc +++ b/mrdocs.rnc @@ -326,7 +326,9 @@ grammar Location = element file { - attribute path {text}, + # attribute full-path {text}, + attribute short-path {text}, + attribute source-path {text}, attribute line {text}, attribute class {"def"} ?, empty diff --git a/share/mrdocs/addons/generator/common/partials/location/source.hbs b/share/mrdocs/addons/generator/common/partials/location/source.hbs index dc7cf379fa..094c4fbadf 100644 --- a/share/mrdocs/addons/generator/common/partials/location/source.hbs +++ b/share/mrdocs/addons/generator/common/partials/location/source.hbs @@ -11,9 +11,9 @@ --}} Declared in {{#>markup/code~}} - {{ str "<" }}{{#unless (and @root.config.base-url (eq dcl.kind "source"))~}} - {{dcl.file}} + {{ str "<" }}{{#if (and @root.config.base-url dcl.shortPath dcl.sourcePath)~}} + {{#>markup/a href=(concat @root.config.base-url dcl.sourcePath '#L' dcl.line)}}{{dcl.shortPath}}{{/markup/a}} {{~else~}} - {{#>markup/a href=(concat @root.config.base-url dcl.file '#L' dcl.line)}}{{dcl.file}}{{/markup/a}} - {{~/unless~}}{{ str ">" }} + {{dcl.shortPath}} + {{~/if~}}{{ str ">" }} {{~/markup/code}} diff --git a/src/lib/AST/ASTVisitor.cpp b/src/lib/AST/ASTVisitor.cpp index 551ee2c099..1ab9e0f5a5 100644 --- a/src/lib/AST/ASTVisitor.cpp +++ b/src/lib/AST/ASTVisitor.cpp @@ -696,7 +696,7 @@ populate( { return; } - I.DefLoc.emplace(file->full_path, file->short_path, line, documented); + I.DefLoc.emplace(file->full_path, file->short_path, file->source_path, line, documented); } else { @@ -705,13 +705,13 @@ populate( [line, file](const Location& l) { return l.LineNumber == line && - l.Path == file->full_path; + l.FullPath == file->full_path; }); if (existing != I.Loc.end()) { return; } - I.Loc.emplace_back(file->full_path, file->short_path, line, documented); + I.Loc.emplace_back(file->full_path, file->short_path, file->source_path, line, documented); } } @@ -2762,6 +2762,15 @@ buildFileInfo(std::string_view const file_path) return tryGetRelativePosixPath(posixPrefix); }; + // Populate file relative to source-root + if (files::isAbsolute(config_->sourceRoot)) + { + if (auto shortPath = tryGetRelativePath(config_->sourceRoot)) + { + file_info.source_path = std::string(*shortPath); + } + } + // Find the best match for the file path in the search directories for (HeaderSearch& HS = sema_.getPreprocessor().getHeaderSearchInfo(); DirectoryLookup const& DL : HS.search_dir_range()) @@ -2780,14 +2789,11 @@ buildFileInfo(std::string_view const file_path) } } - // Fallback to sourceRoot - if (files::isAbsolute(config_->sourceRoot)) + // Fallback to the source root + if (!file_info.source_path.empty()) { - if (auto shortPath = tryGetRelativePath(config_->sourceRoot)) - { - file_info.short_path = std::string(*shortPath); - return file_info; - } + file_info.short_path = file_info.source_path; + return file_info; } // Fallback to system search paths in PATH diff --git a/src/lib/AST/ASTVisitor.hpp b/src/lib/AST/ASTVisitor.hpp index 1d290b1267..fec2b9aee3 100644 --- a/src/lib/AST/ASTVisitor.hpp +++ b/src/lib/AST/ASTVisitor.hpp @@ -95,6 +95,9 @@ class ASTVisitor // The file path relative to a search directory. std::string short_path; + // The file path relative to the source-root directory. + std::string source_path; + // Whether this file passes the file filters std::optional passesFilters; }; diff --git a/src/lib/Gen/xml/XMLWriter.cpp b/src/lib/Gen/xml/XMLWriter.cpp index fa6f2c068c..a950dba8de 100644 --- a/src/lib/Gen/xml/XMLWriter.cpp +++ b/src/lib/Gen/xml/XMLWriter.cpp @@ -626,10 +626,14 @@ XMLWriter:: writeSourceInfo( SourceInfo const& I) { - if(I.DefLoc) + if (I.DefLoc) + { writeLocation(*I.DefLoc, true); - for(auto const& loc : I.Loc) + } + for (auto const& loc: I.Loc) + { writeLocation(loc, false); + } } void @@ -639,9 +643,11 @@ writeLocation( bool def) { tags_.write("file", {}, { - { "path", loc.Filename }, + // { "full-path", loc.FullPath }, + { "short-path", loc.ShortPath }, + { "source-path", loc.SourcePath }, { "line", std::to_string(loc.LineNumber) }, - { "class", "def", def } }); + { "class", "def", def }}); } //------------------------------------------------ diff --git a/src/lib/Lib/ConfigOptions.json b/src/lib/Lib/ConfigOptions.json index dee5e87e5b..6e76d703b8 100644 --- a/src/lib/Lib/ConfigOptions.json +++ b/src/lib/Lib/ConfigOptions.json @@ -42,7 +42,7 @@ { "name": "source-root", "brief": "Path to the root directory of the source code", - "details": "Path to the root directory of the source code. This path is used as a default for input files and a base for relative paths formed from absolute paths.", + "details": "Path to the root directory of the source code. This path is used as a default for input files and a base for relative paths formed from absolute paths. This should typically be the root directory of the git project, as relative paths formed from it can be used to create links to these source files in the repository. Templates use the `base-url` option to create links to the source code.", "type": "dir-path", "default": "", "relative-to": "", diff --git a/src/lib/Metadata/Reduce.cpp b/src/lib/Metadata/Reduce.cpp index 8f58d6af16..877a413dce 100644 --- a/src/lib/Metadata/Reduce.cpp +++ b/src/lib/Metadata/Reduce.cpp @@ -27,8 +27,8 @@ struct LocationEqual Location const& L1) const noexcept { return - std::tie(L0.LineNumber, L0.Filename) == - std::tie(L1.LineNumber, L1.Filename); + std::tie(L0.LineNumber, L0.FullPath) == + std::tie(L1.LineNumber, L1.FullPath); } }; @@ -43,8 +43,8 @@ struct LocationLess Location const& L1) const noexcept { return - std::tie(L0.LineNumber, L0.Filename) < - std::tie(L1.LineNumber, L1.Filename); + std::tie(L0.LineNumber, L0.FullPath) < + std::tie(L1.LineNumber, L1.FullPath); } }; diff --git a/src/lib/Metadata/Source.cpp b/src/lib/Metadata/Source.cpp index 27ca99857e..f3878036f3 100644 --- a/src/lib/Metadata/Source.cpp +++ b/src/lib/Metadata/Source.cpp @@ -39,8 +39,9 @@ tag_invoke( IO& io, Location const& loc) { - io.map("path", loc.Path); - io.map("file", loc.Filename); + io.map("fullPath", loc.FullPath); + io.map("shortPath", loc.ShortPath); + io.map("sourcePath", loc.SourcePath); io.map("line", loc.LineNumber); io.map("documented", loc.Documented); } diff --git a/test-files/golden-tests/core/libcxx.xml b/test-files/golden-tests/core/libcxx.xml index fd73826747..f961338fbd 100644 --- a/test-files/golden-tests/core/libcxx.xml +++ b/test-files/golden-tests/core/libcxx.xml @@ -5,7 +5,7 @@