[clang] Fix missing diagnostic of declaration use when accessing TypeDecls through typename access

[mizvekov]

Summary

Fixes GH58547.

Signed-off-by: Matheus Izvekov <[e-mail]>

[ychen]

Looks straightforward to me with one suggestion.

Is the CI fail related?

clang/include/clang/Sema/Sema.h

2569

I find it hard to understand this enum since it does not correlate with wordings well and need to read code in other places to understand what it is used for. How about using two parameters: bool DiagCtor (replace AK != TypeAccessKind::Explicit) and bool IsImplicitTypename (replace AK == TypeAccessKind::Typename) ?

[mizvekov]

In D136533#3877854, @ychen wrote:

Looks straightforward to me with one suggestion.

Is the CI fail related?

Yeah, the CI fail is because, while there is a change in libcxx diagnostics and we fix it, the same tests are run in different pipelines using stock, released clangs.

So we need a way to handle the difference in expectations. I have pinged libcxx devs about that.

clang/include/clang/Sema/Sema.h

2569

Hmm, enums are more en vogue :)

How about we:

• Rename TypeAccessKind -> DiagCtorKind
• Rename Explicit -> None

?

[philnik]

Would it be possible to just add #if TEST_CLANG_VER >= 1600 around the new warnings?

[mizvekov]

In D136533#3877974, @philnik wrote:

Would it be possible to just add #if TEST_CLANG_VER >= 1600 around the new warnings?

That works for me, thanks!

[mizvekov]

@philnik it seems that didn't work. It seems those pipelines are already running clang-16, according to the cmake logs, they are just not bootstrapping with this patch.

So gating this on clang version will not work.

[erichkeane]

clang/lib/Sema/SemaDecl.cpp

176

So I know our coding standard is a little awkward here, but I'd prefer curley braces here. I think the 'size' of the Diag line here qualifies as 'multiple lines'.

[ychen]

clang/include/clang/Sema/Sema.h

2569

Yep. That works for me. Thanks.

[erichkeane]

I'm happy, thanks!

[glandium]

This breaks compiling many things on macOS, including compiler-rt:

/tmp/llvm2/obj/bin/clang++ --target=aarch64-apple-darwin  -I/tmp/llvm2/compiler-rt/lib/fuzzer/../../include -Wall -Wno-unused-parameter -O3 -DNDEBUG -arch arm64 -isysroot /tmp/MacOSX11.3.sdk -stdlib=libc++ -mmacosx-version-min=10.10 -isysroot /tmp/MacOSX11.3.sdk -fPIC -fno-builtin -fno-exceptions -funwind-tables -fno-stack-protector -fno-sanitize=safe-stack -fvisibility=hidden -fno-lto -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta -O3 -g -Wno-gnu -Wno-variadic-macros -Wno-c99-extensions -fno-omit-frame-pointer -std=c++17 -MD -MT lib/fuzzer/CMakeFiles/RTfuzzer.osx.dir/FuzzerFork.cpp.o -MF lib/fuzzer/CMakeFiles/RTfuzzer.osx.dir/FuzzerFork.cpp.o.d -o lib/fuzzer/CMakeFiles/RTfuzzer.osx.dir/FuzzerFork.cpp.o -c /tmp/llvm2/compiler-rt/lib/fuzzer/FuzzerFork.cpp
In file included from /tmp/llvm2/compiler-rt/lib/fuzzer/FuzzerFork.cpp:11:
In file included from /tmp/llvm2/compiler-rt/lib/fuzzer/FuzzerCommand.h:15:
In file included from /tmp/llvm2/compiler-rt/lib/fuzzer/FuzzerDefs.h:18:
In file included from /tmp/MacOSX11.3.sdk/usr/include/c++/v1/memory:667:
/tmp/MacOSX11.3.sdk/usr/include/c++/v1/type_traits:1672:66: error: 'type' is unavailable: introduced in macOS 10.15
    typedef _LIBCPP_NODEBUG_TYPE typename remove_reference<_Tp>::type _Up;
                                                                 ^
/tmp/MacOSX11.3.sdk/usr/include/c++/v1/filesystem:603:47: note: in instantiation of template class 'std::decay<std::filesystem::path>' requested here
template <class _Source, class _DS = typename decay<_Source>::type,
                                              ^
/tmp/MacOSX11.3.sdk/usr/include/c++/v1/filesystem:648:31: note: in instantiation of default argument for '__is_pathable_char_array<std::filesystem::path>' required here
          bool _IsCharIterT = __is_pathable_char_array<_Tp>::value,
                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/MacOSX11.3.sdk/usr/include/c++/v1/filesystem:741:26: note: in instantiation of default argument for '__is_pathable<std::filesystem::path, false>' required here
      typename enable_if<__is_pathable<_SourceOrIter>::value, _Tp>::type;
                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/MacOSX11.3.sdk/usr/include/c++/v1/filesystem:861:29: note: in instantiation of template type alias '_EnableIfPathable' requested here
  _LIBCPP_INLINE_VISIBILITY _EnableIfPathable<_Source>
                            ^
/tmp/MacOSX11.3.sdk/usr/include/c++/v1/filesystem:965:19: note: while substituting deduced template arguments into function template 'operator/=' [with _Source = path]
    return (*this /= __replacement);
                  ^
/tmp/MacOSX11.3.sdk/usr/include/c++/v1/filesystem:738:24: note: 'path' has been explicitly marked unavailable here
class _LIBCPP_TYPE_VIS path {
                       ^

etc.

[mizvekov]

pinging @kcc for LibFuzzer, I believe this needs to be fixed there, if we don't want to bump the MacOS minimum version.

[thakis]

+1, this breaks building LLVM on macOS.

This isn't limited to libFuzzer. See the many hits for "error:" on https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket/8799329305964156353/+/u/package_clang/stdout?format=raw

Let's revert this until LLVM can build again.

[mizvekov]

I don't have access to a macOS machine, and I don't believe any of the pre-commit CI machines are running it either.
So I don't have much to go on here.

How do you think we shall proceed?

[erichkeane]

In D136533#3882976, @mizvekov wrote:

I don't have access to a macOS machine, and I don't believe any of the pre-commit CI machines are running it either.
So I don't have much to go on here.

How do you think we shall proceed?

One thing you might try is seeing if this is a libcxx-specific thing instead, and try doing a self-build using libcxx.

[mizvekov]

In D136533#3883048, @erichkeane wrote:

One thing you might try is seeing if this is a libcxx-specific thing instead, and try doing a self-build using libcxx.

We tested this on the libc++ CI, as this patch touches a test case in there, and I also tested it on my machine on both windows and linux.

I believe this is a macOS specific issue, and I don't believe the problem is in this patch.

In either case, this seems to be an issue with libc++ that is out there in the wild.

There are likely some cases where folks were using unsupported features for their macOS version, and this bug that we fixed prevented us from correctly diagnosing that.

If I understood everything correctly so far, we can't change shipped libc++.
So we either:

• Bump the minimum macOS version.
• Stop using whatever libc++ declared as being unsupported in those old macOS versions.

libc++ gated that feature on a minimum OS version, if you try to use it otherwise, this doesn't compile, as intended.

Would it even be right to add some kind of libc++ specific workaround, if that configuration was declared as unsupported?

[erichkeane]

In D136533#3883089, @mizvekov wrote:

In D136533#3883048, @erichkeane wrote:

One thing you might try is seeing if this is a libcxx-specific thing instead, and try doing a self-build using libcxx.

We tested this on the libc++ CI, as this patch touches a test case in there, and I also tested it on my machine on both windows and linux.

I believe this is a macOS specific issue, and I don't believe the problem is in this patch.

In either case, this seems to be an issue with libc++ that is out there in the wild.

There are likely some cases where folks were using unsupported features for their macOS version, and this bug that we fixed prevented us from correctly diagnosing that.

If I understood everything correctly so far, we can't change shipped libc++.
So we either:

• Bump the minimum macOS version.
• Stop using whatever libc++ declared as being unsupported in those old macOS versions.

libc++ gated that feature on a minimum OS version, if you try to use it otherwise, this doesn't compile, as intended.

Would it even be right to add some kind of libc++ specific workaround, if that configuration was declared as unsupported?

I don't believe that the libc++ CI does 'self build with libcxx as the host library', so it would not have caught this. That said, this could definitely just be an issue in the version that ships with macos, and we cannot really release something that doesn't self-build on one of our supported platforms.

[mizvekov]

In D136533#3883116, @erichkeane wrote:

I don't believe that the libc++ CI does 'self build with libcxx as the host library', so it would not have caught this. That said, this could definitely just be an issue in the version that ships with macos, and we cannot really release something that doesn't self-build on one of our supported platforms.

That's true, self build is not tested there.

[glandium]

In either case, this seems to be an issue with libc++ that is out there in the wild.

I don't think it is, actually. I think it's doing something legitimate. That is, it's declaring in its headers that some things are only available on some versions of macos, which is fine if you don't use them, or if you use them in a block guarded with __builtin_available. The problem this patch introduces is that it makes it an error to even have those declarations.

[glandium]

(And specifically, FuzzerFork.cpp doesn't use any of those declared things. Its only sin is to #include <fstream>, which happens to have /some/ methods (std::filesystem-related) only available on macos 10.15+.)

Edit: Corollary: anything that #include <fstream>, builds with C++17 or more, and wants to support macos < 10.15, is broken by this patch.

[mizvekov]

In D136533#3884364, @glandium wrote:

(And specifically, FuzzerFork.cpp doesn't use any of those declared things. Its only sin is to #include <fstream>, which happens to have /some/ methods (std::filesystem-related) only available on macos 10.15+.)

Edit: Corollary: anything that #include <fstream>, builds with C++17 or more, and wants to support macos < 10.15, is broken by this patch.

Nonetheless, it seems libc++ uses a deprecated declaration from it's own headers.

@ldionne FYI, we may have a libc++ issue here.

[glandium]

In D136533#3885302, @mizvekov wrote:

Nonetheless, it seems libc++ uses a deprecated declaration from it's own headers.

It's the opposite of deprecated, it's (optionally) using something that is only available in newer versions of macos.

[mizvekov]

In D136533#3886666, @glandium wrote:

It's the opposite of deprecated, it's (optionally) using something that is only available in newer versions of macos.

Right, I mis-typed, but this is probably using the 'availability' attribute, which works on the same mechanism as 'deprecated', except that it gives an error.

[ldionne]

Is it possible that it would be one of those two bugs? https://github.com/llvm/llvm-project/issues?q=is%3Aopen+is%3Aissue+author%3Aldionne+availability+

[mizvekov]

In D136533#3891905, @ldionne wrote:

Is it possible that it would be one of those two bugs? https://github.com/llvm/llvm-project/issues?q=is%3Aopen+is%3Aissue+author%3Aldionne+availability+

I don't think so.

It does look like libc++ makes some uses within itself of types which were marked unavailable.
It's just that we missed diagnosing those cases before, which is what this patch is fixing.

Ie if you see the included test case:

struct A {
  using ta [[deprecated]] = int;
};
using t1 = typename A::ta; // expected-warning {{'ta' is deprecated}}

We missed diagnosing that before, when performing a 'typename' member access.
The availability attribute is supposed to work on the same principles as deprecated does.

This is diagnosed by GCC as well, and I thought libc++ was also tested with it. Is that maybe not the case for those old Apple systems?

In any case, let me know if you think otherwise, or if we will need to implement some kind of workaround...

[ldionne]

Ah, I understand. Thanks for explaining and sorry, I had only taken a quick look.

So -- the reason why this isn't found by the libc++ CI is that it only triggers when we build on macOS, and our CI does not perform a bootstrapping build on macOS. In other words, we're never using your new modified Clang to run our tests on macOS in the libc++ CI.

Regarding the bug itself, I am not sure I understand why this is triggering because the path class itself *is* marked as unavailable, which means that all of its methods should be marked as unavailable. I'll have to pull your patch down, build my Clang and play around with that.

In terms of the next steps:

1. I'll figure out what's wrong with libc++ and if it's indeed a bug in libc++. If so, I'll fix it ASAP.
2. Shipping this change does mean that anyone building anything with a new Clang and a not-yet-updated libc++ with a deployment target before 10.15 (or whatever first version we shipped filesystem in) will fail. IMO that's kind of annoying, but may be OK if we fix libc++ first.

[ldionne]

In D136533#3892949, @ldionne wrote:

2. Shipping this change does mean that anyone building anything with a new Clang and a not-yet-updated libc++ with a deployment target before 10.15 (or whatever first version we shipped filesystem in) will fail. IMO that's kind of annoying, but may be OK if we fix libc++ first.

I guess the interesting question here would be: @thakis, is there a reason why you are using the SDK-provided libc++ but the tip-of-trunk Clang for building Chrome? (I feel like we've talked about this before but I don't remember).

[thakis]

In D136533#3893032, @ldionne wrote:

In D136533#3892949, @ldionne wrote:

2. Shipping this change does mean that anyone building anything with a new Clang and a not-yet-updated libc++ with a deployment target before 10.15 (or whatever first version we shipped filesystem in) will fail. IMO that's kind of annoying, but may be OK if we fix libc++ first.

I guess the interesting question here would be: @thakis, is there a reason why you are using the SDK-provided libc++ but the tip-of-trunk Clang for building Chrome? (I feel like we've talked about this before but I don't remember).

Because that's what you recommended :) We used to use the just-built libc++ but that had other issues.

Previous discussions that I found in a hurry: https://reviews.llvm.org/D128927#3670288 , and https://reviews.llvm.org/D82702#2153627 and onward.

[thakis]

Adding @hans @aeubanks since they were involved over there too.

[ldionne]

In D136533#3894958, @thakis wrote:

In D136533#3893032, @ldionne wrote:

In D136533#3892949, @ldionne wrote:

2. Shipping this change does mean that anyone building anything with a new Clang and a not-yet-updated libc++ with a deployment target before 10.15 (or whatever first version we shipped filesystem in) will fail. IMO that's kind of annoying, but may be OK if we fix libc++ first.

I guess the interesting question here would be: @thakis, is there a reason why you are using the SDK-provided libc++ but the tip-of-trunk Clang for building Chrome? (I feel like we've talked about this before but I don't remember).

Because that's what you recommended :) We used to use the just-built libc++ but that had other issues.

Ugh, OK. Yeah, your previous setup was even much weirder in fact, it used the trunk libc++ headers but linked against the system dylib IIRC. I stand by my previous recommendation, but your setup is still unusual: you're using the SDK libc++ with a ToT clang, which is technically not a supported combination. The best would be to use the Apple-provided Clang on macOS as well, but I understand you might have reasons to want to use the ToT clang instead.

I investigated the issue and the problem reproduces for me with:

cat <<EOF | xcrun <path-to-clang-with-this-patch-applied>/clang++ -xc++ - -mmacosx-version-min=10.12 -std=c++17 -v -nostdinc++ -isystem <path-to-just-built-LLVM-libc++>
#include <filesystem>
#include <type_traits>
int main() { }
EOF

It does not reproduce with trunk -- I bisected the fix to:

commit 5fab33af7f083a0043112742027172e9f297c07f
Author: Nikolas Klauser <[e-mail]>
Date:   Tue Sep 6 00:33:34 2022 +0200

    [libc++] Avoid instantiating type_trait classes

    Use `using` aliases to avoid instantiating lots of types

    Reviewed By: ldionne, #libc

    Spies: libcxx-commits, miyuki

    Differential Revision: https://reviews.llvm.org/D132785

Looking at that commit, which had nothing to do with fixing availability markup, I think this Clang patch might still be missing some diagnostics? Should it diagnose when the typename is accessed through an alias?

If that's the case, libc++ would fail with trunk as well, since 5fab33af7f083 would not silence these issues. And if that's the case, then I am not sure how to fix libc++ at all. I suspect the problem here is really https://github.com/llvm/llvm-project/issues/40340, since we do mark the filesystem classes as unavailable as we should.

[mizvekov]

In D136533#3896313, @ldionne wrote:

Looking at that commit, which had nothing to do with fixing availability markup, I think this Clang patch might still be missing some diagnostics? Should it diagnose when the typename is accessed through an alias?

If that's the case, libc++ would fail with trunk as well, since 5fab33af7f083 would not silence these issues. And if that's the case, then I am not sure how to fix libc++ at all. I suspect the problem here is really https://github.com/llvm/llvm-project/issues/40340, since we do mark the filesystem classes as unavailable as we should.

It's quite possible it's missing other cases, there isn't a general helper in clang to access a declaration which would 'automatically' perform the use-checking, so I expect a lot of places to be repeating code / calling low level accessors which do not.

I haven't come across an issue with type aliases, though I am not on a personal crusade to fix all these right now, this just came across while I was working on something else.

Now that you mention it, looking at the code, I think we don't diagnose an use of a type alias itself, if that is what you mean?

Ie, clang doesn't, GCC does, MSVC doesn't: https://godbolt.org/z/WEo4enjbz

Would fixing that be a problem for libc++?

Otherwise, typename accesses through the type alias pattern itself should work like from any context, except that if the access happens through a dependent entity, we should be diagnosing it when we instantiate the type alias.

Does what you are talking about fall into any of that? Otherwise, do you have a short example?

---

Otherwise, if current libc++ is fine and building older libc++with newer clang is not supported, is everyone fine with merging this back, as is?
@thakis ?

[glandium]

you're using the SDK libc++ with a ToT clang, which is technically not a supported combination.

Where is it specified that it's not a supported combination? Why should it not be supported? You don't even get libc++ from the llvm tree unless you explicitly enable it.

[ldionne]

In D136533#3896536, @mizvekov wrote:

Now that you mention it, looking at the code, I think we don't diagnose an use of a type alias itself, if that is what you mean?

Ie, clang doesn't, GCC does, MSVC doesn't: https://godbolt.org/z/WEo4enjbz

Would fixing that be a problem for libc++?

It might be a problem, but I would argue we should still do it after fixing any problematic cases. It seems like Clang's current behavior is broken, as it basically ignores the [[deprecated]] attribute on aliases?

Otherwise, typename accesses through the type alias pattern itself should work like from any context, except that if the access happens through a dependent entity, we should be diagnosing it when we instantiate the type alias.

Does what you are talking about fall into any of that? Otherwise, do you have a short example?

Sorry, I'm a bit lost. Let's take it back to the errors we're actually seeing with my reproducer above:

In file included from <stdin>:1:
In file included from SDK/usr/include/c++/v1/filesystem:245:
In file included from SDK/usr/include/c++/v1/__filesystem/directory_entry.h:14:
In file included from SDK/usr/include/c++/v1/__chrono/time_point.h:13:
In file included from SDK/usr/include/c++/v1/__chrono/duration.h:14:
In file included from SDK/usr/include/c++/v1/limits:107:
In file included from SDK/usr/include/c++/v1/type_traits:421:
In file included from SDK/usr/include/c++/v1/__functional/invoke.h:17:
In file included from SDK/usr/include/c++/v1/__type_traits/decay.h:13:
SDK/usr/include/c++/v1/__type_traits/add_pointer.h:26:50: error: 'type' is unavailable: introduced in macOS 10.15
                _IsSame<typename remove_cv<_Tp>::type, void>::value>
                                                 ^
SDK/usr/include/c++/v1/__type_traits/add_pointer.h:33:39: note: in instantiation of default argument for '__add_pointer_impl<std::filesystem::file_status>' required here
    {typedef _LIBCPP_NODEBUG typename __add_pointer_impl<_Tp>::type type;};
                                      ^~~~~~~~~~~~~~~~~~~~~~~
SDK/usr/include/c++/v1/__type_traits/decay.h:44:40: note: in instantiation of template class 'std::add_pointer<std::filesystem::file_status>' requested here
                              typename add_pointer<_Up>::type,
                                       ^
SDK/usr/include/c++/v1/__type_traits/decay.h:56:38: note: in instantiation of template class 'std::__decay<std::filesystem::file_status, true>' requested here
    typedef _LIBCPP_NODEBUG typename __decay<_Up, __is_referenceable<_Up>::value>::type type;
                                     ^
SDK/usr/include/c++/v1/__filesystem/path.h:142:47: note: in instantiation of template class 'std::decay<std::filesystem::file_status>' requested here
template <class _Source, class _DS = typename decay<_Source>::type,
                                              ^
SDK/usr/include/c++/v1/__filesystem/path.h:195:31: note: in instantiation of default argument for '__is_pathable_char_array<std::filesystem::file_status>' required here
          bool _IsCharIterT = __is_pathable_char_array<_Tp>::value,
                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
SDK/usr/include/c++/v1/__filesystem/path.h:445:26: note: in instantiation of default argument for '__is_pathable<std::filesystem::file_status, false>' required here
      typename enable_if<__is_pathable<_SourceOrIter>::value, _Tp>::type;
                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
SDK/usr/include/c++/v1/__filesystem/path.h:480:36: note: in instantiation of template type alias '_EnableIfPathable' requested here
  template <class _Source, class = _EnableIfPathable<_Source, void> >
                                   ^
SDK/usr/include/c++/v1/__filesystem/path.h:482:3: note: in instantiation of default argument for 'path<std::filesystem::file_status>' required here
  path(const _Source& __src, format = format::auto_format) {
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
SDK/usr/include/c++/v1/__filesystem/operations.h:99:75: note: while substituting deduced template arguments into function template 'path' [with _Source = file_status, $1 = (no value)]
inline _LIBCPP_HIDE_FROM_ABI bool exists(const path& __p) { return exists(__status(__p)); }
                                                                          ^
SDK/usr/include/c++/v1/__filesystem/file_status.h:28:24: note: 'file_status' has been explicitly marked unavailable here
class _LIBCPP_TYPE_VIS file_status {
                       ^

file_status, exists and path::path are all marked as unavailable. Why do we diagnose the use of an unavailable type wayyy lower in the stack when we instantiate remove_cv? I am not seeing what libc++ is doing wrong.

Otherwise, if current libc++ is fine and building older libc++with newer clang is not supported, is everyone fine with merging this back, as is?
@thakis ?

See below for what I meant by "unsupported". Another way to think about this would be to say that Clang has the burden of keeping a slightly-older libc++ compiling. If that's the case, then one could argue that this change should be held off until Clang no longer cares about being able to compile that version of libc++. This is not really my decision to make, but I would argue that there's probably value in keeping things working (that would bite people not only on OS X, but anyone using an older libc++ with a newer Clang).

If so, then perhaps it would make sense to wait a few months before we merge this, perhaps at least until LLVM 16 is out so there's an officially-released version of libc++ that doesn't break with this change. But I think that's a decision for the Clang folks to make, not libc++.

In D136533#3897516, @glandium wrote:

you're using the SDK libc++ with a ToT clang, which is technically not a supported combination.

Where is it specified that it's not a supported combination? Why should it not be supported? You don't even get libc++ from the llvm tree unless you explicitly enable it.

I guess I should reword it that way: If you're using an older libc++ with a newer Clang, there is literally no way for that old libc++ to guarantee that it can work with that not-known-yet Clang. It would imply being forward compatible with potentially arbitrary changes in Clang, which is nonsensical. IOW, we can't guard against issues that we don't know exist yet. Does that make sense?

[mizvekov]

In D136533#3899628, @ldionne wrote:

It might be a problem, but I would argue we should still do it after fixing any problematic cases. It seems like Clang's current behavior is broken, as it basically ignores the [[deprecated]] attribute on aliases?

Yep. I expect we will find lots of similar cases.

If so, then perhaps it would make sense to wait a few months before we merge this, perhaps at least until LLVM 16 is out so there's an officially-released version of libc++ that doesn't break with this change. But I think that's a decision for the Clang folks to make, not libc++.

I would be completely fine with doing a targeted workaround for libc++. The longer we don't patch this and warn on new code, the less likely we will ever be able to get rid of the bug. The problem is that we don't fully understand what is going on here, and how narrow this workaround can be.
I will try to take a look at this again, and see if I can reduce it to something that makes either libc++ or clang seem unreasonable. But I'll be off to the devmtg soon, and then take some vacations, so it will take a while.

[thakis]

We're happy to switch to any setup that allows us to do a bootstrap build of clang on macOS (ie one where we build clang with whatever host compiler, and then build it again with just-built clang). We've switched to the currently recommended method several times; if there's a new recommended method we can switch to that. We just need to know what it is :)

From what I understand, using newer libc++ headers and linking against system libc++.dylib is explicitly supported (because ABI compat).

[mizvekov]

FYI just another data point, I haven't taken a closer look yet, but now we start diagnosing some deprecated uses of std::experimental::pmr::polymorphic_allocator on current libc++ linux CI.

clang/docs/ReleaseNotes.rst

- Finished implementing C++ DR2565, which results in a requirement becoming
   context, but not in a requires clause evaluated as a boolean.
 - Address the thread identification problems in coroutines.
   `Issue 47177 <https://github.com/llvm/llvm-project/issues/47177>`_
   `Issue 47179 <https://github.com/llvm/llvm-project/issues/47179>`_
 - Fix a crash upon stray coloncolon token in C2x mode.
 - Reject non-type template arguments formed by casting a non-zero integer
   to a pointer in pre-C++17 modes, instead of treating them as null
   pointers.
+- Fix missing diagnostics for uses of declarations when performing typename access,
+  such as when performing member access on a '[[deprecated]]' type alias.
+  `Issue 58547 <https://github.com/llvm/llvm-project/issues/58547>`
 
 Improvements to Clang's diagnostics
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 - Clang will now check compile-time determinable string literals as format strings.
   Fixes `Issue 55805: <https://github.com/llvm/llvm-project/issues/55805>`_.
 - ``-Wformat`` now recognizes ``%b`` for the ``printf``/``scanf`` family of
   functions and ``%B`` for the ``printf`` family of functions. Fixes
   `Issue 56885: <https://github.com/llvm/llvm-project/issues/56885>`_.

clang/include/clang/Sema/Sema.h

public:
   };
 
   DeclGroupPtrTy ConvertDeclToDeclGroup(Decl *Ptr, Decl *OwnedType = nullptr);
 
   void DiagnoseUseOfUnimplementedSelectors();
 
   bool isSimpleTypeSpecifier(tok::TokenKind Kind) const;
 
+  enum class DiagCtorKind { None, Implicit, Typename };
+  QualType getTypeDeclType(DeclContext *LookupCtx, DiagCtorKind DCK,
+                           TypeDecl *TD, SourceLocation NameLoc);
+
   ParsedType getTypeName(const IdentifierInfo &II, SourceLocation NameLoc,
                          Scope *S, CXXScopeSpec *SS = nullptr,
                          bool isClassName = false, bool HasTrailingDot = false,
                          ParsedType ObjectType = nullptr,
                          bool IsCtorOrDtorName = false,
                          bool WantNontrivialTypeSourceInfo = false,
                          bool IsClassTemplateDeductionContext = true,
                          ImplicitTypenameContext AllowImplicitTypename =

clang/lib/Sema/SemaDecl.cpp

#include "clang/Basic/TransformTypeTraits.def"
 
   default:
     break;
   }
 
   return false;
 }
 
+QualType Sema::getTypeDeclType(DeclContext *LookupCtx, DiagCtorKind DCK,
+                               TypeDecl *TD, SourceLocation NameLoc) {
+  auto *LookupRD = dyn_cast_or_null<CXXRecordDecl>(LookupCtx);
+  auto *FoundRD = dyn_cast<CXXRecordDecl>(TD);
+  if (DCK != DiagCtorKind::None && LookupRD && FoundRD &&
+      FoundRD->isInjectedClassName() &&
+      declaresSameEntity(LookupRD, cast<Decl>(FoundRD->getParent())))
+    Diag(NameLoc,
+         DCK == DiagCtorKind::Typename
+             ? diag::ext_out_of_line_qualified_id_type_names_constructor
+             : diag::err_out_of_line_qualified_id_type_names_constructor)
+        << TD->getIdentifier() << /*Type*/ 1
+        << 0 /*if any keyword was present, it was 'typename'*/;
+
+  DiagnoseUseOfDecl(TD, NameLoc);
+  MarkAnyDeclReferenced(TD->getLocation(), TD, /*OdrUse=*/false);
+  return Context.getTypeDeclType(TD);
+}
+
 namespace {
 enum class UnqualifiedTypeNameLookupResult {
   NotFound,
   FoundNonType,
   FoundType
 };
 } // end anonymous namespace
 

 ParsedType Sema::getTypeName(const IdentifierInfo &II, SourceLocation NameLoc,
                              Scope *S, CXXScopeSpec *SS, bool isClassName,
                              bool HasTrailingDot, ParsedType ObjectTypePtr,
                              bool IsCtorOrDtorName,
                              bool WantNontrivialTypeSourceInfo,
                              bool IsClassTemplateDeductionContext,
                              ImplicitTypenameContext AllowImplicitTypename,
                              IdentifierInfo **CorrectedII) {
+  bool IsImplicitTypename = !isClassName && !IsCtorOrDtorName;
   // FIXME: Consider allowing this outside C++1z mode as an extension.
   bool AllowDeducedTemplate = IsClassTemplateDeductionContext &&
-                              getLangOpts().CPlusPlus17 && !IsCtorOrDtorName &&
-                              !isClassName && !HasTrailingDot;
+                              getLangOpts().CPlusPlus17 && IsImplicitTypename &&
+                              !HasTrailingDot;
 
   // Determine where we will perform name lookup.
   DeclContext *LookupCtx = nullptr;
   if (ObjectTypePtr) {
     QualType ObjectType = ObjectTypePtr.get();
     if (ObjectType->isRecordType())
       LookupCtx = computeDeclContext(ObjectType);
   } else if (SS && SS->isNotEmpty()) {
     LookupCtx = computeDeclContext(*SS, false);
 
     if (!LookupCtx) {
       if (isDependentScopeSpecifier(*SS)) {
         // C++ [temp.res]p3:
         //   A qualified-id that refers to a type and in which the
         //   nested-name-specifier depends on a template-parameter (14.6.2)
         //   shall be prefixed by the keyword typename to indicate that the
         //   qualified-id denotes a type, forming an
         //   elaborated-type-specifier (7.1.5.3).
         //
         // We therefore do not perform any name lookup if the result would
         // refer to a member of an unknown specialization.
         // In C++2a, in several contexts a 'typename' is not required. Also
         // allow this as an extension.
-        if (AllowImplicitTypename == ImplicitTypenameContext::No &&
-            !isClassName && !IsCtorOrDtorName)
-          return nullptr;
-        bool IsImplicitTypename = !isClassName && !IsCtorOrDtorName;
         if (IsImplicitTypename) {
+          if (AllowImplicitTypename == ImplicitTypenameContext::No)
+            return nullptr;
           SourceLocation QualifiedLoc = SS->getRange().getBegin();
           if (getLangOpts().CPlusPlus20)
             Diag(QualifiedLoc, diag::warn_cxx17_compat_implicit_typename);
           else
             Diag(QualifiedLoc, diag::ext_implicit_typename)
                 << SS->getScopeRep() << II.getName()
                 << FixItHint::CreateInsertion(QualifiedLoc, "typename ");
         }
ParsedType Sema::getTypeName(const IdentifierInfo &II, SourceLocation NameLoc,
 
   assert(IIDecl && "Didn't find decl");
 
   QualType T;
   if (TypeDecl *TD = dyn_cast<TypeDecl>(IIDecl)) {
     // C++ [class.qual]p2: A lookup that would find the injected-class-name
     // instead names the constructors of the class, except when naming a class.
     // This is ill-formed when we're not actually forming a ctor or dtor name.
-    auto *LookupRD = dyn_cast_or_null<CXXRecordDecl>(LookupCtx);
-    auto *FoundRD = dyn_cast<CXXRecordDecl>(TD);
-    if (!isClassName && !IsCtorOrDtorName && LookupRD && FoundRD &&
-        FoundRD->isInjectedClassName() &&
-        declaresSameEntity(LookupRD, cast<Decl>(FoundRD->getParent())))
-      Diag(NameLoc, diag::err_out_of_line_qualified_id_type_names_constructor)
-          << &II << /*Type*/1;
-
-    DiagnoseUseOfDecl(IIDecl, NameLoc);
-
-    T = Context.getTypeDeclType(TD);
-    MarkAnyDeclReferenced(TD->getLocation(), TD, /*OdrUse=*/false);
+    T = getTypeDeclType(LookupCtx,
+                        IsImplicitTypename ? DiagCtorKind::Implicit
+                                           : DiagCtorKind::None,
+                        TD, NameLoc);
   } else if (ObjCInterfaceDecl *IDecl = dyn_cast<ObjCInterfaceDecl>(IIDecl)) {
     (void)DiagnoseUseOfDecl(IDecl, NameLoc);
     if (!HasTrailingDot)
       T = Context.getObjCInterfaceType(IDecl);
     FoundUsingShadow = nullptr; // FIXME: Target must be a TypeDecl.
   } else if (auto *UD = dyn_cast<UnresolvedUsingIfExistsDecl>(IIDecl)) {
     (void)DiagnoseUseOfDecl(UD, NameLoc);
     // Recover with 'int'

clang/lib/Sema/SemaTemplate.cpp

if (TypeDecl *Type = dyn_cast<TypeDecl>(Result.getFoundDecl())) {
       //
       // Unlike in an elaborated-type-specifier, function names are not ignored
       // in typename-specifier lookup. However, they are ignored in all the
       // contexts where we form a typename type with no keyword (that is, in
       // mem-initializer-ids, base-specifiers, and elaborated-type-specifiers).
       //
       // FIXME: That's not strictly true: mem-initializer-id lookup does not
       // ignore functions, but that appears to be an oversight.
-      auto *LookupRD = dyn_cast_or_null<CXXRecordDecl>(Ctx);
-      auto *FoundRD = dyn_cast<CXXRecordDecl>(Type);
-      if (Keyword == ETK_Typename && LookupRD && FoundRD &&
-          FoundRD->isInjectedClassName() &&
-          declaresSameEntity(LookupRD, cast<Decl>(FoundRD->getParent())))
-        Diag(IILoc, diag::ext_out_of_line_qualified_id_type_names_constructor)
-            << &II << 1 << 0 /*'typename' keyword used*/;
-
+      QualType T = getTypeDeclType(
+          Ctx,
+          Keyword == ETK_Typename ? DiagCtorKind::Typename : DiagCtorKind::None,
+          Type, IILoc);
       // We found a type. Build an ElaboratedType, since the
       // typename-specifier was just sugar.
-      MarkAnyDeclReferenced(Type->getLocation(), Type, /*OdrUse=*/false);
-      return Context.getElaboratedType(Keyword,
-                                       QualifierLoc.getNestedNameSpecifier(),
-                                       Context.getTypeDeclType(Type));
+      return Context.getElaboratedType(
+          Keyword, QualifierLoc.getNestedNameSpecifier(), T);
     }
 
     // C++ [dcl.type.simple]p2:
     //   A type-specifier of the form
     //     typename[opt] nested-name-specifier[opt] template-name
     //   is a placeholder for a deduced class type [...].
     if (getLangOpts().CPlusPlus17) {
       if (auto *TD = getAsTypeTemplateDecl(Result.getFoundDecl())) {

clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.deprecated/p1.cpp

 }
 
 template <typename T>
 [[deprecated]]T some_func2(T t) {
   struct FunS2{};
   FunS2 f;// No warning, entire function is deprecated, so usage here should be fine.
 
 }
+
+namespace GH58547 {
+struct A {
+  using ta [[deprecated]] = int; // expected-note 2{{marked deprecated here}}
+};
+
+using t1 = typename A::ta; // expected-warning {{'ta' is deprecated}}
+
+template <class B1> struct B {
+  using tb = typename B1::ta; // expected-warning {{'ta' is deprecated}}
+};
+
+template struct B<A>; // expected-note {{requested here}}
+} // namespace GH58547

libcxx/test/libcxx/depr/depr.default.allocator/allocator.members/allocate.depr_in_cxx17.verify.cpp

 // ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_ENABLE_CXX20_REMOVED_ALLOCATOR_MEMBERS
 
 #include <memory>
 #include "test_macros.h"
 
 int main(int, char**)
 {
     std::allocator<int> a;
-    TEST_IGNORE_NODISCARD a.allocate(3, nullptr); // expected-warning {{'allocate' is deprecated}}
+    TEST_IGNORE_NODISCARD a.allocate(3, nullptr);
+    // expected-warning@-1 {{'allocate' is deprecated}}
+    // expected-warning@*:* {{'pointer' is deprecated}}
+    // expected-warning@*:* {{'const_pointer' is deprecated}}
 
     return 0;
 }