[Sema] Proposed fix for PR 21837

[davide]

Summary

https://llvm.org/bugs/show_bug.cgi?id=21837
I hit this recently and decided to investigate a little bit.
I'm still not 100% sure if we can arrive here with Access == AS_none, though.

[davide]

FYI, Will add the testcase in the PR if this reviews actually goes anywhere.

[davide]

Gentle ping.

[rsmith]

SemaAccess.cpp

1361–1362

Please add an assert that the Target is marked invalid here. I would also think that assuming invalid declarations are accessible is a better default; we don't want additional follow-on errors after the initial diagnostic that resulted in the entity being marked as invalid.

(And this comment also applies to the other change.)

[davide]

Hrmm, the assertion you suggested to add (Target is marked invalid) fails, so I think this requires a different fix. I'll try to investigate futher over the next days.

SemaAccess.cpp

   AccessSpecifier accessSoFar = AS_public;
 
   // Check whether we have special rights to the declaring class.
   if (entity.isMemberAccess()) {
     NamedDecl *D = entity.getTargetDecl();
     accessSoFar = D->getAccess();
     const CXXRecordDecl *declaringClass = entity.getDeclaringClass();
 
+    if (accessSoFar == AS_none)
+      return diagnoseBadDirectAccess(S, EC, entity);
+
     switch (HasAccess(S, EC, declaringClass, accessSoFar, entity)) {
     // If the declaration is accessible when named in its declaring
     // class, then we must be constrained by the path.
     case AR_accessible:
       accessSoFar = AS_public;
       entity.suppressInstanceContext();
       break;
 

 
   if (Entity.isMemberAccess()) {
     // Determine if the declaration is accessible from EC when named
     // in its declaring class.
     NamedDecl *Target = Entity.getTargetDecl();
     const CXXRecordDecl *DeclaringClass = Entity.getDeclaringClass();
 
     FinalAccess = Target->getAccess();
+
+    if (FinalAccess == AS_none)
+      return AR_inaccessible;
+
     switch (HasAccess(S, EC, DeclaringClass, FinalAccess, Entity)) {
     case AR_accessible:
       // Target is accessible at EC when named in its declaring class.
       // We can now hill-climb and simply check whether the declaring
       // class is accessible as a base of the naming class.  This is
       // equivalent to checking the access of a notional public
       // member with no instance context.
       FinalAccess = AS_public;