Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Header inclusion checking in src/hdrs for cc_library is broken #17055

Closed
silvergasp opened this issue Dec 20, 2022 · 3 comments
Closed

Header inclusion checking in src/hdrs for cc_library is broken #17055

silvergasp opened this issue Dec 20, 2022 · 3 comments
Labels

Comments

@silvergasp
Copy link
Contributor

Description of the bug:

Header inclusion checking seems to be broken. In the documentation for cc_library, it mentions that adding header files to the srcs attribute should make them private to dependent targets. However, this is not the case.

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Create a skeleton repository based on the example in the Bazel documentation;

cc_binary(
    name = "foo",
    srcs = [
        "foo.cc",
        "foo.h",
    ],
    deps = [":bar"],
)

cc_library(
    name = "bar",
    srcs = [
        "bar.cc",
        "bar-impl.h",
    ],
    hdrs = ["bar.h"],
    deps = [":baz"],
)

cc_library(
    name = "baz",
    srcs = [
        "baz.cc",
        "baz-impl.h",
    ],
    hdrs = ["baz.h"],
)

Try and include bar-impl.h from foo.cc. According to the documentation, this should produce an error, however, it does not and instead builds successfully.

Including file Allowed inclusions
foo.h bar.h
foo.cc foo.h bar.h
bar.h bar-impl.h baz.h
bar-impl.h bar.h baz.h
bar.cc bar.h bar-impl.h baz.h
baz.h baz-impl.h
baz-impl.h baz.h
baz.cc baz.h baz-impl.h

Which operating system are you running Bazel on?

Ubuntu 20.04 (WSL)

What is the output of bazel info release?

5.4.0

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

#14346
#7030
#3828

Any other information, logs, or outputs that you want to share?

No response

@fmeum
Copy link
Collaborator

fmeum commented Dec 20, 2022

If you include bar-impl.h from foo.cc, use clang and enable the layering_check feature, you get this error:

bazel build //:foo --repo_env=CC=clang --features=layering_check
foo.cc:3:10: error: use of private header from outside its module: 'bar-impl.h' [-Wprivate-header]
#include "bar-impl.h"
         ^
1 error generated.

This works even better then the docs mention. While they say "For example, Bazel would not complain if in the example above foo.cc directly includes baz.h.", if you try that, you get this error:

bazel build //:foo --repo_env=CC=clang --features=layering_check
foo.cc:3:10: error: module //:foo does not depend on a module exporting 'baz.h'
#include "baz.h"
         ^
1 error generated.

@silvergasp
Copy link
Contributor Author

silvergasp commented Dec 20, 2022

Hmm, so in the example, you gave above, it's pretty clear that this feature is working (with the layering check feature enabled). Perhaps, it's just a documentation error and the docs should just mention the layering_check feature?

@fmeum
Copy link
Collaborator

fmeum commented Dec 20, 2022

I submitted #17057 to improve the documentation of this feature.

hvadehra pushed a commit that referenced this issue Feb 14, 2023
The `layering_check` feature and toolchain support are required to realize header dependency checking in Bazel. If enabled, this also reports errors for explicitly included transitive, non-direct headers.

Fixes #17055
Fixes #15632

Closes #17057.

PiperOrigin-RevId: 499189567
Change-Id: Ia7978193e4572a358e26d24183d125649f8654b9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants