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

Updated test cases #2439

Draft
wants to merge 21 commits into
base: master
Choose a base branch
from
Draft

Conversation

Paebbels
Copy link
Member

@Paebbels Paebbels commented May 14, 2023

New Features

  • Added binding for files_map__find_source_file as files_map.Find_Source_File().
  • Added CLI option --library <LIB> to demo application for pyGHDL.dom.
    • First level of subdirectory names are used as VHDL library names.
    • Files on the root directory itself are compiled into the library named by --library.

Updates

  • Check if a source file was already loaded and throw an exception.
  • Improved exceptions when opening a source file.
  • Bumped dependencies e.g. for pyTooling to ~=6.1.
    • Use updated TerminalUI package.
    • Use updated Attributes package.
    • Removed dependency to pyAttributes as it's now part of pyTooling.
  • Adjustments for pyVHDLModel v0.28.0
  • Added more docstrings.

Bug Fixes

  • Fixed usage of symbols in unit tests and pretty-printing.

@Paebbels
Copy link
Member Author

@tgingold can you have a look into one of the testcases. It causes a raised SYSTEM.ASSERTIONS.ASSERT_FAILURE : files_map.adb:591.

Depending on the environment in CI, the error looks a bit different. My local system (Native Python 3.11 + mcode DLL build with MinGW64) shows this: pyGHDL.libghdl.LibGHDLException: Caught exception when calling 'files_map__reserve_source_file' in libghdl. As this is a wrapping Python exception, the inner error is OSError: [WinError 541541187] Windows Error 0x20474343.

@tgingold
Copy link
Member

The test is trying to load lib_Display/Display.pkg.vhdl twice.

@tgingold
Copy link
Member

This should fix the crash:

--- a/testsuite/pyunit/dom/StopWatch.py
+++ b/testsuite/pyunit/dom/StopWatch.py
@@ -83,7 +83,7 @@ class Designs(TestCase):
     _encoderFiles = _encoderEntityFiles + (
         ("lib_Pretty",    Path("toplevel.Encoder.vhdl")),
     )
-    _displayFiles = _utilityCounterFiles + _displayEntityFiles + _displayPackageFiles + (
+    _displayFiles = _utilityCounterFiles + _displayEntityFiles + (
         ("lib_StopWatch", Path("toplevel.Display.vhdl")),
     )
     _stopwatchFiles = _utilityEntityFiles + _displayEntityFiles + _stopwatchEntityFiles + (

@Paebbels
Copy link
Member Author

While this removes the mistake, how can we make the code more save. Also a user using pyGHDL.dom could make this mistake.

Options I see:

  • I could have an index of loaded files and raise an exception that file was already loaded.
  • I could ask libghdl, if the file was already loaded / exists in libghdl's file mappings.
  • Can we handle the exception?

Looking at the code, name_table.Get_Identifier registers the filename to libghdl. Should I have checked / can I check here for an error code?

@Paebbels
Copy link
Member Author

Oh I see, name_table.Get_Identifier returns an existing id, if it's already in the system, otherwise creates it. And then files_map.Reserve_Source_File fails because there is already a reserved memory region to it.

Could files_map.Reserve_Source_File return a zero or so in case of failure? Then it could be checked.

@Paebbels
Copy link
Member Author

@tgingold
Copy link
Member

tgingold commented May 16, 2023 via email

@Paebbels
Copy link
Member Author

Find_Source_File is not yet in the Python binding. I found Find_Source_File in files_map with 2 parameters. Currently I use Reserve_Source_File with first parameter (directory) set to name_table.Null_Identifier. Can I assume Find_Source_File also works well with directory set to name_table.Null_Identifier?

Is this ok, or do I need to setup directory structures as well? If so, how should it be done?

@tgingold
Copy link
Member

tgingold commented May 16, 2023 via email

@Paebbels
Copy link
Member Author

Paebbels commented May 16, 2023

What am I missing?

AttributeError: function 'files_map__find_source_file' not found

@BindToLibGHDL("files_map__find_source_file")
def Find_Source_File(Directory: NameId, Name: NameId) -> SourceFileEntry:
    """
    Return an existing entry for a filename.

    :param Directory: ``Null_Identifier`` for :obj:`DirectoryId` means current directory.
    :param Name:      File name
    :return:          Return ``No_Source_File_Entry``, if the file does not exist.
    """
    return 0

Is there an export missing in the Ada code?

@tgingold
Copy link
Member

tgingold commented May 16, 2023 via email

@Paebbels
Copy link
Member Author

Ah, sorry, now I see it. Search directed me to the body file adb.

With a check, I can now emit this exception in Python:

pyGHDL.dom.DOMException: Source file 'C:\Git\GitHub\GHDL\ghdl\testsuite\pyunit\dom\examples\Display\lib_Display\Display.pkg.vhdl' already loaded.

@Paebbels Paebbels self-assigned this May 16, 2023
@Paebbels Paebbels force-pushed the paebbels/updated-tests branch 2 times, most recently from 0671eff to 5e8f9d0 Compare May 26, 2023 07:11
@Paebbels Paebbels force-pushed the paebbels/updated-tests branch 2 times, most recently from b748d8a to c3eed8c Compare January 21, 2024 17:10
@Paebbels Paebbels force-pushed the paebbels/updated-tests branch 2 times, most recently from 26055ee to e8fd2a1 Compare April 12, 2024 05:42
(cherry picked from commit 8853d4b3ddd84ed06e33c4902b24e6784915709e)
(cherry picked from commit bd3191d87cb3ae599c41f6e4363d5a3930a34355)
(cherry picked from commit 5b955b635907e97aecae9a4be489d4946de933bf)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants