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

Transformations around noexcept are minorly confusing #251

Closed
Quuxplusone opened this issue Oct 8, 2019 · 2 comments
Closed

Transformations around noexcept are minorly confusing #251

Quuxplusone opened this issue Oct 8, 2019 · 2 comments
Labels
enhancement New feature or request

Comments

@Quuxplusone
Copy link

Based on the first example from this blog post:
https://cppinsights.io/s/dfd1da7c

class Foo
{
};
static_assert(noexcept(Foo( Foo() )), "");
static_assert(not noexcept( new Foo() ), "");

int main()
{
  Foo f;
}

produces

  // inline constexpr Foo() noexcept = default;
  // inline constexpr Foo(const Foo &) = default;
  // inline constexpr Foo(Foo &&) = default;
  // inline ~Foo() noexcept = default;

This is technically correct, but surprising, because you redundantly place noexcept on the default constructor and on the destructor, but you don't redundantly place noexcept on the other two special members (even though those members also are implicitly noexcept).

And then the static_asserts are transformed into

/* PASSED: static_assert(noexcept(true) , ""); */

/* PASSED: static_assert(!noexcept(false) , ""); */

I think this is weird, first because noexcept(true) seems less informative than the original code, and second because the "lowered" line

static_assert(!noexcept(false) , "");

would fail if you put it into a C++ source file. (The expression false is in fact non-throwing, so noexcept(false) is true.) So this particular C++ code is being lowered into not-really-C++ pseudocode. I think this is likely to be more confusing than helpful.

@andreasfertig
Copy link
Owner

Hello @Quuxplusone,

thank you for bringing these two issues up.

This is technically correct, but surprising, because you redundantly place noexcept on the default constructor and on the destructor, but you don't redundantly place noexcept on the other two special members (even though those members also are implicitly noexcept).

Well, at this point it is the result of what Clang tells the implementation. There is no special case for the different special members. In your example only the default constructor is used and with that only its noexcept is evaluated by Clang. You can see it in the AST output as well godbolt. It changes if, for example the copy constructor is invoked:

#include <utility>

class Foo
{
  public:
  Foo() = default;
};
static_assert(noexcept(Foo( Foo() )), "");
static_assert(not noexcept( new Foo() ), "");

int main()
{
  Foo f;
  Foo ff = f;
  Foo fff = std::move(f);
}

While I can see that it is confusing, at this point it is what Clang knows. One solution that comes to my mind is to completely hide unused special members. So remove everything except the default constructor in your example. Would that be less confusing for you?

This is technically correct, but surprising, because you redundantly place noexcept on the...

I'm not sure what redundantly means here for you. Because it is compiler provided it is always noexcept and hence redundant? Consider this modified example C++ Insights

class Foo
{
  public:
  Foo() = default;
};

int main()
{
  Foo f;
}

Here as a user I did not write noexecpt but the compiler makes it noexcept for me. I consider this helpful for starters.

And then the static_asserts are transformed into

I agree that putting the static_assert in a comment is probably a bad idea. I was thinking about leaving it as it is there for some time. I can also look into retaining the original expression.

Feel free to send a PRs for all the things.

Andreas

@Quuxplusone
Copy link
Author

This is technically correct, but surprising, because you redundantly place noexcept on the...

I'm not sure what redundantly means here for you. Because it is compiler provided it is always noexcept and hence redundant?

Yes. Basically I saw two possible ways CppInsights might be choosing its output in this case: either it would say "this ctor is noexcept, so I will mark it noexcept in the output (i.e. I always produce output with noexcept-specifications even if they're redundant)," or it would say "this ctor is noexcept, but that's actually the default anyway, so I don't need to write noexcept redundantly in the output (i.e. I always produce output that is valid C++ code, but I may omit certain redundant bits)." So I was (wrongly) complaining that CppInsights seemed to choose inconsistently between these two points of view depending on whether it was a default ctor or a move-ctor. I had not realized that CppInsights' output would actually change depending on whether the ctor was called or not.

This example clearly shows what's going on internally: https://cppinsights.io/s/d95de9d0
A's copy ctor is marked noexcept (but its move ctor isn't). B's move ctor is marked noexcept (but its copy ctor isn't). Yet their class definitions (in the left-hand pane) are identical.

So at least I see where the noexcept-specifications are coming from now. Seems tricky to fix.

@andreasfertig andreasfertig added the enhancement New feature or request label Oct 27, 2019
andreasfertig added a commit that referenced this issue Oct 27, 2019
Fixed #251: Suppress unused compiler generated special members.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants