Skip to content

compile-time assert numControllers == len(controllersArray) #3924

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.

Sign up for GitHub

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

Merged
knative-prow-robot merged 7 commits into from
May 29, 2019
Merged

compile-time assert numControllers == len(controllersArray) #3924

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
09134c8
compile-time assert numControllers == len(controllersArray)
syedriko Apr 27, 2019
8b5cb66
Merge branch 'master' into compile-time-assert
syedriko Apr 27, 2019
6dd42eb
Merge branch 'master' into compile_time_assert
syedriko Apr 28, 2019
4a01be3
Merge branch 'master' into compile_time_assert
syedriko Apr 29, 2019
5925a8d
Incorporated review feedback from @grantr
syedriko Apr 29, 2019
8db60f7
Merge branch 'master' into compile_time_assert
syedriko May 3, 2019
7ffdd89
Merge branch 'master' into compile_time_assert
syedriko May 3, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions cmd/controller/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func main() {

// Build all of our controllers, with the clients constructed above.
// Add new controllers to this array.
controllers := []*controller.Impl{
controllers := [...]*controller.Impl{
Copy link
Copy Markdown
Member

@dprotaso dprotaso May 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're only concerned about the upper bound then you could just have this line be[numControllers]*controller.Impl

Then you can drop the compile time assertion below - since the compiler will complain if this array has more that numController elements

ie. https://play.golang.org/p/Q3Coj5TecnI

Copy link
Copy Markdown
Contributor Author

@syedriko syedriko May 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it won't if there're less: knative/eventing#1112 (comment)

configuration.NewController(
opt,
configurationInformer,
Expand Down Expand Up @@ -147,9 +147,12 @@ func main() {
endpointsInformer,
),
}
if len(controllers) != numControllers {
logger.Fatalf("Number of controllers and QPS settings mismatch: %d != %d", len(controllers), numControllers)
}
// This line asserts at compile time that the length of controllers is equal to numControllers.
// It is based on https://go101.org/article/tips.html#assert-at-compile-time, which notes that
// var _ [N-M]int
// asserts at compile time that N >= M, which we can use to establish equality of N and M:
// (N >= M) && (M >= N) => (N == M)
var _ [numControllers - len(controllers)][len(controllers) - numControllers]int

// Watch the logging config map and dynamically update logging levels.
opt.ConfigMapWatcher.Watch(logging.ConfigMapName(), logging.UpdateLevelFromConfigMap(logger, atomicLevel, component))
Expand Down Expand Up @@ -181,7 +184,7 @@ func main() {

// Start all of the controllers.
logger.Info("Starting controllers.")
controller.StartAll(stopCh, controllers...)
controller.StartAll(stopCh, controllers[:]...)
}

func flush(logger *zap.SugaredLogger) {
Expand Down