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

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

In D136533#3894958 <https://reviews.llvm.org/D136533#3894958>, @thakis wrote:

> In D136533#3893032 <https://reviews.llvm.org/D136533#3893032>, @ldionne wrote:
>
>> In D136533#3892949 <https://reviews.llvm.org/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 <nikolasklauser at berlin.de>
  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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136533/new/

https://reviews.llvm.org/D136533