Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang-tidy] Fix failing test after #80864 #81171

Merged
merged 1 commit into from
Feb 8, 2024

Conversation

sdkrystian
Copy link
Member

The following test case in clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp is failing:

#ifdef PR64602 // Should not crash
template <class T = void>
struct S
{
    auto foo(auto);
};

template <>
auto S<>::foo(auto)
{
    return 1;
}
// CHECK8: error: template parameter list matching the non-templated nested type 'S<>' should be empty ('template<>') [clang-diagnostic-error]
#endif

#80864 fixes a bug where we would (incorrectly) append invented template parameters to empty template parameter lists, which causes this test to fail.

@sdkrystian sdkrystian changed the title [clang-tidy] Fix failing test after 17f0680f69f44d340fd0205f7763b2830357c0d5 [clang-tidy] Fix failing test after #80864 Feb 8, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 8, 2024

@llvm/pr-subscribers-clang-tidy

@llvm/pr-subscribers-clang-tools-extra

Author: Krystian Stasiowski (sdkrystian)

Changes

The following test case in clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp is failing:

#ifdef PR64602 // Should not crash
template &lt;class T = void&gt;
struct S
{
    auto foo(auto);
};

template &lt;&gt;
auto S&lt;&gt;::foo(auto)
{
    return 1;
}
// CHECK8: error: template parameter list matching the non-templated nested type 'S&lt;&gt;' should be empty ('template&lt;&gt;') [clang-diagnostic-error]
#endif

#80864 fixes a bug where we would (incorrectly) append invented template parameters to empty template parameter lists, which causes this test to fail.


Full diff: https://github.com/llvm/llvm-project/pull/81171.diff

1 Files Affected:

  • (modified) clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp (+2-1)
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
index 547f634a101c58..d0efc5ca763753 100644
--- a/clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
@@ -68,5 +68,6 @@ auto S<>::foo(auto)
 {
     return 1;
 }
-// CHECK8: error: template parameter list matching the non-templated nested type 'S<>' should be empty ('template<>') [clang-diagnostic-error]
+// CHECK8: error: conflicting types for 'foo' [clang-diagnostic-error]
+// CHECK8: note: previous declaration is here
 #endif

Copy link
Collaborator

@erichkeane erichkeane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems right to me, though a 2nd reviewer would be nice. I don't think we should wait for CI on this due to it fixing a bot-breakage.

Copy link
Member

@PiotrZSL PiotrZSL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, purpose of this test is just to verify that errors (any) are shown in clang-tidy

@sdkrystian
Copy link
Member Author

@erichkeane I'm building the check-clang-extra target locally just to make sure that the test passes... should be done in ~5 mins

@sdkrystian sdkrystian merged commit a56fa16 into llvm:main Feb 8, 2024
3 of 4 checks passed
@sdkrystian sdkrystian deleted the fix-clang-tidy-test branch February 26, 2024 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants