Skip to content

Commit

Permalink
Support relative include statements for virtual paths
Browse files Browse the repository at this point in the history
Recent improvements allows us to finally resolve this limitation of DWYU.
This is is a niche use case, and doing relative includes to virtual paths
is something I would consider a bad practice. Still, given it compiles DWYU
should be able to analyze it.
  • Loading branch information
martis42 committed Sep 10, 2023
1 parent 28075e4 commit a06deb2
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 17 deletions.
3 changes: 1 addition & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -169,12 +169,11 @@ Examples for this can be seen at the [implementation_deps test cases](test/aspec

## Known limitations

- If includes are added through a macro, this is invisible to DWYU.
- Includes which are added through a preprocessor token are not recognized.
- Fundamental support for processing preprocessor defines is present.
However, if header _A_ specifies a define _X_ and is included in header _B_, header _B_ is not aware of _X_ from header _A_.
Right now only defines specified through Bazel (e.g. toolchain or `cc_*` target attributes) or defines specified inside a file itself are used to process a file and discover include statements.
We aim to resolve this limitation in a future release.
- Include statements utilizing `..` are not recognized if they are used on virtual or system include paths.

## Applying automatic fixes

Expand Down
22 changes: 16 additions & 6 deletions src/analyze_includes/evaluate_includes.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,19 +125,29 @@ def _check_for_invalid_includes(
)
if not legal_include:
# Might be a relative include
curr_dir = inc.file.parent
path_matching_include_statement = (curr_dir / inc.include).resolve()
legal_include = _include_resolves_to_any_file(
included_path=path_matching_include_statement, files=target_under_inspection.header_files
)
if not legal_include:
roots_for_relative_includes = [Path(root) for root in [inc.file.parent] + include_paths]

for root in roots_for_relative_includes:
path_matching_include_statement = (root / inc.include).resolve()

# Relative include to target under inspection
if _include_resolves_to_any_file(
included_path=path_matching_include_statement, files=target_under_inspection.header_files
):
legal_include = True
break

# Relative include to dependency
for dep in dependencies:
if _include_resolves_to_any_file(
included_path=path_matching_include_statement, files=dep.header_files
):
legal_include = True
dep.usage.update(usage)
break
if legal_include:
break

if not legal_include:
invalid_includes.append(inc)
return invalid_includes
Expand Down
27 changes: 21 additions & 6 deletions src/analyze_includes/test/evaluate_includes_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -355,14 +355,10 @@ def test_success_for_valid_dependencies_with_virtual_include_paths(self):
Include(file=Path("file1"), include="foo.h"),
Include(file=Path("file2"), include="dir/bar.h"),
],
private_includes=[
Include(file=Path("file4"), include="path/baz.h"),
],
private_includes=[Include(file=Path("file4"), include="path/baz.h")],
system_under_inspection=SystemUnderInspection(
target_under_inspection=CcTarget(name="foo", header_files=["self/own_header.h"]),
deps=[
CcTarget(name="foo_pkg", header_files=["foo.h", "some/virtual/dir/bar.h"]),
],
deps=[CcTarget(name="foo_pkg", header_files=["foo.h", "some/virtual/dir/bar.h"])],
implementation_deps=[CcTarget(name="baz_pkg", header_files=["long/nested/path/baz.h"])],
include_paths=["", "long/nested", "some/virtual"],
defines=[],
Expand All @@ -372,6 +368,25 @@ def test_success_for_valid_dependencies_with_virtual_include_paths(self):

self.assertTrue(result.is_ok())

def test_success_for_valid_dependencies_with_virtual_include_paths_and_relative_include_statements(self):
result = evaluate_includes(
public_includes=[
Include(file=Path("file1"), include="../../self/own_header.h"),
Include(file=Path("file1"), include="some/virtual/../../dir/foo.h"),
],
private_includes=[Include(file=Path("file2"), include="some/../../../some/virtual/dir/bar.h")],
system_under_inspection=SystemUnderInspection(
target_under_inspection=CcTarget(name="self", header_files=["self/own_header.h"]),
deps=[CcTarget(name="foo_pkg", header_files=["dir/foo.h"])],
implementation_deps=[CcTarget(name="bar_pkg", header_files=["some/virtual/dir/bar.h"])],
include_paths=["", "some/virtual", "other/virtual"],
defines=[],
),
ensure_private_deps=True,
)

self.assertTrue(result.is_ok())

def test_success_for_internal_relative_includes_with_flat_structure(self):
result = evaluate_includes(
public_includes=[Include(file=Path("foo.h"), include="bar.h")],
Expand Down
3 changes: 2 additions & 1 deletion test/aspect/relative_includes/use_system_include.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
#include "some/sub/dir/foo.h"
#include "some/sub/dir/../dir/bar.h"
// include from virtually prefixed path
#include <sub/dir/bar.h>
#include <sub/../sub/dir/bar.h>
#include <../some/sub/dir/bar.h>

int main() {
return useSystemInclude() + doBar() + doFoo();
Expand Down
5 changes: 4 additions & 1 deletion test/aspect/relative_includes/use_virtual_prefix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@
#include "virtual_prefix.h"
#include "some/sub/dir/bar.h"
#include "some/sub/dir/../dir/bar.h"
// reach into virtual paths from repository root
#include "test/aspect/relative_includes/_virtual_includes/virtual_prefix/custom/prefix/some/sub/dir/foo.h"
// include from virtually prefixed path
#include "custom/prefix/some/sub/dir/foo.h"
#include "../virtual_prefix/custom/prefix/some/sub/dir/foo.h"
#include "custom/prefix/../prefix/some/sub/dir/foo.h"

int main() {
return useVirtualPrefix() + doBar() + doFoo();
Expand Down
5 changes: 4 additions & 1 deletion test/aspect/relative_includes/use_virtual_strip.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@
#include "some/virtual_strip.h"
#include "some/sub/dir/bar.h"
#include "some/sub/dir/../dir/bar.h"
// reach into virtual paths from repository root
#include "test/aspect/relative_includes/_virtual_includes/virtual_strip/sub/dir/foo.h"
// include from virtually stripped path
#include "sub/dir/foo.h"
#include "../virtual_strip/sub/dir/foo.h"
#include "sub/dir/../dir/foo.h"

int main() {
return useVirtualStrip() + doBar() + doFoo();
Expand Down

0 comments on commit a06deb2

Please sign in to comment.