AaronBallman wrote:
> > Overall, I like the idea, but I wonder if a pragma is really the best way
> > forward in 2025. I can imagine standards committees wanting this
> > functionality, and a pragma would likely be just fine in C, but not
> > acceptable in C++.
> > Did you consider using an attribute instead? e.g.,
> > `[[clang::deprecated("this header is deprecated")]];` on a null statement
> > in a header file could be a viable approach that both committees would be
> > interested in standardizing under the existing `[[deprecated]]` attribute.
>
> I didn't consider this before, but I've done that now and I think a `#pragma`
> is the right tool for the job. IMO deprecating a header is fundamentally part
> of the preprocessor, so it should be written in a way that looks like it is
> in fact part of preprocessing. A pragma seems like the natural tool to use in
> this case. I guess it'd be possible to write it as `#deprecated`, but I don't
> see a good reason it should be written like that.
That's compelling rationale, thanks! I agree that the preprocessor *alone*
should be able to handle emitting the diagnostic and that would mean an
attribute would be the wrong tool for the job because that requires the
compiler to get involved.
> > Some things the PR needs to consider:
> > ```
> > * This needs documentation in the language extensions file. In particular,
> > where within a header does the marking show up? Can it be anywhere? Just
> > the "first semantic line" (non-comment, non-header guard)?
> > ```
>
> I don't think there is a reason to restrict it to a particular position in
> the header. Whether it's a good idea for readability is questionable, but in
> general I think there are good reasons to not put it right at the top. e.g.
> in libc++ we'd put it after at least a `#pragma GCC system_header` and an
> include, since these are always the first things in a header right after the
> header guard. I don't see why this would need to change for this pragma.
The situation I'm worried about is where we don't see the deprecation pragma
and the user includes the header file transitively. e.g.,
```
// Foobar.h
#ifndef FOOBAR_H
#define FOOBAR_H
#include "Baz.h" // Assume this happens through lots of nested includes
#endif
// Baz.h
#ifndef BAZ_H
#define BAZ_H
#include "Foobar.h"
#pragma clang deprecate_header("don't use Baz.h")
#endif
// Main.c
#include "Baz.h"
```
We'd want a warning in Main.c because of the include, but how would we diagnose
the include from `Foobar.h`?
Is the basic idea for the implementation to wait until the end of the TU and
see if the header is included in the TU and if so, diagnose? I was assuming the
implementation would be "while processing a `#include`, see if the header is
deprecated and if so, diagnose." but that wouldn't work here, right?
>
> > ```
> > * What should happen if the deprecated header ends up including another
> > deprecated header? Normally, deprecated things can reference other
> > deprecated things on the assumption they're all being deprecated together.
> > So perhaps the same is true here? Is it true if deprecated header A
> > includes non-deprecated header B which then includes deprecated header C
> > though?
> > ```
>
> This is a good question and something I definitely didn't consider before.
> I'm not sure what the answer is TBH. For the libc++ use-case it's definitely
> not necessary, and I'm struggling a bit to come up with a scenario where
> you'd have multiple levels of deprecated headers. I think you'd usually
> deprecate a header because you either have a new header that should be used,
> or your header doesn't provide anything meaningful anymore and should just
> not be included anymore. Either way I don't know why you'd include another
> deprecated header. You're _not_ deprecating the contents of the header after
> all.
Contriving a situation: You have a library to handle cryptography; there are
headers for cryptographic algorithms and there are headers for implementation
details. Say you want to deprecate one particular algorithm -- you might
deprecate the public interface (`InsecureHash.h`) as well as the private
interface (`InsecureHashImpl.h`). Does the public implementation file
(`InsecureHash.c`) which includes the private interface (`InsecureHashImpl.h`)
get a diagnostic? The user probably doesn't want one. But if the user includes
`InsecureHashImpl.h` from `SecureHash.c`, they probably do want to know they're
using the deprecated header.
>
> > ```
> > * We usually suppress diagnostics coming from system headers but this one
> > kind of spans the idea. The system header specifies the diagnostic should
> > happen but the diagnostic itself happens on the inclusion line which may be
> > user code. We need test coverage to make sure user code does get the
> > diagnostic and system headers do not.
> >
> > * What happens if the marking is used in a module that is imported? I would
> > imagine we'd want the same diagnostic behavior? Perhaps another reason the
> > name should be under `deprecated` rather than `deprecated_header`?
> > ```
>
> Did I miss the part where you suggested to use `deprecated` instead of
> `deprecated_header`?
This was when I was thinking about `[[deprecated]];` as the way to surface it.
> What module are you talking about? A Clang module or a C++ module? Or are you
> taking about C++ header units? FWIW in my header C++ header units and Clang
> modules are basically just headers with fancy names, so I think
> `deprecated_header` still applies and should behave the same. With C++
> modules IMO the right place to add a deprecation note would be on the module
> declaration itself, and the pragma should be rejected there.
I was thinking about a C++ module which has a global module fragment that
includes a deprecated header file.
https://github.com/llvm/llvm-project/pull/168041
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
