Skip to content
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 @@ void AddIncludePaths(llvm::StringRef PathStr,
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 @@ void AddIncludePaths(llvm::StringRef PathStr,
#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,
clang::HeaderSearchOptions& HOpts,
const char* Delim /* = Cpp::utils::platform::kEnvDelim */) {
#define DEBUG_TYPE "GetIncludePaths"

const int val = 10;
llvm::SmallVector<llvm::StringRef, val> Paths;
if ((Delim != nullptr) && (*Delim != 0))
SplitPaths(PathStr, Paths, kAllowNonExistant, Delim, HOpts.Verbose);
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) {
bool Exists = false;
for (const clang::HeaderSearchOptions::Entry& E : HOpts.UserEntries) {
if ((E.Path == Path))
Exists = true;
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);
}