-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[AST] Fix an assertion failure in TypeName::getFullyQualifiedName #159312
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
Conversation
This popped up during our internal intergrates of upstream changes. It started happening after ba9d1c4, which started using `TemplateSpecializationType` in this place and the code was not prepared to handle it.
@llvm/pr-subscribers-clang Author: Ilya Biryukov (ilya-biryukov) ChangesThis popped up during our internal integrates of upstream changes. It started happening after ba9d1c4, which started using Full diff: https://github.com/llvm/llvm-project/pull/159312.diff 3 Files Affected:
diff --git a/clang/lib/AST/QualTypeNames.cpp b/clang/lib/AST/QualTypeNames.cpp
index ee7fec3372fcf..b0e9a600f41a3 100644
--- a/clang/lib/AST/QualTypeNames.cpp
+++ b/clang/lib/AST/QualTypeNames.cpp
@@ -58,9 +58,9 @@ static bool getFullyQualifiedTemplateName(const ASTContext &Ctx,
NestedNameSpecifier NNS = std::nullopt;
TemplateDecl *ArgTDecl = TName.getAsTemplateDecl();
- // ArgTDecl won't be NULL because we asserted that this isn't a
- // dependent context very early in the call chain.
- assert(ArgTDecl != nullptr);
+ if (!ArgTDecl) // ArgTDecl can be null in dependent contexts.
+ return false;
+
QualifiedTemplateName *QTName = TName.getAsQualifiedTemplateName();
if (QTName &&
@@ -252,6 +252,9 @@ createNestedNameSpecifierForScopeOf(const ASTContext &Ctx, const Decl *Decl,
bool WithGlobalNsPrefix) {
assert(Decl);
+ // Some declaration cannot be qualified.
+ if (Decl->isTemplateParameter())
+ return std::nullopt;
const DeclContext *DC = Decl->getDeclContext()->getRedeclContext();
const auto *Outer = dyn_cast<NamedDecl>(DC);
const auto *OuterNS = dyn_cast<NamespaceDecl>(DC);
diff --git a/clang/unittests/AST/CMakeLists.txt b/clang/unittests/AST/CMakeLists.txt
index f27d34e8a0719..3a12a4a06a33a 100644
--- a/clang/unittests/AST/CMakeLists.txt
+++ b/clang/unittests/AST/CMakeLists.txt
@@ -26,6 +26,7 @@ add_clang_unittest(ASTTests
ExternalASTSourceTest.cpp
NamedDeclPrinterTest.cpp
ProfilingTest.cpp
+ QualTypeNamesTest.cpp
RandstructTest.cpp
RawCommentForDeclTest.cpp
RecursiveASTVisitorTest.cpp
diff --git a/clang/unittests/AST/QualTypeNamesTest.cpp b/clang/unittests/AST/QualTypeNamesTest.cpp
new file mode 100644
index 0000000000000..71e4c87cedc72
--- /dev/null
+++ b/clang/unittests/AST/QualTypeNamesTest.cpp
@@ -0,0 +1,55 @@
+//===- unittests/AST/QualTypeNamesTest.cpp --------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+//
+// This file contains tests for helpers from QualTypeNames.h.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/AST/QualTypeNames.h"
+#include "ASTPrint.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclarationName.h"
+#include "clang/AST/TypeBase.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace {
+
+TEST(QualTypeNamesTest, TemplateParameters) {
+ constexpr llvm::StringLiteral Code = R"cpp(
+ template <template<class> class T> struct Foo {
+ using type_of_interest = T<int>;
+ };
+ )cpp";
+ auto AST = tooling::buildASTFromCode(Code);
+ ASSERT_NE(AST, nullptr);
+
+ auto &Ctx = AST->getASTContext();
+ auto FooLR = Ctx.getTranslationUnitDecl()->lookup(
+ DeclarationName(AST->getPreprocessor().getIdentifierInfo("Foo")));
+ ASSERT_TRUE(FooLR.isSingleResult());
+
+ auto TypeLR =
+ llvm::cast<ClassTemplateDecl>(FooLR.front())
+ ->getTemplatedDecl()
+ ->lookup(DeclarationName(
+ AST->getPreprocessor().getIdentifierInfo("type_of_interest")));
+ ASSERT_TRUE(TypeLR.isSingleResult());
+
+ auto Type = cast<TypeAliasDecl>(TypeLR.front())->getUnderlyingType();
+ ASSERT_TRUE(isa<TemplateSpecializationType>(Type));
+
+ EXPECT_EQ(TypeName::getFullyQualifiedName(Type, Ctx, Ctx.getPrintingPolicy()), "T<int>");
+}
+
+} // namespace
+} // namespace clang
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
bea01fb
to
f62d7ec
Compare
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 Ilya!
bool WithGlobalNsPrefix) { | ||
assert(Decl); | ||
|
||
// Some declaration cannot be qualified. |
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.
This looks clearer to me:
// Some declaration cannot be qualified. | |
// Template parameters can't be qualified. |
Oh I was just about to review, but this looks fine, thanks! |
This popped up during our internal integrates of upstream changes. It started happening after ba9d1c4, which started using
TemplateSpecializationType
in this place and the code was not prepared to handle it.