[clang] "Transitive" deprecation diagnistic not emitted when coming from `-isystem`

[LebedevRI]

This has been brought up by halide folk, it came up in https://github.com/halide/Halide/pull/6555. Consider: https://godbolt.org/z/n8EaEsv4s

# 1 "include/header.hpp" 1 3
// include/header.hpp

template <typename T>
struct Target {
  Target(T d) {}
};

template <>
struct Target<int> {
  [[deprecated("...")]]
  Target(int d) {}
};

template <typename T>
struct Indirect : public Target<T> {
  Indirect(T d) : Target<T>(d) {}
};
# 70 "include/header.hpp" 2

struct Example {
  Indirect<int> x{1};
  Indirect<float> y{1.0};
};

This does not emit any diags: https://godbolt.org/z/n8EaEsv4s But if you use -I, you get: https://godbolt.org/z/bvs488n7v

<source>:17:19: warning: 'Target' is deprecated: ... [-Wdeprecated-declarations]
  Indirect(T d) : Target<T>(d) {}
                  ^
<source>:22:18: note: in instantiation of member function 'Indirect<int>::Indirect' requested here
  Indirect<int> x{1};
                 ^
<source>:11:5: note: 'Target' has been explicitly marked deprecated here
  [[deprecated("...")]]
    ^
1 warning generated.
Compiler returned: 0

This particular case seems rather suboptimal.

While we could probably solve this easily with -Walways-emit-deprecation-warnings or something, i'm wondering if this is a bug in the first place, or we can come up with some subset of cases that we could diagnose?

What do people think? CC @AaronBallman @zygoloid @dwblaikie @alexreinking @steven-johnson

[llvmbot]

@llvm/issue-subscribers-clang-frontend

[LebedevRI]

Does anyone have any thoughts on the matter? :) I would have guessed that deprecation diagnostics should always be emitted, system header or not.

[dwblaikie]

Yeah, I don't think you should get a warning in this case - because you didn't write the caller, even if the caller is dependent on the template parameter you provided. It's certainly not clear-cut though.

If the header had code like

void f1() // deprecated
void f2() {
  f1();
}

And you call f2, I think it's reasonable not to get a deprecation warning - that's a header implementation detail that shouldn't leak into the user. If the system header provider wanted f2 to also be deprecated, they could flag it as such.

For dependent cases like the example above - I'm not sure we'll be able to differentiate the cases that boil down to something morally equivalent to the f1/f2 case, and cases that are more like, say, std::sort calling a deprecated op< function, perhaps.

[AaronBallman]

I agree with @dwblaikie -- I don't think it's clear cut on how to distinguish between situations to warn on and situations to silence on. His example was exactly the one I was worried about -- it definitely comes up in system headers.

Your idea about a new warning flag that you opt into is interesting, but we don't typically add new off-by-default warning flags because we have evidence that users almost never enable them (at least, not enough to make it worth the effort to maintain). So I'd prefer if we can try to find some heuristic by which to decide when to warn if at all possible, that way we can leave the functionality enabled by default.

[Endilll]

Appears to be fixed in Clang 16: https://godbolt.org/z/dEKsjhecr

[dwblaikie]

@Endilll not sure I follow - this bug is claiming the diagnostic /should/ be emitted - but in Clang 16 it is not emitted, right?

[Endilll]

You're right. What my previous CE link showcases seems to be a regression in 16. I suspect https://reviews.llvm.org/D147302, which apparently broke -Wdeprecated-declarations and then it was fixed in c107231cde99c0b8cdda5c5befe6354750ca03f2, but I'm not sure that fix has fully recovered the diagnostic.

[llvmbot]

@llvm/issue-subscribers-c-1

Author: Roman Lebedev (LebedevRI)

This has been brought up by [halide](https://github.com/halide/Halide) folk, it came up in https://github.com/halide/Halide/pull/6555. Consider: https://godbolt.org/z/n8EaEsv4s ```c++ # 1 "include/header.hpp" 1 3 // include/header.hpp template <typename T> struct Target { Target(T d) {} }; template <> struct Target<int> { [[deprecated("...")]] Target(int d) {} }; template <typename T> struct Indirect : public Target<T> { Indirect(T d) : Target<T>(d) {} }; # 70 "include/header.hpp" 2 struct Example { Indirect<int> x{1}; Indirect<float> y{1.0}; }; ``` This does not emit any diags: https://godbolt.org/z/n8EaEsv4s But if you use `-I`, you get: https://godbolt.org/z/bvs488n7v ``` <source>:17:19: warning: 'Target' is deprecated: ... [-Wdeprecated-declarations] Indirect(T d) : Target<T>(d) {} ^ <source>:22:18: note: in instantiation of member function 'Indirect<int>::Indirect' requested here Indirect<int> x{1}; ^ <source>:11:5: note: 'Target' has been explicitly marked deprecated here [[deprecated("...")]] ^ 1 warning generated. Compiler returned: 0 ``` This particular case seems rather suboptimal. While we could probably solve this easily with `-Walways-emit-deprecation-warnings` or something, i'm wondering if this is a bug in the first place, or we can come up with some subset of cases that we could diagnose? What do people think? CC @AaronBallman @zygoloid @dwblaikie @alexreinking @steven-johnson