Skip to content

Commit 30ee9cd

Browse files
committed
change fsSetIncludePaths to remove unknown macros and replace the error message with a debug message
1 parent d0e59a1 commit 30ee9cd

6 files changed

Lines changed: 162 additions & 75 deletions

File tree

‎lib/importproject.cpp‎

Lines changed: 38 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1689,7 +1689,8 @@ struct ImportProject::PropertyValueExpander {
16891689

16901690
const std::string name = parseIdentifier();
16911691
if (name.empty() || !hasValue(name)) {
1692-
const std::size_t end = findMatchingParen(mStr, start + 2);
1692+
// mStr[start] is '$' and mStr[start + 1] is the opening '('.
1693+
const std::size_t end = findMatchingParen(mStr, start + 1);
16931694
mPos = (end != std::string::npos) ? end + 1 : mStr.size();
16941695
return mUnknownAsEmpty ? std::string() : mStr.substr(start, mPos - start);
16951696
}
@@ -3292,40 +3293,48 @@ void ImportProject::fsSetIncludePaths(FileSettings &fs, const std::string &basep
32923293
if (startsWith(ipath, "%("))
32933294
continue;
32943295
std::string s(Path::fromNativeSeparators(ipath));
3295-
if (!found.insert(s).second)
3296-
continue;
3297-
if (s[0] == '/' || (s.size() > 1U && s.compare(1, 2, ":/") == 0)) {
3298-
if (!endsWith(s, '/'))
3299-
s += '/';
3300-
fs.includePaths.push_back(std::move(s));
3301-
continue;
3296+
3297+
if (s.find("$(") != std::string::npos) {
3298+
// MSBuild expands an undefined property to the empty string, so
3299+
// Visual Studio sees "$(Undefined)include" as "include" (relative to
3300+
// the project directory) and "$(Undefined)\include" as "\include"
3301+
// (root of the project directory's drive). Do the same here instead of keeping
3302+
// the literal "$(...)" text, which can never name a real directory.
3303+
//
3304+
// It's not possible to determine whether an unknown property is
3305+
// intentional or a deficiency in the importer, so expand once with
3306+
// unknowns preserved purely to report them as debug messages.
3307+
// Debug messages are not currently communicated to the GUI or CLI
3308+
// from importers.
3309+
std::string withUnknowns = s;
3310+
expandMSBuildVariables(withUnknowns, properties);
3311+
checkUnexpandedExpressions(withUnknowns, "include path");
3312+
3313+
PropertyValueExpander expander{*this, properties, s, /*unknownAsEmpty*/ true};
3314+
s = Path::fromNativeSeparators(expander.expand());
3315+
trimWhitespace(s);
3316+
// An entry consisting only of unknown macros is dropped by Visual Studio.
3317+
if (s.empty())
3318+
continue;
33023319
}
33033320

3304-
if (endsWith(s, '/')) // this is a temporary hack, simplifyPath can crash if path ends with '/'
3321+
// simplifyPath can crash if path ends with '/'. Keep the separator of a
3322+
// bare root ("/", "C:/") so its PathKind doesn't change.
3323+
while (s.size() > 1U && endsWith(s, '/') &&
3324+
!(s.size() == 3U && s[1] == ':'))
33053325
s.pop_back();
33063326

3307-
if (s.find("$(") == std::string::npos) {
3308-
s = Path::simplifyPath(basepath + s);
3309-
} else if (!simplifyPathWithVariables(s, properties)) {
3310-
// A macro in this entry didn't resolve (simplifyPathWithVariables()
3311-
// already left `s` with the literal, unexpanded "$(...)" text in it
3312-
// This does NOT silently drop the entry: a directory that can never exist is
3313-
// harmless to keep in includePaths (nothing will ever match it), but
3314-
// dropping it silently left the user with no way to find out why headers
3315-
// that should have been under it went unfound -- addDebug() alone isn't
3316-
// visible by default (see debugs' doc comment in importproject.h), and
3317-
// even --enable=missingInclude only reports the symptom (a header not
3318-
// found) with no link back to this cause. So the literal entry is kept
3319-
// AND a normal, always-visible message is recorded -- errors (unlike
3320-
// debugs) is printed unconditionally by the CLI, matching how the
3321-
// missingFile/missingIncludeExplicit fixes for ClCompile/ForcedIncludeFiles
3322-
// are also always-visible, not gated behind --debug.
3323-
errors.emplace_back("AdditionalIncludeDirectories entry has an unresolved macro, "
3324-
"include path will not be found: '" + s + "'");
3325-
}
3327+
// Resolve the entry the same way as every other path in a project file:
3328+
// absolute and UNC paths are kept, root-relative paths ("\include")
3329+
// take the drive letter of the project directory, and relative paths
3330+
// are relative to the project directory.
3331+
s = toAbsolute(s, basepath, properties);
33263332
if (s.empty())
33273333
continue;
3328-
fs.includePaths.push_back(s.back() == '/' ? s : (s + '/'));
3334+
if (!endsWith(s, '/'))
3335+
s += '/';
3336+
if (found.insert(s).second)
3337+
fs.includePaths.push_back(std::move(s));
33293338
}
33303339
}
33313340

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#include "myheader.h"
22
#ifndef FROM_REAL_HEADER
3-
#error myheader.h (under RealInc, reached only via the unresolved AdditionalIncludeDirectories macro) was not found
3+
#error myheader.h (under RealInc) was not found
44
#endif
55
int main() { return 0; }

‎test/cli/vcxproj_include_dir_macro/vcxproj_include_dir_macro.vcxproj‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,12 @@
2020

2121
<!-- CppcheckTestIncDirMacro is not defined anywhere in this project (no
2222
PropertyGroup, no property sheet) and is not expected to be set as an
23-
environment variable either -- it is deliberately unresolvable, to
24-
exercise fsSetIncludePaths()'s handling of a macro that never resolves.
25-
See vcxproj_include_dir_macro_test.py: this entry must not be silently
26-
dropped from includePaths (RealInc's real header would then simply go
27-
unfound with no indication why), and cppcheck must report it visibly. -->
23+
environment variable either -- it is deliberately unresolvable.
24+
MSBuild expands an undefined property to the empty string, so this
25+
entry is "\RealInc": the root of the project directory's drive, NOT
26+
the RealInc folder next to this project. Visual Studio would not find
27+
myheader.h through it, and neither must cppcheck.
28+
See vcxproj_include_dir_macro_test.py. -->
2829
<ItemDefinitionGroup>
2930
<ClCompile>
3031
<AdditionalIncludeDirectories>$(CppcheckTestIncDirMacro)\RealInc;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<Project DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
3+
<ItemGroup Label="ProjectConfigurations">
4+
<ProjectConfiguration Include="Debug|x64">
5+
<Configuration>Debug</Configuration>
6+
<Platform>x64</Platform>
7+
</ProjectConfiguration>
8+
</ItemGroup>
9+
<PropertyGroup Label="Globals">
10+
<ProjectGuid>{eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee}</ProjectGuid>
11+
<RootNamespace>incdirmacrorelativetest</RootNamespace>
12+
<WindowsTargetPlatformVersion>10.0</WindowsTargetPlatformVersion>
13+
</PropertyGroup>
14+
<Import Project="$(VCTargetsPath)\Microsoft.Cpp.Default.props" />
15+
<PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|x64'" Label="Configuration">
16+
<ConfigurationType>Application</ConfigurationType>
17+
<PlatformToolset>v143</PlatformToolset>
18+
</PropertyGroup>
19+
<Import Project="$(VCTargetsPath)\Microsoft.Cpp.props" />
20+
21+
<!-- Same unresolvable CppcheckTestIncDirMacro as in
22+
vcxproj_include_dir_macro.vcxproj, but with no separator after it.
23+
MSBuild expands it to the empty string, leaving "RealInc", which is
24+
relative to the project directory -- so Visual Studio finds
25+
myheader.h through it, and so must cppcheck.
26+
See vcxproj_include_dir_macro_test.py. -->
27+
<ItemDefinitionGroup>
28+
<ClCompile>
29+
<AdditionalIncludeDirectories>$(CppcheckTestIncDirMacro)RealInc;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
30+
</ClCompile>
31+
</ItemDefinitionGroup>
32+
33+
<ItemGroup>
34+
<ClCompile Include="main.cpp" />
35+
</ItemGroup>
36+
37+
<Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
38+
</Project>
Lines changed: 35 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,59 +1,54 @@
1-
21
# python -m pytest vcxproj_include_dir_macro_test.py
32
#
43
# Regression coverage for an unresolvable macro in AdditionalIncludeDirectories
5-
# (fsSetIncludePaths(), lib/importproject.cpp). Unlike a ClCompile item's own
6-
# Include/Update/Remove path (see vcxproj_item_macro_path_test.py and friends),
7-
# an include-search-path entry isn't restricted to config-invariant macros --
8-
# AdditionalIncludeDirectories is ordinary ItemDefinitionGroup metadata,
9-
# expanded unconditionally like any other (see vcxproj_property_order_test.py).
10-
# The problem here is different: when a macro in one entry genuinely can't be
11-
# resolved at all (not a project property, not an environment variable),
12-
# fsSetIncludePaths() used to silently drop that whole entry from the search
13-
# path list. Any header that lived under it would then simply go unfound, with
14-
# nothing in cppcheck's normal output pointing back at the real cause -- the
15-
# only trace was an addDebug() call, and addDebug()/debugs is not surfaced by
16-
# the CLI under any flag (it's accumulated but never read anywhere outside
17-
# ImportProject itself).
4+
# (fsSetIncludePaths(), lib/importproject.cpp).
5+
#
6+
# MSBuild expands an undefined property to the empty string, and the result is
7+
# handed to cl.exe like any other include directory. cppcheck must do the same
8+
# rather than keep the literal "$(...)" text, which can never name a real
9+
# directory. Whether the expanded entry finds anything depends on what is left:
1810
#
19-
# The fix keeps the entry (as its literal, unexpanded text -- a directory that
20-
# can never exist is harmless to search) and records a normal, always-visible
21-
# message in ImportProject::errors, printed unconditionally by the CLI exactly
22-
# like every other project-import error -- not gated behind --debug like
23-
# addDebug() traces are.
11+
# $(Undefined)\RealInc -> "\RealInc" root of the project directory's drive
12+
# $(Undefined)RealInc -> "RealInc" relative to the project directory
2413
#
25-
# This fixture's only <ClCompile> item is main.cpp, which #includes
26-
# "myheader.h" -- present only under RealInc/, reachable exclusively via
27-
# $(CppcheckTestIncDirMacro)\RealInc, where CppcheckTestIncDirMacro is not
28-
# defined anywhere (no PropertyGroup, and not expected to be a real
29-
# environment variable). main.cpp's #error fires if and only if that header
30-
# was not found, independently confirming the entry was really dropped from
31-
# the search path rather than merely failing to warn.
14+
# Both fixture projects compile main.cpp, which #includes "myheader.h" --
15+
# present only under RealInc/ next to the projects. CppcheckTestIncDirMacro is
16+
# not defined anywhere (no PropertyGroup, and not expected to be a real
17+
# environment variable). main.cpp's #error fires if and only if the header was
18+
# not found, which independently shows how the entry was resolved.
3219

3320
import os
3421

3522
from testutils import cppcheck
3623

3724
__script_dir = os.path.dirname(os.path.abspath(__file__))
3825

26+
__not_found = ('vcxproj_include_dir_macro/main.cpp:3:2: error: #error myheader.h (under RealInc) '
27+
'was not found [preprocessorErrorDirective]')
28+
3929

40-
def test_vcxproj_include_dir_macro():
30+
def __run(project):
4131
args = [
42-
'--project=vcxproj_include_dir_macro/vcxproj_include_dir_macro.vcxproj',
32+
'--project=vcxproj_include_dir_macro/%s' % project,
4333
'--no-cppcheck-build-dir',
4434
]
4535
ret, stdout, stderr = cppcheck(args, cwd=__script_dir)
4636
assert ret == 0, stdout
37+
# The unresolved macro is expanded to an empty string, as Visual Studio
38+
# does -- it is not reported as a project import error.
39+
assert 'cppcheck: error:' not in stdout, stdout
40+
return stderr.replace('\\', '/')
41+
42+
43+
def test_vcxproj_include_dir_macro_root_relative():
44+
# "\RealInc" is the root of the drive, not the RealInc folder next to the
45+
# project, so myheader.h must NOT be found.
46+
stderr = __run('vcxproj_include_dir_macro.vcxproj')
47+
assert __not_found in stderr, stderr
48+
4749

48-
# A normal, always-visible message naming the exact unresolved macro path --
49-
# not silently dropped, and not hidden behind --debug.
50-
assert "cppcheck: error: AdditionalIncludeDirectories entry has an unresolved macro, " \
51-
"include path will not be found: '$(CppcheckTestIncDirMacro)/RealInc'" in stdout, stdout
52-
53-
# myheader.h under RealInc/ must NOT have been found -- the unresolved
54-
# entry stays inert (literal, matching no real directory), it is not
55-
# somehow resolved anyway. main.cpp's own #error is the independent proof.
56-
filename = 'vcxproj_include_dir_macro/main.cpp'
57-
normalized_stderr = stderr.replace('\\', '/')
58-
assert ('%s:3:2: error: #error myheader.h (under RealInc, reached only via the unresolved '
59-
'AdditionalIncludeDirectories macro) was not found [preprocessorErrorDirective]' % filename) in normalized_stderr, stderr
50+
def test_vcxproj_include_dir_macro_relative():
51+
# "RealInc" is relative to the project directory, so myheader.h IS found.
52+
stderr = __run('vcxproj_include_dir_macro_relative.vcxproj')
53+
assert __not_found not in stderr, stderr
54+
assert 'preprocessorErrorDirective' not in stderr, stderr

‎test/testimportproject.cpp‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@ class TestImportProject : public TestFixture {
5757
TEST_CASE(setIncludePaths1);
5858
TEST_CASE(setIncludePaths2);
5959
TEST_CASE(setIncludePaths3); // macro names are case insensitive
60+
TEST_CASE(setIncludePathsUnknownMacro); // unknown macros expand to empty string
61+
TEST_CASE(setIncludePathsRootRelative); // \foo takes the project directory's drive
6062
TEST_CASE(setRelativePathsInclude); // #14746
6163
TEST_CASE(processCompileCommands1);
6264
TEST_CASE(processCompileCommands2); // #8563, #9567
@@ -163,6 +165,48 @@ class TestImportProject : public TestFixture {
163165
ASSERT_EQUALS("c:/abc/other/", fs.includePaths.front());
164166
}
165167

168+
void setIncludePathsUnknownMacro() const {
169+
// MSBuild/Visual Studio expand an undefined property to the empty string.
170+
FileSettings fs{"test.cpp", Standards::Language::CPP, 0};
171+
std::list<std::string> in;
172+
in.emplace_back("$(UnknownMacro)include"); // -> relative to project dir
173+
in.emplace_back("$(SolutionDir)$(UnknownMacro)other"); // -> known part kept
174+
in.emplace_back("$(UnknownMacro)"); // -> dropped
175+
in.emplace_back("$(UnknownMacro)\\abs\\dir"); // -> root-relative
176+
in.emplace_back("$(Unknown1)$(Unknown2)include"); // duplicate after expansion
177+
PropertiesMap properties;
178+
properties["SolutionDir"] = "c:/abc/";
179+
TestImporter importer;
180+
importer.fsSetIncludePaths(fs, "/home/fred/", in, properties);
181+
ASSERT_EQUALS(3U, fs.includePaths.size());
182+
auto it = fs.includePaths.cbegin();
183+
ASSERT_EQUALS("/home/fred/include/", *it++);
184+
ASSERT_EQUALS("c:/abc/other/", *it++);
185+
ASSERT_EQUALS("/abs/dir/", *it);
186+
for (const std::string &p : fs.includePaths)
187+
ASSERT(p.find("$(") == std::string::npos);
188+
}
189+
190+
void setIncludePathsRootRelative() const {
191+
FileSettings fs{"test.cpp", Standards::Language::CPP, 0};
192+
std::list<std::string> in;
193+
in.emplace_back("\\inc"); // root-relative -> project drive
194+
in.emplace_back("$(UnknownMacro)\\abs\\dir"); // root-relative after expansion
195+
in.emplace_back("D:\\other\\..\\lib"); // drive-absolute, simplified
196+
in.emplace_back("\\\\server\\share\\inc"); // UNC kept
197+
in.emplace_back("sub\\"); // relative -> project dir
198+
PropertiesMap properties;
199+
TestImporter importer;
200+
importer.fsSetIncludePaths(fs, "C:/proj/", in, properties);
201+
ASSERT_EQUALS(5U, fs.includePaths.size());
202+
auto it = fs.includePaths.cbegin();
203+
ASSERT_EQUALS("C:/inc/", *it++);
204+
ASSERT_EQUALS("C:/abs/dir/", *it++);
205+
ASSERT_EQUALS("D:/lib/", *it++);
206+
ASSERT_EQUALS("//server/share/inc/", *it++);
207+
ASSERT_EQUALS("C:/proj/sub/", *it);
208+
}
209+
166210
void setRelativePathsInclude() const {
167211
const std::string cwd = Path::fromNativeSeparators(Path::getCurrentPath());
168212
TestImporter importer;

0 commit comments

Comments
 (0)