[PATCH] D139986: [clang][TypePrinter] Teach isSubstitutedDefaultArgument about integral types

Tue Dec 20 08:26:57 PST 2022

Mordante added a comment.

In D139986#4005997 <https://reviews.llvm.org/D139986#4005997>, @dblaikie wrote:

> In D139986#4003873 <https://reviews.llvm.org/D139986#4003873>, @Mordante wrote:
>
>> In D139986#4001180 <https://reviews.llvm.org/D139986#4001180>, @Michael137 wrote:
>>
>>> Missed couple of test cases in libcxx
>>> About to fix those
>>
>> There were more breakage due to this patch, which I fixed in D140272 <https://reviews.llvm.org/D140272>.
>>
>> Next time please don't commit patches when the pre-commit CI is red. This build https://buildkite.com/llvm-project/libcxx-ci/builds/16118 shows the patch will break libc++. These pre-commit CI jobs were added specifically to aid the Clang developers to validate whether their changes break libc++. Libc++ heavily relies on its pre-commit CI so breaking the CI has a huge impact.
>> Note that when libc++ breaks there might be other projects that use the latest Clang HEAD that will be affected too. (Not likely with diagnostics, but likely when the modular build fails.)
>>
>> When you have issues resolving the libc++ issues you can always reach out to us for assistance.
>
> Perhaps these LibC++ tests shouldn't be testing/using clang warnings? (could the warnings be turned off in the libc++ tests?)

In the failing tests we want to make sure we marked a struct as `[[deprecated]]`. In other tests we validate error messages to make sure things are ill-formed as mandated. We test the diagnostic to make sure the compilation fails for the expected reason and not a different compilation error. Do you have a suggestion how we can do that without checking Clang diagnostics?

In general I consider it bad practice to commit code when the CI is red.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139986