-
Notifications
You must be signed in to change notification settings - Fork 10.8k
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][Sema] Allow access to a public template alias declaration that refers to friend's private nested type #83847
Conversation
@llvm/pr-subscribers-clang Author: Qizhi Hu (jcsxky) ChangesFull diff: https://github.com/llvm/llvm-project/pull/83847.diff 4 Files Affected:
diff --git a/clang/include/clang/Sema/Lookup.h b/clang/include/clang/Sema/Lookup.h
index 2f2f2607a937fe..a07609bfb7c9bc 100644
--- a/clang/include/clang/Sema/Lookup.h
+++ b/clang/include/clang/Sema/Lookup.h
@@ -752,6 +752,10 @@ class LookupResult {
IDNS &= ~Decl::IDNS_LocalExtern;
}
+ void setNestedNameSpecifier(NestedNameSpecifier *Q) { Qualifier = Q; }
+
+ NestedNameSpecifier *getNestedNameSpecifier() const { return Qualifier; }
+
private:
void diagnoseAccess() {
if (!isAmbiguous() && isClassLookup() &&
@@ -792,6 +796,7 @@ class LookupResult {
CXXBasePaths *Paths = nullptr;
CXXRecordDecl *NamingClass = nullptr;
QualType BaseObjectType;
+ NestedNameSpecifier *Qualifier = nullptr;
// Parameters.
Sema *SemaPtr;
diff --git a/clang/lib/Sema/SemaAccess.cpp b/clang/lib/Sema/SemaAccess.cpp
index 4af3c0f30a8e8a..9077066da29d61 100644
--- a/clang/lib/Sema/SemaAccess.cpp
+++ b/clang/lib/Sema/SemaAccess.cpp
@@ -254,6 +254,9 @@ struct AccessTarget : public AccessedEntity {
namingClass = cast<CXXRecordDecl>(namingClass->getParent());
return namingClass->getCanonicalDecl();
}
+ void setNestedNameSpecifier(NestedNameSpecifier *Q) { Qualifier = Q; }
+
+ NestedNameSpecifier *getNestedNameSpecifier() const { return Qualifier; }
private:
void initialize() {
@@ -274,6 +277,7 @@ struct AccessTarget : public AccessedEntity {
mutable bool CalculatedInstanceContext : 1;
mutable const CXXRecordDecl *InstanceContext;
const CXXRecordDecl *DeclaringClass;
+ NestedNameSpecifier *Qualifier = nullptr;
};
}
@@ -1481,6 +1485,20 @@ static Sema::AccessResult CheckAccess(Sema &S, SourceLocation Loc,
}
EffectiveContext EC(S.CurContext);
+ if (Entity.getNestedNameSpecifier())
+ if (const Type *Ty = Entity.getNestedNameSpecifier()->getAsType())
+ switch (Ty->getTypeClass()) {
+ case Type::TypeClass::SubstTemplateTypeParm:
+ if (auto *SubstTemplateTy = Ty->getAs<SubstTemplateTypeParmType>())
+ if (auto *D = SubstTemplateTy->getAssociatedDecl())
+ if (auto *RD = dyn_cast_or_null<CXXRecordDecl>(D->getDeclContext()))
+ EC.Records.push_back(RD);
+
+ break;
+ default:
+ break;
+ }
+
switch (CheckEffectiveAccess(S, EC, Loc, Entity)) {
case AR_accessible: return Sema::AR_accessible;
case AR_inaccessible: return Sema::AR_inaccessible;
@@ -1916,6 +1934,7 @@ void Sema::CheckLookupAccess(const LookupResult &R) {
AccessTarget Entity(Context, AccessedEntity::Member,
R.getNamingClass(), I.getPair(),
R.getBaseObjectType());
+ Entity.setNestedNameSpecifier(R.getNestedNameSpecifier());
Entity.setDiag(diag::err_access);
CheckAccess(*this, R.getNameLoc(), Entity);
}
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index a7910bda874c8d..d1555cfeb03e15 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -11275,6 +11275,7 @@ Sema::CheckTypenameType(ElaboratedTypeKeyword Keyword,
DeclarationName Name(&II);
LookupResult Result(*this, Name, IILoc, LookupOrdinaryName);
+ Result.setNestedNameSpecifier(QualifierLoc.getNestedNameSpecifier());
if (Ctx)
LookupQualifiedName(Result, Ctx, SS);
else
diff --git a/clang/test/SemaTemplate/PR25708.cpp b/clang/test/SemaTemplate/PR25708.cpp
new file mode 100644
index 00000000000000..e9e96ebd015e04
--- /dev/null
+++ b/clang/test/SemaTemplate/PR25708.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -std=c++11 -verify %s
+// RUN: %clang_cc1 -std=c++14 -verify %s
+// RUN: %clang_cc1 -std=c++17 -verify %s
+// RUN: %clang_cc1 -std=c++20 -verify=cxx20 %s
+// expected-no-diagnostics
+
+struct FooAccessor
+{
+ template <typename T>
+ using Foo = typename T::Foo;
+};
+
+class Type
+{
+ friend struct FooAccessor;
+
+ using Foo = int;
+};
+
+int main()
+{
+ FooAccessor::Foo<Type> t;
+}
\ No newline at end of file
|
99b66fb
to
712213e
Compare
76c7e3e
to
8541506
Compare
gently ping~ |
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.
1 nit, else LGTM.
clang/docs/ReleaseNotes.rst
Outdated
@@ -409,7 +413,7 @@ RISC-V Support | |||
CUDA/HIP Language Changes | |||
^^^^^^^^^^^^^^^^^^^^^^^^^ | |||
|
|||
- PTX is no longer included by default when compiling for CUDA. Using | |||
- PTX is no longer included by default when compiling for CUDA. Using |
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.
Unrelated change, please revert.
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.
Fixed.
clang/lib/Sema/SemaTemplate.cpp
Outdated
@@ -4342,10 +4342,17 @@ QualType Sema::CheckTemplateIdType(TemplateName Name, | |||
InstantiatingTemplate Inst(*this, TemplateLoc, Template); | |||
if (Inst.isInvalid()) | |||
return QualType(); | |||
if (!AliasTemplate->getDeclContext()->isFileContext()) { | |||
ContextRAII SavedContext(*this, AliasTemplate->getDeclContext()); |
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.
nit: This can be simplified using std::optional
:
std::optional<ContextRAII> C;
if (!A.getDeclContext()->isFileContext())
C.emplace(*this, A->getDeclContext());
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.
nit: This can be simplified using
std::optional
:std::optional<ContextRAII> C; if (!A.getDeclContext()->isFileContext()) C.emplace(*this, A->getDeclContext());
Neat trick, I like it!
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.
Looks more clear. Thanks for your guidance!
clang/test/SemaTemplate/PR25708.cpp
Outdated
// RUN: %clang_cc1 -std=c++11 -verify %s | ||
// RUN: %clang_cc1 -std=c++14 -verify %s | ||
// RUN: %clang_cc1 -std=c++17 -verify %s | ||
// RUN: %clang_cc1 -std=c++20 -verify %s |
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.
Do we really need so many runs? I wonder if only -std=c++11
is sufficient.
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.
Fixed.
clang/docs/ReleaseNotes.rst
Outdated
@@ -259,6 +259,10 @@ Bug Fixes in This Version | |||
operator. | |||
Fixes (#GH83267). | |||
|
|||
- Allow access to a public template alias declaration that refers to friend's |
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.
I suspect this should have been placed under the group Bug Fixes to C++ Support
...?
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.
Updated and put it to the right place.
…t refers to friend's private nested type
This patch attempts to fix #25708
Current access check missed qualifier(
NestedNameSpecifier
) in friend class checking. Add it toRecords
ofEffectiveContext
by changing theDeclContext
makesMatchesFriend
work.