Skip to content

Commit 4ffd254

Browse files
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 4ffd254

File tree

3 files changed

+89
-21
lines changed

3 files changed

+89
-21
lines changed

clang/lib/AST/ExprConstantMeta.cpp

Lines changed: 16 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -5628,39 +5628,36 @@ 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");
5631+
auto validate = [&](Decl *D, CXXRecordDecl *&NamingCls) -> bool {
5632+
auto *DC = dyn_cast<CXXRecordDecl>(D->getNonTransparentDeclContext());
5633+
if (!NamingCls)
5634+
NamingCls = DC;
56345635

5635-
if (auto *Ctx = cast<CXXRecordDecl>(D->getDeclContext());
5636-
Ctx->isBeingDefined())
5636+
if (DC && DC->isBeingDefined())
56375637
return Diagnoser(Range.getBegin(),
56385638
diag::metafn_access_query_class_being_defined)
5639-
<< Ctx << Range;
5640-
5639+
<< DC << Range;
56415640
return false;
56425641
};
56435642

56445643
switch (RV.getReflectionKind()) {
56455644
case ReflectionKind::Type: {
56465645
NamedDecl *D = findTypeDecl(RV.getReflectedType());
5647-
if (validate(D))
5646+
if (validate(D, NamingCls))
56485647
return true;
5649-
5650-
if (!NamingCls)
5651-
NamingCls = cast<CXXRecordDecl>(D->getDeclContext());
5648+
else if (!NamingCls)
5649+
return SetAndSucceed(Result, makeBool(C, true));
56525650

56535651
bool Accessible = UnconditionalAccess ||
56545652
Meta.IsAccessible(D, AccessDC, NamingCls);
56555653
return SetAndSucceed(Result, makeBool(C, Accessible));
56565654
}
56575655
case ReflectionKind::Declaration: {
56585656
ValueDecl *D = RV.getReflectedDecl();
5659-
if (validate(D))
5657+
if (validate(D, NamingCls))
56605658
return true;
5661-
5662-
if (!NamingCls)
5663-
NamingCls = cast<CXXRecordDecl>(D->getDeclContext());
5659+
else if (!NamingCls)
5660+
return SetAndSucceed(Result, makeBool(C, true));
56645661

56655662
bool Accessible = UnconditionalAccess ||
56665663
Meta.IsAccessible(RV.getReflectedDecl(), AccessDC,
@@ -5669,11 +5666,10 @@ bool is_accessible(APValue &Result, ASTContext &C, MetaActions &Meta,
56695666
}
56705667
case ReflectionKind::Template: {
56715668
TemplateDecl *D = RV.getReflectedTemplate().getAsTemplateDecl();
5672-
if (validate(D))
5669+
if (validate(D, NamingCls))
56735670
return true;
5674-
5675-
if (!NamingCls)
5676-
NamingCls = cast<CXXRecordDecl>(D->getDeclContext());
5671+
else if (!NamingCls)
5672+
return SetAndSucceed(Result, makeBool(C, true));
56775673

56785674
bool Accessible = UnconditionalAccess ||
56795675
Meta.IsAccessible(D, AccessDC, NamingCls);
@@ -5710,8 +5706,7 @@ bool is_accessible(APValue &Result, ASTContext &C, MetaActions &Meta,
57105706
case ReflectionKind::Namespace:
57115707
case ReflectionKind::DataMemberSpec:
57125708
case ReflectionKind::Annotation:
5713-
return DiagnoseReflectionKind(Diagnoser, Range, "a class member",
5714-
DescriptionOf(RV));
5709+
return SetAndSucceed(Result, makeBool(C, true));
57155710
}
57165711
llvm_unreachable("invalid reflection type");
57175712
}

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: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -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)