-
Notifications
You must be signed in to change notification settings - Fork 11.9k
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
[AST] Print the separator "," for template arguments in ConceptReference::print #91750
Conversation
ConceptReference::print
@llvm/pr-subscribers-clang Author: Haojian Wu (hokein) ChangesFull diff: https://github.com/llvm/llvm-project/pull/91750.diff 3 Files Affected:
diff --git a/clang/lib/AST/ASTConcept.cpp b/clang/lib/AST/ASTConcept.cpp
index b3ec99448b3e1..6bef68fc87eea 100644
--- a/clang/lib/AST/ASTConcept.cpp
+++ b/clang/lib/AST/ASTConcept.cpp
@@ -106,9 +106,15 @@ void ConceptReference::print(llvm::raw_ostream &OS,
ConceptName.printName(OS, Policy);
if (hasExplicitTemplateArgs()) {
OS << "<";
+ bool First = true;
// FIXME: Find corresponding parameter for argument
- for (auto &ArgLoc : ArgsAsWritten->arguments())
+ for (auto &ArgLoc : ArgsAsWritten->arguments()) {
+ if (First)
+ First = false;
+ else
+ OS << ", ";
ArgLoc.getArgument().print(Policy, OS, /*IncludeType*/ false);
+ }
OS << ">";
}
}
diff --git a/clang/unittests/AST/CMakeLists.txt b/clang/unittests/AST/CMakeLists.txt
index 54765e36db008..29d2b39cff8b1 100644
--- a/clang/unittests/AST/CMakeLists.txt
+++ b/clang/unittests/AST/CMakeLists.txt
@@ -24,6 +24,7 @@ add_clang_unittest(ASTTests
CommentLexer.cpp
CommentParser.cpp
CommentTextTest.cpp
+ ConceptPrinterTest.cpp
DataCollectionTest.cpp
DeclPrinterTest.cpp
DeclTest.cpp
diff --git a/clang/unittests/AST/ConceptPrinterTest.cpp b/clang/unittests/AST/ConceptPrinterTest.cpp
new file mode 100644
index 0000000000000..bd1f6bbfa5658
--- /dev/null
+++ b/clang/unittests/AST/ConceptPrinterTest.cpp
@@ -0,0 +1,57 @@
+//===- unittests/AST/ConceptPrinterTest.cpp --- Concept printer tests -----===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "ASTPrint.h"
+#include "clang/AST/ASTConcept.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/ExprConcepts.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/SmallString.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace ast_matchers;
+using namespace tooling;
+
+namespace {
+
+static void PrintConceptReference(raw_ostream &Out, const ASTContext *Context,
+ const ConceptSpecializationExpr *T,
+ PrintingPolicyAdjuster PolicyAdjuster) {
+ assert(T && T->getConceptReference() &&
+ "Expected non-null concept reference");
+
+ PrintingPolicy Policy = Context->getPrintingPolicy();
+ if (PolicyAdjuster)
+ PolicyAdjuster(Policy);
+ T->getConceptReference()->print(Out, Policy);
+}
+
+::testing::AssertionResult
+PrintedConceptMatches(StringRef Code, const std::vector<std::string> &Args,
+ const StatementMatcher &NodeMatch,
+ StringRef ExpectedPrinted) {
+ return PrintedNodeMatches<ConceptSpecializationExpr>(
+ Code, Args, NodeMatch, ExpectedPrinted, "", PrintConceptReference);
+}
+const internal::VariadicDynCastAllOfMatcher<Stmt, ConceptSpecializationExpr>
+ conceptSpecializationExpr;
+} // unnamed namespace
+
+TEST(ConceptPrinter, ConceptReference) {
+ std::string Code = R"cpp(
+ template <typename, typename> concept D = true;
+ template<typename T, typename U>
+ requires D<T, U>
+ void g(T);
+ )cpp";
+ auto Matcher = conceptSpecializationExpr().bind("id");
+
+ ASSERT_TRUE(PrintedConceptMatches(Code, {"-std=c++20"}, Matcher, "D<T, U>"));
+}
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I'm surprised that this part was missing previously...
@@ -24,6 +24,7 @@ add_clang_unittest(ASTTests | |||
CommentLexer.cpp | |||
CommentParser.cpp | |||
CommentTextTest.cpp | |||
ConceptPrinterTest.cpp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: we probably want to update the gn build as well: https://github.com/llvm/llvm-project/tree/main/llvm/utils/gn
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done (we're not required to update gn build files, and there's a bot that can automate the porting).
clang/lib/AST/ASTConcept.cpp
Outdated
for (auto &ArgLoc : ArgsAsWritten->arguments()) { | ||
if (First) | ||
First = false; | ||
else | ||
OS << ", "; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you use llvm::ListSeparator
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I wasn't aware we have such a helpful utility.
No description provided.