Skip to content

Commit 3cc9fcd

Browse files
katzdmkatzdm
committed
Fix some accessibility corner case bugs.
Anonymous structs, anonymous unions, unscoped enums. You know, all of the most important aspects of this wonderful language of ours.
1 parent 633d867 commit 3cc9fcd

File tree

3 files changed

+90
-24
lines changed

3 files changed

+90
-24
lines changed

‎clang/lib/AST/ExprConstantMeta.cpp

Lines changed: 15 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5628,39 +5628,34 @@ bool is_accessible(APValue &Result, ASTContext &C, MetaActions &Meta,
56285628
if (!Evaluator(RV, Args[0], true))
56295629
return true;
56305630

5631-
auto validate = [&](Decl *D) -> bool {
5632-
if (!D || !D->getDeclContext() || !isa<CXXRecordDecl>(D->getDeclContext()))
5633-
return DiagnoseReflectionKind(Diagnoser, Range, "a class member");
5634-
5635-
if (auto *Ctx = cast<CXXRecordDecl>(D->getDeclContext());
5636-
Ctx->isBeingDefined())
5631+
auto resolveNamingCls = [&](Decl *D, CXXRecordDecl *&NamingCls) -> bool {
5632+
if (!NamingCls)
5633+
NamingCls = dyn_cast<CXXRecordDecl>(D->getNonTransparentDeclContext());
5634+
if (NamingCls && NamingCls->isBeingDefined())
56375635
return Diagnoser(Range.getBegin(),
56385636
diag::metafn_access_query_class_being_defined)
5639-
<< Ctx << Range;
5640-
5637+
<< NamingCls << Range;
56415638
return false;
56425639
};
56435640

56445641
switch (RV.getReflectionKind()) {
56455642
case ReflectionKind::Type: {
56465643
NamedDecl *D = findTypeDecl(RV.getReflectedType());
5647-
if (validate(D))
5644+
if (resolveNamingCls(D, NamingCls))
56485645
return true;
5649-
5650-
if (!NamingCls)
5651-
NamingCls = cast<CXXRecordDecl>(D->getDeclContext());
5646+
else if (!NamingCls)
5647+
return SetAndSucceed(Result, makeBool(C, true));
56525648

56535649
bool Accessible = UnconditionalAccess ||
56545650
Meta.IsAccessible(D, AccessDC, NamingCls);
56555651
return SetAndSucceed(Result, makeBool(C, Accessible));
56565652
}
56575653
case ReflectionKind::Declaration: {
56585654
ValueDecl *D = RV.getReflectedDecl();
5659-
if (validate(D))
5655+
if (resolveNamingCls(D, NamingCls))
56605656
return true;
5661-
5662-
if (!NamingCls)
5663-
NamingCls = cast<CXXRecordDecl>(D->getDeclContext());
5657+
else if (!NamingCls)
5658+
return SetAndSucceed(Result, makeBool(C, true));
56645659

56655660
bool Accessible = UnconditionalAccess ||
56665661
Meta.IsAccessible(RV.getReflectedDecl(), AccessDC,
@@ -5669,11 +5664,10 @@ bool is_accessible(APValue &Result, ASTContext &C, MetaActions &Meta,
56695664
}
56705665
case ReflectionKind::Template: {
56715666
TemplateDecl *D = RV.getReflectedTemplate().getAsTemplateDecl();
5672-
if (validate(D))
5667+
if (resolveNamingCls(D, NamingCls))
56735668
return true;
5674-
5675-
if (!NamingCls)
5676-
NamingCls = cast<CXXRecordDecl>(D->getDeclContext());
5669+
else if (!NamingCls)
5670+
return SetAndSucceed(Result, makeBool(C, true));
56775671

56785672
bool Accessible = UnconditionalAccess ||
56795673
Meta.IsAccessible(D, AccessDC, NamingCls);
@@ -5710,8 +5704,7 @@ bool is_accessible(APValue &Result, ASTContext &C, MetaActions &Meta,
57105704
case ReflectionKind::Namespace:
57115705
case ReflectionKind::DataMemberSpec:
57125706
case ReflectionKind::Annotation:
5713-
return DiagnoseReflectionKind(Diagnoser, Range, "a class member",
5714-
DescriptionOf(RV));
5707+
return SetAndSucceed(Result, makeBool(C, true));
57155708
}
57165709
llvm_unreachable("invalid reflection type");
57175710
}

‎clang/lib/Sema/SemaReflect.cpp

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,12 +178,34 @@ class MetaActionsImpl : public MetaActions {
178178
bool IsAccessible(NamedDecl *Target, DeclContext *Ctx,
179179
CXXRecordDecl *NamingCls) override {
180180
bool Result = false;
181+
182+
// If 'Target' is a (possibly nested) anonymous struct/union or unscoped
183+
// enumerator, replace it with its parent recursively until it's no longer
184+
// such a member.
185+
while (Target && Target->getDeclContext() &&
186+
Target->getDeclContext() != NamingCls &&
187+
[](DeclContext *DC) {
188+
if (auto *RD = dyn_cast<CXXRecordDecl>(DC))
189+
return RD->isAnonymousStructOrUnion();
190+
else return DC->isTransparentContext();
191+
}(Target->getDeclContext()))
192+
if (isa<TranslationUnitDecl>(Target->getDeclContext()))
193+
// Can happen if Target was a member of a static anonymous union at
194+
// namespace scope.
195+
return true;
196+
else
197+
Target = cast<NamedDecl>(Target->getDeclContext());
198+
181199
if (auto *Cls = dyn_cast_or_null<CXXRecordDecl>(Target->getDeclContext())) {
182200
if (Cls != NamingCls &&
183201
!S.IsDerivedFrom(SourceLocation{},
184202
QualType(NamingCls->getTypeForDecl(), 0),
185203
QualType(Cls->getTypeForDecl(), 0)))
186204
return false;
205+
else if (NamingCls->isAnonymousStructOrUnion())
206+
// Clang's access-checking machinery isn't equipped to deal with checks
207+
// where the "naming" class (ha!) is anonymous - can't imagine why!
208+
return true;
187209

188210
DeclContext *PreviousDC = S.CurContext;
189211
{

‎libcxx/test/std/experimental/reflection/member-visibility.pass.cpp

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,10 +143,10 @@ struct Derived : Access {
143143
static_assert(is_accessible(^^Access::PublicBase::mem, ctx));
144144
static_assert(is_accessible(^^Access::ProtectedBase::mem, ctx));
145145
static_assert(is_accessible(^^::PrivateBase::mem, ctx));
146-
static_assert(!is_accessible(^^::PrivateBase::mem, obj_ctx));
147146
static_assert(!is_accessible(Access::r_prot, ctx));
148-
static_assert(is_accessible(Access::r_prot, obj_ctx));
149147
};
148+
static_assert(!is_accessible(^^::PrivateBase::mem, Derived::obj_ctx));
149+
static_assert(is_accessible(Access::r_prot, Derived::obj_ctx));
150150

151151
static constexpr auto gctx = access_context::current();
152152
static_assert(is_accessible(^^Access::pub, gctx));
@@ -288,4 +288,55 @@ static_assert(!is_private(std::meta::info{}));
288288
static_assert(!is_private(^^int));
289289
} // namespace queries
290290

291+
// ==============
292+
// unscoped_enums
293+
// ==============
294+
295+
namespace unscoped_enums {
296+
class C {
297+
enum { E };
298+
public:
299+
enum { F };
300+
static constexpr auto r = ^^E;
301+
};
302+
303+
static_assert(!is_accessible(C::r,
304+
std::meta::access_context::unprivileged()));
305+
static_assert(!is_accessible(C::r,
306+
std::meta::access_context::unprivileged().via(^^C)));
307+
308+
static_assert(is_accessible(^^C::F,
309+
std::meta::access_context::unprivileged()));
310+
static_assert(is_accessible(^^C::F,
311+
std::meta::access_context::unprivileged().via(^^C)));
312+
} // namespace unscoped_enums
313+
314+
// ========================
315+
// anonymous_structs_unions
316+
// ========================
317+
318+
namespace anonymous_structs_unions {
319+
class C {
320+
struct { struct { struct { int a; }; }; };
321+
public:
322+
union { union { union { int b; }; }; };
323+
static constexpr auto r_a = ^^a;
324+
};
325+
326+
static_assert(is_accessible(C::r_a,
327+
std::meta::access_context::unprivileged()));
328+
static_assert(!is_accessible(C::r_a,
329+
std::meta::access_context::unprivileged().via(^^C)));
330+
331+
static_assert(is_accessible(^^C::b,
332+
std::meta::access_context::unprivileged()));
333+
static_assert(is_accessible(^^C::b,
334+
std::meta::access_context::unprivileged().via(^^C)));
335+
336+
static union { int a; };
337+
338+
static_assert(is_accessible(^^a,
339+
std::meta::access_context::unprivileged()));
340+
} // namespace anonymous_structs_unions
341+
291342
int main() { }

0 commit comments

Comments
 (0)