Skip to content
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

fixes "double friend" issue #918

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fixes "double friend" issue #918

wants to merge 2 commits into from

Conversation

cjdb
Copy link

@cjdb cjdb commented May 17, 2023

C++ specifiers can go in almost any order, but the friend logic was overlooking this for some cases. In particular, constexpr friend wasn't being accounted for.

The following cases have been verified.

struct cat {
  /// Checks if the cats are the same.
  constexpr friend bool operator==(cat x, cat y)
  {
    return true;
  }

  /// Makes the cat cute.
  friend void make_cute(cat c)
  {
  }

  /// Makes the cat cuter.
  friend void make_cuter(cat c);

  /// Makes the cat the cutest.
  constexpr friend void make_cutest(cat c);
};

/// This won't be picked up.
constexpr void make_cutest(cat c)
{}

Fixes #916

C++ specifiers can go in almost any order, but the `friend` logic was
overlooking this for some cases. In particular, `constexpr friend`
wasn't being accounted for.

The following cases have been verified.

```cpp
struct cat {
  /// Checks if the cats are the same.
  constexpr friend bool operator==(cat x, cat y)
  {
    return true;
  }

  /// Makes the cat cute.
  friend void make_cute(cat c)
  {
  }

  /// Makes the cat cuter.
  friend void make_cuter(cat c);

  /// Makes the cat the cutest.
  constexpr friend void make_cutest(cat c);
};

/// This won't be picked up.
constexpr void make_cutest(cat c)
{}
```

Fixes breathe-doc#916
@cjdb
Copy link
Author

cjdb commented May 17, 2023

Is there a way to add unit tests? I haven't worked out how to do that for this project yet :(

@emilydolson
Copy link
Contributor

Since I have a vested interest in this getting merged (in that I would love for it to be fixed, and also for the fix for #917 to get merged), I messed around with the testing infrastructure and I wrote what I believe is a sufficient test for this fix. @cjdb, I'm sending it to you as a PR on the friend branch of your fork.

@emilydolson emilydolson mentioned this pull request Jun 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

inline constexpr friends inject extra keywords
2 participants