fixes, safe asserts, and more tests

Author: Tal500

integration_test.py

@@ -51,10 +51,11 @@ def test_relative_header_1(record_property, tmpdir, with_pragma_once, is_sys):
     else:
         assert double_include_error in stderr
 
+@pytest.mark.parametrize("with_pragma_once", (False, True))
 @pytest.mark.parametrize("inv", (False, True))
 @pytest.mark.parametrize("source_relative", (False, True))
-def test_relative_header_2(record_property, tmpdir, inv, source_relative):
-    header_file, _ = __test_relative_header_create_header(tmpdir)
+def test_relative_header_2(record_property, tmpdir, with_pragma_once, inv, source_relative):
+    header_file, double_include_error = __test_relative_header_create_header(tmpdir, with_pragma_once=with_pragma_once)
 
     test_file = __test_relative_header_create_source(tmpdir, "test.h", header_file, inv=inv)
 
@@ -63,11 +64,14 @@ def test_relative_header_2(record_property, tmpdir, inv, source_relative):
     _, stdout, stderr = simplecpp(args, cwd=tmpdir)
     record_property("stdout", stdout)
     record_property("stderr", stderr)
-    assert stderr == ''
-    if source_relative and not inv:
-        assert '#line 8 "test.h"' in stdout
+    if with_pragma_once:
+        assert stderr == ''
+        if inv:
+            assert f'#line 8 "{pathlib.PurePath(tmpdir).as_posix()}/test.h"' in stdout
+        else:
+            assert '#line 8 "test.h"' in stdout
     else:
-        assert f'#line 8 "{pathlib.PurePath(tmpdir).as_posix()}/test.h"' in stdout
+        assert double_include_error in stderr
 
 @pytest.mark.parametrize("is_sys", (False, True))
 @pytest.mark.parametrize("inv", (False, True))
@@ -89,15 +93,16 @@ def test_relative_header_3(record_property, tmpdir, is_sys, inv, source_relative
         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:
+        if inv:
             assert f'#line 8 "{pathlib.PurePath(test_subdir).as_posix()}/test.h"' in stdout
+        else:
+            assert '#line 8 "test_subdir/test.h"' in stdout
 
 @pytest.mark.parametrize("use_short_path", (False, True))
+@pytest.mark.parametrize("relative_include_dir", (False, True))
 @pytest.mark.parametrize("is_sys", (False, True))
 @pytest.mark.parametrize("inv", (False, True))
-def test_relative_header_4(record_property, tmpdir, use_short_path, is_sys, inv):
+def test_relative_header_4(record_property, tmpdir, use_short_path, relative_include_dir, is_sys, inv):
     test_subdir = os.path.join(tmpdir, "test_subdir")
     os.mkdir(test_subdir)
     header_file, _ = __test_relative_header_create_header(test_subdir)
@@ -106,17 +111,22 @@ def test_relative_header_4(record_property, tmpdir, use_short_path, is_sys, inv)
 
     test_file = __test_relative_header_create_source(tmpdir, header_file, "test.h", is_include2_sys=is_sys, inv=inv)
 
-    args = [format_include_path_arg(test_subdir), test_file]
+    args = [format_include_path_arg("test_subdir" if relative_include_dir else test_subdir), test_file]
 
     _, stdout, stderr = simplecpp(args, cwd=tmpdir)
     record_property("stdout", stdout)
     record_property("stderr", stderr)
     assert stderr == ''
+    if (use_short_path and not inv) or (relative_include_dir and 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("with_pragma_once", (False, True))
+@pytest.mark.parametrize("relative_include_dir", (False, True))
 @pytest.mark.parametrize("is_sys", (False, True))
 @pytest.mark.parametrize("inv", (False, True))
-def test_relative_header_5(record_property, tmpdir, with_pragma_once, is_sys, inv): # test relative paths with ..
+def test_relative_header_5(record_property, tmpdir, with_pragma_once, relative_include_dir, is_sys, inv): # test relative paths with ..
     ## in this test, the subdir role is the opposite then the previous - it contains the test.c file, while the parent tmpdir contains the header file
     header_file, double_include_error = __test_relative_header_create_header(tmpdir, with_pragma_once=with_pragma_once)
     if is_sys:
@@ -128,16 +138,42 @@ def test_relative_header_5(record_property, tmpdir, with_pragma_once, is_sys, in
     os.mkdir(test_subdir)
     test_file = __test_relative_header_create_source(test_subdir, header_file, header_file_second_path, is_include2_sys=is_sys, inv=inv)
 
-    args = ([format_include_path_arg(tmpdir)] if is_sys else []) + ["test.c"]
+    args = ([format_include_path_arg(".." if relative_include_dir else tmpdir)] if is_sys else []) + ["test.c"]
 
     _, stdout, stderr = simplecpp(args, cwd=test_subdir)
     record_property("stdout", stdout)
     record_property("stderr", stderr)
     if with_pragma_once:
         assert stderr == ''
-        if inv:
+        if (relative_include_dir or not is_sys) and inv:
             assert '#line 8 "../test.h"' in stdout
         else:
             assert f'#line 8 "{pathlib.PurePath(tmpdir).as_posix()}/test.h"' in stdout
     else:
         assert double_include_error in stderr
+
+@pytest.mark.parametrize("with_pragma_once", (False, True))
+@pytest.mark.parametrize("relative_include_dir", (False, True))
+@pytest.mark.parametrize("is_sys", (False, True))
+@pytest.mark.parametrize("inv", (False, True))
+def test_relative_header_6(record_property, tmpdir, with_pragma_once, relative_include_dir, is_sys, inv): # test relative paths with .. that is resolved only by an include dir
+    ## in this test, both the header and the source file are at the same dir, but there is a dummy inclusion dir as a subdir
+    header_file, double_include_error = __test_relative_header_create_header(tmpdir, with_pragma_once=with_pragma_once)
+
+    test_subdir = os.path.join(tmpdir, "test_subdir")
+    os.mkdir(test_subdir)
+    test_file = __test_relative_header_create_source(tmpdir, header_file, "../test.h", is_include2_sys=is_sys, inv=inv)
+
+    args = [format_include_path_arg("test_subdir" if relative_include_dir else test_subdir), "test.c"]
+
+    _, stdout, stderr = simplecpp(args, cwd=tmpdir)
+    record_property("stdout", stdout)
+    record_property("stderr", stderr)
+    if with_pragma_once:
+        assert stderr == ''
+        if relative_include_dir and inv:
+            assert '#line 8 "test.h"' in stdout
+        else:
+            assert f'#line 8 "{pathlib.PurePath(tmpdir).as_posix()}/test.h"' in stdout
+    else:
+        assert double_include_error in stderr

simplecpp.cpp

@@ -2719,9 +2719,7 @@ static std::string toAbsolutePath(const std::string& path) {
     return simplecpp::simplifyPath(path);
 }
 
-// templated function for compiler optimization opportunities
-template <bool withTrailingSlash>
-static std::string dirPath(const std::string& path) {
+static std::string dirPath(const std::string& path, bool withTrailingSlash=true) {
     const std::size_t firstSlash = path.find_last_of("\\/");
     if (firstSlash == std::string::npos) {
         return "";
@@ -2742,7 +2740,7 @@ static std::string extractRelativePathFromAbsolute(const std::string& absoluteSi
     std::string leadingParenting;
     while (!startsWith_(normalizedAbsolutePath, currentPrefix)) {
         leadingParenting = "../" + leadingParenting;
-        currentPrefix = dirPath<false>(currentPrefix);
+        currentPrefix = dirPath(currentPrefix, false);
     }
     const std::size_t size = currentPrefix.size();
     std::string relativeFromMeetingPath = normalizedAbsolutePath.substr(size, normalizedAbsolutePath.size() - size);
@@ -3169,26 +3167,24 @@ static std::string openHeader(std::ifstream &f, const std::string &path)
     return "";
 }
 
-// templated function for compiler optimization opportunities
-template <bool expandBaseWithAbsolutePath, bool exactRelative> 
 static std::string getRelativeFileName(const std::string &baseFile, const std::string &header)
 {
     const std::string baseFileSimplified = simplecpp::simplifyPath(baseFile);
-    const std::string baseFileNormalized = (expandBaseWithAbsolutePath && !isAbsolutePath(baseFileSimplified)) ? (currentDirectory() + "/" + baseFileSimplified) : baseFileSimplified;
+    const std::string baseFileAbsolute = isAbsolutePath(baseFileSimplified) ?
+        baseFileSimplified :
+        simplecpp::simplifyPath(currentDirectory() + "/" + baseFileSimplified);
 
     const std::string headerSimplified = simplecpp::simplifyPath(header);
-    const std::string path = isAbsolutePath(headerSimplified) ? headerSimplified : (dirPath<true>(baseFileNormalized) + headerSimplified);
-    if (exactRelative) {
-        const std::string absolutePath = expandBaseWithAbsolutePath ? path : toAbsolutePath(path);
-        const std::string absoluteBaseFile = expandBaseWithAbsolutePath ? baseFileNormalized : toAbsolutePath(baseFileNormalized);
-        return extractRelativePathFromAbsolute(absolutePath, dirPath<true>(absoluteBaseFile));
-    }
-    return path;
+    const std::string path = isAbsolutePath(headerSimplified) ?
+        headerSimplified :
+        simplecpp::simplifyPath(dirPath(baseFileAbsolute) + headerSimplified);
+
+    return extractRelativePathFromAbsolute(path);
 }
 
 static std::string openHeaderRelative(std::ifstream &f, const std::string &sourcefile, const std::string &header)
 {
-    return openHeader(f, getRelativeFileName<false, false>(sourcefile, header));
+    return openHeader(f, getRelativeFileName(sourcefile, header));
 }
 
 // returns the simplified header path:
@@ -3205,8 +3201,9 @@ static std::string getIncludePathFileName(const std::string &includePath, const
     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);
+    const std::string absoluteSimplifiedHeaderPath = simplecpp::simplifyPath(basePath + simplifiedHeader);
+    // preserve absoluteness/relativieness of the including dir
+    return isAbsolutePath(includePath) ? absoluteSimplifiedHeaderPath : extractRelativePathFromAbsolute(absoluteSimplifiedHeaderPath);
 }
 
 static std::string openHeaderIncludePath(std::ifstream &f, const simplecpp::DUI &dui, const std::string &header)
@@ -3270,7 +3267,7 @@ static std::string getFileIdPath(const std::map<std::string, simplecpp::TokenLis
     }
 
     if (!systemheader) {
-        const std::string relativeOrAbsoluteFilename = getRelativeFileName<true, true>(sourcefile, header);// unknown if absolute or relative, but always simplified
+        const std::string relativeOrAbsoluteFilename = getRelativeFileName(sourcefile, header);// unknown if absolute or relative, but always simplified
         const std::string match = findPathInMapBothRelativeAndAbsolute(filedata, relativeOrAbsoluteFilename);
         if (!match.empty()) {
             return match;
@@ -3294,6 +3291,16 @@ static bool hasFile(const std::map<std::string, simplecpp::TokenList *> &filedat
     return !getFileIdPath(filedata, sourcefile, header, dui, systemheader).empty();
 }
 
+static void safeInsertTokenListToMap(std::map<std::string, simplecpp::TokenList *> &filedata, const std::string &header2, simplecpp::TokenList *tokens, const std::string &header, const std::string &sourcefile, bool systemheader, const char* contextDesc)
+{
+    const bool inserted = filedata.insert(std::make_pair(header2, tokens)).second;
+    if (!inserted) {
+        std::cerr << "error in " << contextDesc << " - attempt to add a tokenized file to the file map, but this file is already in the map! Details:" <<
+            "header: " << header << " header2: " << header2 << " source: " << sourcefile << " systemheader: " << systemheader << std::endl;
+        abort();
+    }
+}
+
 std::map<std::string, simplecpp::TokenList*> simplecpp::load(const simplecpp::TokenList &rawtokens, std::vector<std::string> &filenames, const simplecpp::DUI &dui, simplecpp::OutputList *outputList)
 {
 #ifdef SIMPLECPP_WINDOWS
@@ -3370,7 +3377,7 @@ std::map<std::string, simplecpp::TokenList*> simplecpp::load(const simplecpp::To
         TokenList *tokens = new TokenList(header2, filenames, outputList);
         if (dui.removeComments)
             tokens->removeComments();
-        ret[header2] = tokens;
+        safeInsertTokenListToMap(ret, header2, tokens, header, rawtok->location.file(), systemheader, "simplecpp::load");
         if (tokens->front())
             filelist.push_back(tokens->front());
     }
@@ -3657,7 +3664,7 @@ void simplecpp::preprocess(simplecpp::TokenList &output, const simplecpp::TokenL
                         TokenList * const tokens = new TokenList(header2, files, outputList);
                         if (dui.removeComments)
                             tokens->removeComments();
-                        filedata[header2] = tokens;
+                        safeInsertTokenListToMap(filedata, header2, tokens, header, rawtok->location.file(), systemheader, "simplecpp::preprocess");
                     }
                 }
                 if (header2.empty()) {