Skip to content

Clang incorrectly treats hidden friend function definitions as a member of the class for purposes of member access #57723

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

Open
strega-nil-ms opened this issue Sep 13, 2022 · 6 comments
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema"

Comments

@strega-nil-ms
Copy link

Hey all! Not sure if the title is correct/useful, but basically, given:

  • struct kitty
  • struct puppy, with a friend kitty declaration, and a private data member x
  • int get_x(puppy p), which is a hidden friend of kitty

get_x is allowed to access puppy::x, despite it not being a friend of puppy. Note that if get_x is defined out-of-line from kitty, it is not able to access puppy::x, and that MSVC and GCC correctly implement this.

Examples

The bug

godbolt link

struct kitty;

struct puppy {
    friend kitty;
private:
    int x;
};

struct kitty {
    friend int get_x(puppy p) {
        return p.x;
    }
};

Out-of-line definition of the friend function

godbolt link

struct kitty;

struct puppy {
    friend kitty;
private:
    int x;
};

struct kitty {
    friend int get_x(puppy p)
};
friend int get_x(puppy p) {
    return p.x;
}
@EugeneZelenko EugeneZelenko added clang:frontend Language frontend issues, e.g. anything involving "Sema" and removed new issue labels Sep 13, 2022
@llvmbot
Copy link
Member

llvmbot commented Sep 13, 2022

@llvm/issue-subscribers-clang-frontend

@shafik
Copy link
Collaborator

shafik commented Sep 13, 2022

It looks like the difference is in Sema::CheckLookupAccess(...) when it calls:

CheckAccess(*this, R.getNameLoc(), Entity);

The call to CheckEffectiveAccess(...) is saying accessible in the first case. I believe the issue is in how EffectiveContext is calculating the Records set.

@zygoloid
Copy link
Collaborator

I think last time we looked into this, it was very unclear which compiler was correct. Friendship normally means anything lexically within the befriended entity is granted access; excluding nested friends from that would be odd.

@shafik
Copy link
Collaborator

shafik commented Sep 13, 2022

I was basing this on [class.friend]p10 which says:

Friendship is neither inherited nor transitive.

but I am not confident of this take.

Is there a defect report? I could not find an obvious one that related to this. Otherwise if you think it is worth it I can ask core about this and see what they say.

@strega-nil-ms
Copy link
Author

strega-nil-ms commented Sep 14, 2022

@zygoloid I believe that [class.friend]p2 should tell us the correct interpretation:

Declaring a class to be a friend implies that private and protected members of the class granting friendship can be named in the base-specifiers and member declarations of the befriended class.

That paragraph does not include friend declarations (a friend declaration is neither a base-specifier nor a member declaration).

@shafik
Copy link
Collaborator

shafik commented Jun 21, 2023

I believe this ends up being DR 1699 which is now with EWG: cplusplus/papers#1573

Also see #44053

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema"
Projects
None yet
Development

No branches or pull requests

5 participants