Preserve relativeness of included paths (w.r.t. current directory) (#428)

Co-authored-by: Tal Hadad <thadad@istraresearch.com>

Author: Tal500

integration_test.py

@@ -1,6 +1,7 @@
 ## test with python -m pytest integration_test.py
 
 import os
+import pathlib
 import pytest
 from testutils import simplecpp, format_include_path_arg, format_include
 
@@ -14,6 +15,7 @@ def __test_relative_header_create_header(dir, with_pragma_once=True):
                 #else
                 #error header_was_already_included
                 #endif
+                const int dummy = 1;
                 """)
     return header_file, "error: #error header_was_already_included"
 
@@ -48,33 +50,43 @@ def test_relative_header_1(tmpdir, with_pragma_once, is_sys):
         assert double_include_error in stderr
 
 @pytest.mark.parametrize("inv", (False, True))
-def test_relative_header_2(tmpdir, inv):
+@pytest.mark.parametrize("source_relative", (False, True))
+def test_relative_header_2(tmpdir, inv, source_relative):
     header_file, _ = __test_relative_header_create_header(tmpdir)
 
     test_file = __test_relative_header_create_source(tmpdir, "test.h", header_file, inv=inv)
 
-    args = [test_file]
+    args = ["test.c" if source_relative else test_file]
 
-    _, _, stderr = simplecpp(args, cwd=tmpdir)
+    _, stdout, stderr = simplecpp(args, cwd=tmpdir)
     assert stderr == ''
+    if source_relative and not inv:
+        assert '#line 8 "test.h"' in stdout
+    else:
+        assert f'#line 8 "{pathlib.PurePath(tmpdir).as_posix()}/test.h"' in stdout
 
 @pytest.mark.parametrize("is_sys", (False, True))
 @pytest.mark.parametrize("inv", (False, True))
-def test_relative_header_3(tmpdir, is_sys, inv):
+@pytest.mark.parametrize("source_relative", (False, True))
+def test_relative_header_3(tmpdir, is_sys, inv, source_relative):
     test_subdir = os.path.join(tmpdir, "test_subdir")
     os.mkdir(test_subdir)
     header_file, _ = __test_relative_header_create_header(test_subdir)
 
     test_file = __test_relative_header_create_source(tmpdir, "test_subdir/test.h", header_file, is_include1_sys=is_sys, inv=inv)
 
-    args = [test_file]
+    args = ["test.c" if source_relative else test_file]
 
-    _, _, stderr = simplecpp(args, cwd=tmpdir)
+    _, stdout, stderr = simplecpp(args, cwd=tmpdir)
 
     if is_sys:
         assert "missing header: Header not found" in stderr
     else:
         assert stderr == ''
+        if source_relative and not inv:
+            assert '#line 8 "test_subdir/test.h"' in stdout
+        else:
+            assert f'#line 8 "{pathlib.PurePath(test_subdir).as_posix()}/test.h"' in stdout
 
 @pytest.mark.parametrize("use_short_path", (False, True))
 @pytest.mark.parametrize("is_sys", (False, True))

simplecpp.cpp

@@ -3145,11 +3145,11 @@ static std::string openHeader(std::ifstream &f, const std::string &path)
     return "";
 }
 
-static std::string getRelativeFileName(const std::string &sourcefile, const std::string &header)
+static std::string getRelativeFileName(const std::string &baseFile, const std::string &header)
 {
     std::string path;
-    if (sourcefile.find_first_of("\\/") != std::string::npos)
-        path = sourcefile.substr(0, sourcefile.find_last_of("\\/") + 1U) + header;
+    if (baseFile.find_first_of("\\/") != std::string::npos)
+        path = baseFile.substr(0, baseFile.find_last_of("\\/") + 1U) + header;
     else
         path = header;
     return simplecpp::simplifyPath(path);
@@ -3160,12 +3160,22 @@ static std::string openHeaderRelative(std::ifstream &f, const std::string &sourc
     return openHeader(f, getRelativeFileName(sourcefile, header));
 }
 
+// returns the simplified header path:
+// * If the header path is absolute, returns it in absolute path
+// * Otherwise, returns it in relative path with respect to the current directory
 static std::string getIncludePathFileName(const std::string &includePath, const std::string &header)
 {
-    std::string path = toAbsolutePath(includePath);
-    if (!path.empty() && path[path.size()-1U]!='/' && path[path.size()-1U]!='\\')
-        path += '/';
-    return path + header;
+    std::string simplifiedHeader = simplecpp::simplifyPath(header);
+
+    if (isAbsolutePath(simplifiedHeader)) {
+        return simplifiedHeader;
+    }
+
+    std::string basePath = toAbsolutePath(includePath);
+    if (!basePath.empty() && basePath[basePath.size()-1U]!='/' && basePath[basePath.size()-1U]!='\\')
+        basePath += '/';
+    const std::string absolutesimplifiedHeaderPath = basePath + simplifiedHeader;
+    return extractRelativePathFromAbsolute(absolutesimplifiedHeaderPath).first;
 }
 
 static std::string openHeaderIncludePath(std::ifstream &f, const simplecpp::DUI &dui, const std::string &header)
@@ -3183,17 +3193,16 @@ static std::string openHeader(std::ifstream &f, const simplecpp::DUI &dui, const
     if (isAbsolutePath(header))
         return openHeader(f, header);
 
-    if (systemheader) {
-        // always return absolute path for systemheaders
-        return toAbsolutePath(openHeaderIncludePath(f, dui, header));
+    // prefer first to search the header relatively to source file if found, when not a system header
+    if (!systemheader) {
+        std::string relativeHeader = openHeaderRelative(f, sourcefile, header);
+        if (!relativeHeader.empty()) {
+            return relativeHeader;
+        }
     }
 
-    std::string ret;
-
-    ret = openHeaderRelative(f, sourcefile, header);
-    if (ret.empty())
-        return toAbsolutePath(openHeaderIncludePath(f, dui, header));// in a similar way to system headers
-    return ret;
+    // search the header on the include paths (provided by the flags "-I...")
+    return openHeaderIncludePath(f, dui, header);
 }
 
 static std::string findPathInMapBothRelativeAndAbsolute(const std::map<std::string, simplecpp::TokenList *> &filedata, const std::string& path) {
@@ -3212,8 +3221,9 @@ static std::string findPathInMapBothRelativeAndAbsolute(const std::map<std::stri
         }
     } else {
         const std::string absolutePath = toAbsolutePath(path);
-        if (filedata.find(absolutePath) != filedata.end())
+        if (filedata.find(absolutePath) != filedata.end()) {
             return absolutePath;
+        }
     }
     // otherwise
     return "";
@@ -3226,7 +3236,10 @@ static std::string getFileIdPath(const std::map<std::string, simplecpp::TokenLis
     }
     if (isAbsolutePath(header)) {
         const std::string simplifiedHeaderPath = simplecpp::simplifyPath(header);
-        return (filedata.find(simplifiedHeaderPath) != filedata.end()) ? simplifiedHeaderPath : "";
+        const std::string match = findPathInMapBothRelativeAndAbsolute(filedata, simplifiedHeaderPath);
+        if (!match.empty()) {
+            return match;
+        }
     }
 
     if (!systemheader) {
@@ -3235,18 +3248,17 @@ static std::string getFileIdPath(const std::map<std::string, simplecpp::TokenLis
         if (!match.empty()) {
             return match;
         }
+    } else if (filedata.find(header) != filedata.end()) {
+        return header;// system header that its file is already in the filedata - return that as is
     }
 
     for (std::list<std::string>::const_iterator it = dui.includePaths.begin(); it != dui.includePaths.end(); ++it) {
-        const std::string match = findPathInMapBothRelativeAndAbsolute(filedata, simplecpp::simplifyPath(getIncludePathFileName(*it, header)));
+        const std::string match = findPathInMapBothRelativeAndAbsolute(filedata, getIncludePathFileName(*it, header));
         if (!match.empty()) {
             return match;
         }
     }
 
-    if (systemheader && filedata.find(header) != filedata.end())
-        return header;// system header that its file wasn't found in the included paths but alreasy in the filedata - return this as is
-
     return "";
 }