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 25 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 8fd2965191f3484bdf960e0ef08df6da04c41647)
(cherry picked from commit fdfe104531660a91c6d3b107a14d572b2333d5bc)
(cherry picked from commit b783c9f8834116da368cbcb69667b3b88bca7d1f)
(cherry picked from commit f779b5c59499f8d3b3ba5cad48cb1001bc02e656)
(cherry picked from commit 7616d44a65bc20e504278390f986ada25e6ff2a1)
(cherry picked from commit e7e4844d52d72ba17761858ef2a32be4024724ba)
(cherry picked from commit 5b955b635907e97aecae9a4be489d4946de933bf)
@Paebbels
Copy link
Member Author

Paebbels commented Jun 10, 2024

@Tcenova is interested in this PR.

See also #2661.

(cherry picked from commit 306029bdf403e0a0fea8d9a6aab9b4e1820024d5)
(cherry picked from commit f6f77b446383e999b835dd1f9964c783164ccae9)
(cherry picked from commit 2b8e7d946c9f4cdc21bf6e8c45ed44680c98609b)
(cherry picked from commit 2871586c9026053e4d7b8788c22fd6d10ab639df)
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