This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Diagnose when header search paths are set up incorrectly
ClosedPublic

Authored by ldionne on Aug 8 2022, 2:16 PM.

Details

Summary

An issue I often see in codebases compiled for unusual platforms is
that header search paths are specified manually and are subtly wrong.
For example, people will manually add -isystem <some-toolchain>/usr/include,
which ends up messing up the layering of header search paths required by
libc++ (because the C Standard Library now appears *before* libc++ in
the search paths). Without this patch, this will end up causing
compilation errors that are pretty inscrutable. This patch aims to
improve the user experience by diagnosing this issue explicitly.

In all cases I can think of, I would expect that a compilation error
occur if these header search paths are not layered properly. This
should only provide an explicit diagnostic instead of failing due
to seemingly unrelated compilation errors.

Diff Detail

Event Timeline

ldionne created this revision.Aug 8 2022, 2:16 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 8 2022, 2:16 PM
ldionne requested review of this revision.Aug 8 2022, 2:16 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 8 2022, 2:16 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
ldionne added a subscriber: Restricted Project.Aug 10 2022, 12:09 PM

Pinging vendors. I don't foresee this being an issue, however it is possible that it will cause some breakage. I wouldn't expect that, though -- if someone is broken by this, they would almost certainly already be broken anyway because libc++ simply doesn't work correctly if the right layering of includes isn't respected.

Also, note to self: 98464615

ldionne accepted this revision.Aug 16 2022, 2:34 PM

I actually built a significant codebase with this change, and the only few issues that popped up were places where include paths were indeed set up incorrectly. In most cases, I had also spent significant time in the past figuring out incorrect header path ordering in those parts of the code base, which would have been greatly helped by this diagnostic.

So yeah, I guess this gives me the confidence I needed that this change is good.

This revision is now accepted and ready to land.Aug 16 2022, 2:34 PM
ldionne updated this revision to Diff 453138.Aug 16 2022, 2:56 PM

Remove quotes from #error