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

Added more functions for introspecting pads of a component #182

Merged
merged 1 commit into from
Oct 2, 2024

Conversation

callendorph
Copy link
Contributor

These tools are for constructing keepouts around components or parts of components, typically from a wrapping module.

@callendorph callendorph requested review from bhusang and tjknoth October 2, 2024 15:26
@callendorph
Copy link
Contributor Author

I need to more forward to meet deadlines. I'm going to merge. Please let me know if there is anything I need to address in a follow on PR before a release.

@callendorph callendorph merged commit 2f170b0 into main Oct 2, 2024
1 check passed
<DOC>
public defn to-lp-pad-seq (objs:Seq<JITXObject>) -> Seq<LandPatternPad> :
for obj in objs seq:
obj as LandPatternPad
Copy link
Contributor

Choose a reason for hiding this comment

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

Only possible change is to match and throw an exception instead of cast -- slightly scary to expose a blind cast via public API

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK - perhaps I'm being too cavalier with the as operator. Will that fatal if it doesn't match correct ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm pretty sure it will fatal, and I think weird stuff can happen in optimized mode

Honestly this function seems a little specific for jsl; it feels more like user code. We could actually provide a more general version of this function in jsl instead.
There are two options:

downcast-objs<T> (objs:Seq<JITXObject>) -> Seq<T>, which matches all objects in the sequence against the explicit type parameter

downcast-objs<?T> (objs:Seq<JITXObject&?T> -> Seq<T> which infers the subtype of JITXObject -- but might not always infer the strongest type T if you give it a heterogeneous collection

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ya - so I think this function should be in JITX not JSL if anywhere.
In this particular case, I'm confident that this JITXObject is a LandPatternPad. I could put this code in the design but it is going to be duplicated everywhere I use this pattern.
The other option is to change the bounds function to accept a JITXObject but that really just moves the problem. When bounds tries to inspect a LandPatternPad and it doesn't find one - then an error will be raised there. I think the jsl/landpatterns/leads/bounds function was probably written when I was trying for typing - but perhaps I need to re-evaluate that instead of adding this function.

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.

2 participants