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

Go: avoid interface calls for zero-cost validation of messages without rules #383

Closed
CAFxX opened this issue Aug 24, 2020 · 3 comments
Closed
Labels
Stale Activity has stalled on this issue/pull-request Wont Fix Out of scope of this project

Comments

@CAFxX
Copy link

CAFxX commented Aug 24, 2020

Currently the go generator generates something like this for embedded messages:

func (m *Foo) Validate() error {
	if m == nil {
		return nil
	}

	if v, ok := interface{}(m.GetBar()).(interface{ Validate() error }); ok {
		if err := v.Validate(); err != nil {
			return FooValidationError{
				field:  "Bar",
				reason: "embedded message failed validation",
				cause:  err,
			}
		}
	}

	// ...

	return nil
}

The problem is that, if Bar has no validation rules, all this code is pointless.

It would be ideal if instead we generated, at least in the case of messages that we know are in the same package, and that therefore are being generated together:

func (m *Foo) Validate() error {
	if m == nil {
		return nil
	}

	if err := m.GetBar().Validate(); err != nil {
		return FooValidationError{
			field:  "Bar",
			reason: "embedded message failed validation",
			cause:  err,
		}
	}

	// ...

	return nil
}

This way if (*Bar).Validate() is just return nil (i.e. there are no validation rules), the compiler can completely remove the whole if (or it can potentially inline the call, if there's a single rule).

@stale
Copy link

stale bot commented Oct 4, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Stale Activity has stalled on this issue/pull-request label Oct 4, 2020
@stale stale bot closed this as completed Oct 11, 2020
@elliotmjackson elliotmjackson reopened this Oct 5, 2022
@CAFxX
Copy link
Author

CAFxX commented Oct 25, 2022

Just pinging to signal that this issue is not stale, as it still applies.

@elliotmjackson elliotmjackson added the Wont Fix Out of scope of this project label Aug 10, 2023
@chrispine
Copy link
Contributor

As explained in the README, this project is in maintenance mode. We recommend upgrading to protovalidate.

I'm going to go ahead and close this issue, but feel free to open one against protovalidate if needed. Thanks!

@chrispine chrispine closed this as not planned Won't fix, can't repro, duplicate, stale Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Stale Activity has stalled on this issue/pull-request Wont Fix Out of scope of this project
Projects
None yet
Development

No branches or pull requests

3 participants