rjmccall added a comment. (I didn't mean to submit the last comment before I finished the review, sorry)
It looks like you've chosen to treat this as a DR that we should apply under all standards. Have you investigated whether it causes compatibility problems? It does look like it'd be painful to try to support both rules. ================ Comment at: clang/lib/Sema/SemaAccess.cpp:891 + // Note that declaresSameEntity doesn't correctly determine whether + // two type declarations declare the same type entity. + if ((!Best || Best->getAccess() > Corresponding->getAccess()) && ---------------- Is it okay to check them in sequence like this, or do we need to check in advance which case to use? ================ Comment at: clang/lib/Sema/SemaAccess.cpp:983 + } + return false; + }; ---------------- Is this not doing a *lot* of extra work? I suppose this is only in the slow path where we weren't able to immediately recognize that the found decl is public or we're in a scope with obviously adequate access? ================ Comment at: clang/lib/Sema/SemaLookup.cpp:2296 + } + } + ---------------- I wonder under this new model if we can record that we saw something along ambiguous paths and avoid a lot of work in access-checking in the common case where we don't? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94987/new/ https://reviews.llvm.org/D94987 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
