Skip to content
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.

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

Add support for GetIncludePaths #178

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions include/clang/Interpreter/CppInterOp.h
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,9 @@ namespace Cpp {
/// Returns the resource-dir path (for headers).
const char* GetResourceDir();

/// Get Include Paths
void GetIncludePath(std::vector<std::string>& includePaths);

/// Secondary search path for headers, if not found using the
/// GetResourceDir() function.
void AddIncludePath(const char *dir);
Expand Down
4 changes: 4 additions & 0 deletions lib/Interpreter/CppInterOp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2530,6 +2530,10 @@ namespace Cpp {
getInterp().AddIncludePath(dir);
}

void GetIncludePath(std::vector<std::string>& Paths) {
getInterp().GetIncludePath(Paths);
}

namespace {

class clangSilent {
Expand Down
17 changes: 16 additions & 1 deletion lib/Interpreter/CppInterOpInterpreter.h
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,10 @@ class Interpreter {
const_cast<const Interpreter*>(this)->getDynamicLibraryManager());
}

/// @brief As a interface to store paths added in AddIncludePaths
std::vector<std::string> include;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: member variable 'include' has public visibility [cppcoreguidelines-non-private-member-variables-in-classes]

  std::vector<std::string> include;
                           ^

Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need this. The include paths are stored in the HeaderSearch already.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So we know that AddincludePaths() is fetching the paths and giving it to HeaderSearch, now GetIncludePaths() had the aim of exposing all the paths which were added(not present in HeaderSearch) to the user, for this we had to get/store all those paths which were being added to HeaderSearch with a vector ( as an interface) as AddIncludePaths() does'nt store any paths (makes it available for HeaderSearch).

Initially, I tried other methods to store all the paths from AddIncludePaths() itself but for that we require a change in function definition and would not require another function GetIncludePaths(). I also tried creating wrappers around AddIncludePaths() but to maintain the function signature I could'nt find a way to store the paths.

///

///\brief Adds multiple include paths separated by a delimter.
///
///\param[in] PathsStr - Path(s)
Expand All @@ -359,7 +363,7 @@ class Interpreter {

// Save the current number of entries
size_t Idx = HOpts.UserEntries.size();
Cpp::utils::AddIncludePaths(PathsStr, HOpts, Delim);
Cpp::utils::GetIncludePaths(include, PathsStr, HOpts, Delim);

clang::Preprocessor& PP = CI->getPreprocessor();
clang::SourceManager& SM = PP.getSourceManager();
Expand All @@ -383,6 +387,17 @@ class Interpreter {
return AddIncludePaths(PathsStr, nullptr);
}

///\brief Stores include paths.
///\param[in] includePaths - Store Path(s)
///
void GetIncludePaths(std::vector<std::string>& includePaths) {
includePaths = std::move(include);
}

void GetIncludePath(std::vector<std::string>& includePaths) {
return GetIncludePaths(includePaths);
}

CompilationResult loadLibrary(const std::string& filename, bool lookup) {
DynamicLibraryManager* DLM = getDynamicLibraryManager();
std::string canonicalLib;
Expand Down
28 changes: 27 additions & 1 deletion lib/Interpreter/Paths.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,6 @@
if (!Exists)
PathsChecked.push_back(Path);
}

const bool IsFramework = false;
const bool IsSysRootRelative = true;
for (llvm::StringRef Path : PathsChecked)
Expand All @@ -449,5 +448,32 @@
#undef DEBUG_TYPE
}

void GetIncludePaths(
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]

      if ((Exists = E.Path == Path))
           ^
Additional context

lib/Interpreter/Paths.cpp:450: if it should be an assignment, move it out of the 'if' condition

      if ((Exists = E.Path == Path))
           ^

lib/Interpreter/Paths.cpp:450: if it is meant to be an equality check, change '=' to '=='

      if ((Exists = E.Path == Path))
           ^

std::vector<std::string>& includePaths, llvm::StringRef PathStr,

Check warning on line 452 in lib/Interpreter/Paths.cpp

View check run for this annotation

Codecov / codecov/patch

lib/Interpreter/Paths.cpp#L451-L452

Added lines #L451 - L452 were not covered by tests
clang::HeaderSearchOptions& HOpts,
const char* Delim /* = Cpp::utils::platform::kEnvDelim */) {
#define DEBUG_TYPE "GetIncludePaths"

Check warning on line 455 in lib/Interpreter/Paths.cpp

View check run for this annotation

Codecov / codecov/patch

lib/Interpreter/Paths.cpp#L454-L455

Added lines #L454 - L455 were not covered by tests

const int val = 10;
llvm::SmallVector<llvm::StringRef, val> Paths;
if ((Delim != nullptr) && (*Delim != 0))
SplitPaths(PathStr, Paths, kAllowNonExistant, Delim, HOpts.Verbose);

Check warning on line 460 in lib/Interpreter/Paths.cpp

View check run for this annotation

Codecov / codecov/patch

lib/Interpreter/Paths.cpp#L457-L460

Added lines #L457 - L460 were not covered by tests
else
Paths.push_back(PathStr);

// Avoid duplicates
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: unused variable 'Path' [clang-diagnostic-unused-variable]

    for (llvm::StringRef Path : PathsChecked)
                         ^

for (llvm::StringRef Path : Paths) {

Check warning on line 465 in lib/Interpreter/Paths.cpp

View check run for this annotation

Codecov / codecov/patch

lib/Interpreter/Paths.cpp#L462-L465

Added lines #L462 - L465 were not covered by tests
bool Exists = false;
for (const clang::HeaderSearchOptions::Entry& E : HOpts.UserEntries) {
if ((E.Path == Path))
Exists = true;

Check warning on line 469 in lib/Interpreter/Paths.cpp

View check run for this annotation

Codecov / codecov/patch

lib/Interpreter/Paths.cpp#L469

Added line #L469 was not covered by tests
break;
}
if (!Exists)
includePaths.push_back((std::string)Path);
}
#undef DEBUG_TYPE
}

} // namespace utils
} // namespace Cpp
13 changes: 13 additions & 0 deletions lib/Interpreter/Paths.h
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,19 @@ bool LookForFile(const std::vector<const char*>& Args, std::string& File,
void AddIncludePaths(llvm::StringRef PathStr, clang::HeaderSearchOptions& HOpts,
const char* Delim = Cpp::utils::platform::kEnvDelim);

///\brief Store multiple include paths separated by a delimter into the
/// given HeaderSearchOptions. This stores the paths but does no further
/// processing.
///
///\param[in] includePaths - To store paths added in HeaderSearchOptions
///\param[in] PathStr - Path(s)
///\param[in] Opts - HeaderSearchOptions to add paths into
///\param[in] Delim - Delimiter to separate paths or NULL if a single path
///
void GetIncludePaths(std::vector<std::string>& includePaths,
llvm::StringRef PathStr, clang::HeaderSearchOptions& HOpts,
const char* Delim = Cpp::utils::platform::kEnvDelim);

///\brief Write to cling::errs that directory does not exist in a format
/// matching what 'clang -v' would do
///
Expand Down
8 changes: 8 additions & 0 deletions unittests/CppInterOp/InterpreterTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,3 +84,11 @@ TEST(InterpreterTest, CreateInterpreter) {
EXPECT_TRUE(Cpp::GetNamed("cpp17"));
EXPECT_FALSE(Cpp::GetNamed("cppUnknown"));
}

TEST(InterpreterTest, GetIncludePath) {
std::vector <std::string> includePaths {};
const char* dir = Cpp::GetResourceDir();
Cpp::AddIncludePath(dir);
Cpp::GetIncludePath(includePaths);
EXPECT_TRUE(includePaths.size() > 0);
}
Loading