[clang] [clang][modules] Headers meant to be included multiple times can be completely invisible in clang module builds (PR #83660)

James Y Knight via cfe-commits cfe-commits at lists.llvm.org
Fri Apr 12 14:54:18 PDT 2024


jyknight wrote:

> The problem I'm trying to fix is that nobody knows when it's appropriate to use #import vs #include

But you haven't really (and I think cannot) fixed that.

> using header guards or #pragma once is very "un-Objective-C". 

Yes, this is quite unfortunate. The best answer would be to change the recommended style, but unfortunately, it's probably 20-30 years too late to do so effectively. (I mean, it would be a start if Apple added `#pragma once` to all of the SDK headers.)

Your initial comment said:
> Normally this isn't a big problem, [...] However, when clang modules are involved, this can cause the header to be hidden entirely.

And I am basically agreeing with this. The current state of objc include/import, while unfortunate, has existed for decades, and is usually not a big problem. And, I think that means we shouldn't add more complexity to the semantics of `#import` in an attempt to fix it. But the "header is hidden entirely" _is_ a big problem, and that part is due to the bug I described -- so if that bug is fixed, we're back to "isn't a big problem", without making this change in semantics.

> An alternative solution would be @jansvoboda11's https://github.com/llvm/llvm-project/pull/71117, but I believe that will cause issues with non-modular headers building into multiple modules.

I haven't looked at that, but it sounds promising. The end result should be that `#import`ed and `#pragma once`-guarded files are treated the same way as `#ifndef`-guarded files. I wouldn't expect that to cause issues with non-modular headers included in multiple modules, given that works for `#ifndef`-guarded files, today.

> Also this change isn't trying to modify #include in any kind of way, only #import. So it's quite intentional that your test behaves the same before and after.

I know you weren't trying to fix that -- but I'm saying that's the thing that _should_ be fixed, and that doing so would fix the _serious_ part of the problem you described in the initial message.

https://github.com/llvm/llvm-project/pull/83660


More information about the cfe-commits mailing list