[clang] [Headers] Don't declare unreachable() from stddef.h in C++ (PR #86748)

Ian Anderson via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 2 09:31:54 PDT 2024


ian-twilightcoder wrote:

> > > FWIW, I did verify that it's very unlikely the changes in this PR will break existing code: https://sourcegraph.com/search?q=context:global+__need_unreachable+-file:.*clang.*&patternType=keyword&sm=0, so that's a good thing.
> > > > I do wonder if we could have the broader builtin headers discussion independent of this patch? Is everyone happy with this patch? We can keep talking about the builtin headers in here independent of merging right?
> > > 
> > > 
> > > I guess I don't see these as independent topics; if we decide that C++ mode should not have observable differences in C headers, then the changes here are incorrect. I think we should have this discussion in a broader context (like Discourse) before moving forward with these changes.
> > > Also, I'd still like an explanation for [this question](https://github.com/llvm/llvm-project/pull/86748#issuecomment-2023145043):
> > > > I don't understand why this is making into C++ builds at all:
> > > 
> > > 
> > > because it may turn out we don't need these changes in the first place because the issue is elsewhere.
> > 
> > 
> > Right now I just noticed that in a C++ test I was writing that stddef.h alone doesn't give me unreachable, but __needs_unreachable does. And that's probably wrong because unreachable belongs to  in C++ and _not_ stddef.h. The C++ standard has some frustrating divergence with the C standard as to what the c stdlib headers declare...
> 
> I don't yet agree that it's wrong -- you define the macro saying you want `unreachable` from `stddef.h`, so you get `unreachable` from `stddef.h`. Morally, it's very similar to:
> 
> ```
> #define unreachable "I am doing something which causes myself pain"
> #include <utility>
> ```

I was thinking that a lot of the low level Apple SDK C headers use the `__need_` macros to add things they use, and that shouldn't mess up C++ clients. Although I guess if they actually use `unreachable` then they need to have a `__cplusplus` in there to include \<utility> instead of just not getting `unreachable` at all. So, I guess there's a problem either way. However, I still don't really like the idea that someone can make a mistake in the SDK and break the C++ declaration of `unreachable`. I also don't like that `unreachable` is currently inconsistent with `nullptr_t`, which we almost never declare in C++ mode either, for similar reasons.

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


More information about the cfe-commits mailing list