Page MenuHomePhabricator

[clang][Driver] Implement AddClangSystemIncludeArgs and HasNativeLLVMSupport for the OpenBSD clang driver.
ClosedPublic

Authored by 3405691582 on Aug 22 2020, 7:03 PM.

Details

Summary

If not overridden, AddClangSystemIncludeArgs's implementation is empty, so by default, no system include args are added to the clang driver. This means that invoking clang without the frontend must include a manual -I /usr/include flag, which is inconsistent behavior. Therefore, override and implement this method to match. Some boilerplate is also borrowed for handling of the other driver flags.

While we are here, also override and enable HasNativeLLVMSupport.

Diff Detail

Event Timeline

3405691582 created this revision.Aug 22 2020, 7:03 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 22 2020, 7:03 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
3405691582 requested review of this revision.Aug 22 2020, 7:03 PM

Finally got the formatting fixed properly.

brad added a comment.Aug 22 2020, 10:16 PM

Looks reasonable. I'm a little surprised I don't see this in the FreeBSD driver. What about FreeBSD, NetBSD and DragonFly?

3405691582 updated this revision to Diff 287263.EditedAug 23 2020, 12:56 PM
3405691582 edited the summary of this revision. (Show Details)

Those platforms still use the legacy mechanism in InitHeaderSearch.cpp. For platforms which have moved include handling to the driver, the mechanism in InitHeaderSearch::AddDefaultIncludePaths becomes a noop.

This reminds me of a couple of things: we should also ensure that include management gets delegated in the same way -- it doesn't cause an immediate problem, which is why I missed it here -- so I've updated the diff and summary here to do that. Furthermore, as I understand it, the lack of /usr/include is only applicable when something is invoking clang programatically[1] rather than with the frontend.

[1] In the concrete that motivated this, this is exposed with the Swift programming language and its clang-based C interop. When swiftc tries to build a module, despite absolute path references to platform header files, those headers may include ongoing references with angle brackets, which fails because the invoked preprocessor doesn't have the search paths set up.

brad accepted this revision.Aug 23 2020, 2:25 PM

I was wondering why we had not seen any issues like this but your followup post clarified the situation.

Thank you.

LGTM.

This revision is now accepted and ready to land.Aug 23 2020, 2:25 PM
3405691582 added a comment.EditedAug 23 2020, 4:07 PM

Thanks! please commit on my behalf as appropriate.