[libcxx] Fix the broken test after D82657.

[hokein]

Summary

D82657 slightly changes clang's diagnostic. After that commit, clang emits a secondary diagnostic in one of the libcxx test, this patch fixes it.

[hokein]

I'm not sure this is the correct solution to fix the failing test due to the new diagnostic of the compiler.

The test will pass if it runs under the trunk clang; and it fails if it runs under an old clang;

After D82657

• it breaks one of the builtbots: http://lab.llvm.org:8011/builders/llvm-clang-win-x-armv7l/builds/945, this buildbot seems to use trunk clang;
• there are other buildbots (e.g. http://lab.llvm.org:8011/builders/libcxx-libcxxabi-x86_64-linux-ubuntu-cxx03/builds/3823/steps/test.libcxx/logs/stdio) which uses /usr/local/bin/clang++

Any ideas?

[ldionne]

In D86685#2241223, @hokein wrote:

I'm not sure this is the correct solution to fix the failing test due to the new diagnostic of the compiler.

The test will pass if it runs under the trunk clang; and it fails if it runs under an old clang;

After D82657

• it breaks one of the builtbots: http://lab.llvm.org:8011/builders/llvm-clang-win-x-armv7l/builds/945, this buildbot seems to use trunk clang;
• there are other buildbots (e.g. http://lab.llvm.org:8011/builders/libcxx-libcxxabi-x86_64-linux-ubuntu-cxx03/builds/3823/steps/test.libcxx/logs/stdio) which uses /usr/local/bin/clang++

Any ideas?

The correct fix is to ensure that Clang doesn't produce a second diagnostic, since that second diagnostic is bogus. Working around the issue by annotating the libc++ tests isn't the right solution.

[sammccall]

@hokein here's a minimized version:

template <int>
struct flag {
  static constexpr bool value = true;
};

template <class T>
struct trait : flag<sizeof(T)> {};

template <class T, bool Inferred = trait<T>::value>
struct a {};

template <class T>
class b {
  a<T> x;
  using U = a<T>;
};

template <int>
struct Impossible {
  static_assert(false, "");
};

constexpr auto v = trait<b<Impossible<0>>>::value;

I see the static_assert and a spurious:

error: no member named 'value' in 'trait<Impossible<0>>'

It does look like a bug somewhere in clang.

Strangely, -Xclang -fno-recovery-ast-type doesn't make it go away - how sure are we that your patch is the culprit? And how sure are we that those flags work properly?

[sammccall]

Strangely, -Xclang -fno-recovery-ast-type doesn't make it go away - how sure are we that your patch is the culprit? And how sure are we that those flags work properly?

Turns out this is an existing clang bug, and my simplified testcase shows a variant that is broken with or without -frecovery-ast-type.

So it's good that libcxx happens not to be triggering the bug on this codepath at the moment, but it's mostly getting lucky.

ISTM the best path is to suppress this failure (like in this patch, but with a FIXME) and then try to fix the latent clang bug that was uncovered.
(These sort of FIXMEs are pretty common with secondary diagnostics and there's still a net improvement)

[ldionne]

In that case, please use // expected-error@memory:* 0+ {{no member named 'value' in}} and add a proper FIXME comment with a link to this review or similar. The 0+ will make sure the test passes whether or not Clang emits the secondary diagnostic.

[hokein]

add FIXME.

[ldionne]

Please add a link to https://reviews.llvm.org/D86685 and commit this.

[hokein]

In D86685#2242545, @ldionne wrote:

In that case, please use // expected-error@memory:* 0+ {{no member named 'value' in}} and add a proper FIXME comment with a link to this review or similar. The 0+ will make sure the test passes whether or not Clang emits the secondary diagnostic.

Thanks. Added one. I will followup to fix this bogus diagnostic in clang.

libcxx/test/libcxx/utilities/memory/util.smartptr/util.smartptr.shared/function_type_default_deleter.fail.cpp

 int main(int, char**) {
   {
     SPtr<0> s; // OK
     SPtr<1> s1(nullptr); // OK
     SPtr<2> s2(getFn<2>(), Deleter{}); // OK
     SPtr<3> s3(nullptr, Deleter{}); // OK
   }
   // expected-error-re@memory:* 2 {{static_assert failed{{.*}} "default_delete cannot be instantiated for function types"}}
+  // FIXME: suppress this bogus diagnostic, see https://reviews.llvm.org/D86685.
+  // expected-error@memory:* 0+ {{no member named 'value' in}}
   {
     SPtr<4> s4(getFn<4>()); // expected-note {{requested here}}
     SPtr<5> s5(getFn<5>(), std::default_delete<FnType<5>>{}); // expected-note {{requested here}}
   }
 
   return 0;
 }