Skip to content

Change working directory of C++ parser #572

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
andocz wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Change working directory of C++ parser #572

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
c46d0bb
Make C++ parser use the working directory of compilation commands
andocz Mar 28, 2022
5e257ab
Don't make source file path absolute in ClangTool invocation
andocz Mar 28, 2022
ce956a8
Fix inaccuracies in SourceManager::getFile's comments
andocz Mar 28, 2022
96345c4
Add PrefixingFileSystem that emulates chdir even for non-existent dirs
andocz Mar 28, 2022
19cda25
Rename PrefixingFileSystem to FakeWorkDirFileSystem
andocz Mar 30, 2022
581d2e9
Remove FakeWorkDirFileSystem
andocz Apr 4, 2022
cca922e
Save build directory in database
andocz Apr 4, 2022
10de276
Use saved build directory when invoking Clang tool in reparser
andocz Apr 4, 2022
ecae845
Convert all path arguments to canonical in DatabaseFileSystem
andocz Apr 4, 2022
e0c6dba
If the build directory doesn't exist, use '/' and emit warning
andocz Apr 4, 2022
03cb8e9
Fix segfault in PPIncludeCallback::InclusionDirective
andocz Apr 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions model/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
set(ODB_SOURCES
include/model/buildaction.h
include/model/builddirectory.h
include/model/buildlog.h
include/model/buildsourcetarget.h
include/model/filecontent.h
Expand Down
37 changes: 37 additions & 0 deletions model/include/model/builddirectory.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
#ifndef CC_MODEL_BUILDDIRECTORY_H
#define CC_MODEL_BUILDDIRECTORY_H

#include <memory>

#include <odb/core.hxx>

#include <model/buildaction.h>
#include <model/file.h>

namespace cc
{
namespace model
{

struct BuildAction;

#pragma db object
struct BuildDirectory
Copy link
Contributor

@whisperity whisperity Apr 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why this has to be a brand new table, foreign key, and all that jazz for the storage. BuildAction already contains string command;, for example. If "directory" is just another member of the entries in a compilation database JSON (just like "command"), then why isn't this reflected in the schema? Every build action MUST have a build directory (it just may or may not exist or point to anything meaningful).

This struct, as it is now, won't do uniqueing of build directories either, because it is the build directory that refers to one build action each. If we want a separate record to only store one directory ("string") exactly once, then it is the buildaction that has to refer into this relation's key.

Copy link
Contributor Author

@andocz andocz Apr 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to keep backwards compatibility with existing databases, that's why I didn't want to change BuildAction's schema. Is that not a priority? In that case you're right, storing it in BuildAction would make more sense.

Deduplicating the strings is not worth the hassle, in the CodeCompass SQLite database BuildAction only takes up 0.012% of the space.

Copy link
Collaborator

@mcserep mcserep Feb 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can easily reparse projects, backward-compatibility with existing databases is not a high priority.

{
#pragma db id auto
std::uint64_t id;

#pragma db not_null
std::string directory;

#pragma db not_null
#pragma db on_delete(cascade)
std::shared_ptr<BuildAction> action;
};

typedef std::shared_ptr<BuildDirectory> BuildDirectoryPtr;

} // model
} // cc

#endif // CC_MODEL_BUILDDIRECTORY_H
7 changes: 7 additions & 0 deletions plugins/cpp/parser/src/cppparser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@

#include <model/buildaction.h>
#include <model/buildaction-odb.hxx>
#include <model/builddirectory.h>
#include <model/builddirectory-odb.hxx>
#include <model/buildsourcetarget.h>
#include <model/buildsourcetarget-odb.hxx>
#include <model/file.h>
Expand Down Expand Up @@ -254,6 +256,7 @@ void CppParser::addCompileCommand(

std::vector<model::BuildSource> sources;
std::vector<model::BuildTarget> targets;
model::BuildDirectory buildDir;

for (const auto& srcTarget : extractInputOutputs(command_))
{
Expand All @@ -277,6 +280,9 @@ void CppParser::addCompileCommand(

targets.push_back(std::move(buildTarget));
}

buildDir.directory = command_.Directory;
buildDir.action = buildAction_;

_ctx.srcMgr.persistFiles();

Expand All @@ -285,6 +291,7 @@ void CppParser::addCompileCommand(
_ctx.db->persist(buildSource);
for (model::BuildTarget buildTarget : targets)
_ctx.db->persist(buildTarget);
_ctx.db->persist(buildDir);
});
}

Expand Down