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

odb: In dbITerm::getGeometries, return layer + rect #6049

Merged
merged 1 commit into from
Nov 23, 2024

Conversation

smunaut
Copy link
Contributor

@smunaut smunaut commented Oct 29, 2024

Currently only the rect is returned but that's not very useful. So instead we return pairs of layer + rect.

The appropriate SWIG mapping is also added so the result is usable both in Python and in TCL.

Note that someone should definitely have a close look at the SWIG mapping because I have very little experience with those.
I did test that it seems to work as expected in both TCL and Python though.

@gadfort
Copy link
Collaborator

gadfort commented Oct 29, 2024

Could this return a set of dbShape instead? https://github.com/The-OpenROAD-Project/OpenROAD/blob/master/src/odb/include/odb/dbShape.h

@maliberty
Copy link
Member

Looking your suggestion I noticed dbITermShapeItr which might be what is wanted here.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

src/odb/src/db/dbITerm.cpp Outdated Show resolved Hide resolved
@maliberty
Copy link
Member

@smunaut this has compilation and clang-tidy errors? Do you plan to fix this up?

@smunaut
Copy link
Contributor Author

smunaut commented Nov 19, 2024

clang-tidy was just a suggested, I've applied it.

The compilation errors ... they're just warning turned to errors but
(1) I don't have those locally
(2) I have no clue whatsoever what is wrong and what the compiler wants for me. As far as I can tell I'm doing the EXACT same thing as the swig binding just above for another type and this one doesn't trigger it ... so I'd need a C++ or SWIG expert to look at it, because I don't understand what it thinks is wrong.

Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@smunaut
Copy link
Contributor Author

smunaut commented Nov 19, 2024

Ok, I think I fixed it actually. For some reason it says pr-hread failed but if you click on it, all build succeeded. I think it got a bit confused because I pushed a new version before the previous build was done ?

@maliberty
Copy link
Member

You need to merge master to resolve the c++20 compiler issue.

Currently only the rect is returned but that's not very
useful. So instead we return pairs of layer + rect.

The appropriate SWIG mapping is also added so the result
is usable both in Python and in TCL.

Signed-off-by: Sylvain Munaut <[email protected]>
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@smunaut
Copy link
Contributor Author

smunaut commented Nov 23, 2024

Rebased to master, all checks passed.

@maliberty maliberty merged commit 35d3dd3 into The-OpenROAD-Project:master Nov 23, 2024
11 checks passed
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.

3 participants