GabrielDosReis/ipr

ipr::Region::enclosing() will always be null for Global_scope

Closed this issue · 4 comments

This creates complications when walking up the region enclosing scopes. This will result in a crash unless the user explicitly tests the Region::owner() field of each region for Global_scope which is clunky.

This is made worse by the fact that Global_scope has no category so requires a dynamic_cast<> to check.

The expectation is that traversal of the enclosing scopes should check for when they reach the global scope - logically necessary, if you have to check for something - and here the expectation is to check for null enclosing scope for the global scope.

That makes Region::enclsoing() the only function in the IPR that callability requires validating a precondition. Assuming a completely populated/valid IPR tree all other preconditions are baked into the interface using Optional where needed.

Without at least some form of contract (even in comments) this feels like tribal knowledge that is sure to trip up users of the IPR with runtime errors.

Without at least some form of contract (even in comments) this feels like tribal knowledge that is sure to trip up users of the IPR with runtime errors.

Good point.

Reopening for further considerations of resolutions.

I thought more about this. Instead of making enclosing() return Optional, I believe a better compromise is to add a predicate to Region that tests whether the region is global. Yes, that still requires you to test a separate predicate, but that properly reflects the singularity of the global region: every single region that is not the global region has a parent, and by far all regions (except one) are not global. When examining the region of a for loop or a class scope, do you really have to test first if that region is global when obviously it is not? This is very different from say declarations where the distribution of definitions vs. non-defining declarations is way less than 99.999%. This compromise pushes the complexity where it belongs: you have to test for the predicate only in the situations where you're unsure that you're in a global scope. Furthermore, the crash is actually an exception that is thrown and never caught.