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.

Sign up for GitHub

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

Jump to bottom

Resolve include directives relative to each file #109

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from
Open

Resolve include directives relative to each file #109

wants to merge 8 commits into from

Conversation

ngwood
Copy link
Contributor

@ngwood ngwood commented Jan 22, 2022

This commit changes how ADMS resolves include directives. ADMS currently
does this relative to the current working directory, each user-provided
search path and the ADMS include directory, in that order. This is not
typical of Verilog-A compilers or compilers in general.

Following are just a few examples of why the current ADMS behaviour is
undesirable.

Example 1:

A.va            `include "sub/B.va"
sub/B.va        `include "C.va"
sub/C.va        // desired file
C.va            // stray file

"C.va" will always find the stray file in the current working
directory.

Example 2:

dir/A.va        `include "sub/B.va"
dir/sub/B.va    `include "C.va"
dir/sub/C.va    // desired file
dir/C.va        // stray file

"sub/B.va" will only find the desired file if "dir" is manually
added to the search path. "C.va" would then always find the stray
file, unless "dir/sub" had been previously added to the search
path. Either of these actions could have unintended consequences.

This commit addresses these limitations.

ADMS will now first look for files relative to the directory of the
file containing the include directives and will behave the same way
regardless of the current working directory or how many nested include
directives are used, without the need to specify additional user-defined
search paths.

ngwood added 4 commits January 22, 2022 14:53
@ngwood
Resolve include directives relative to each file
6dcc6dc 
This commit changes how ADMS resolves include directives. ADMS currently
does this relative to the current working directory, each user-provided
search path and the ADMS include directory, in that order. This is not
typical of Verilog-A compilers or compilers in general.

Following are just a few examples of why the current ADMS behaviour is
undesirable.

Example 1:

    A.va            `include "sub/B.va"
    sub/B.va        `include "C.va"
    sub/C.va        // desired file
    C.va            // stray file

    "C.va" will always find the stray file in the current working
    directory.

Example 2:

    dir/A.va        `include "sub/B.va"
    dir/sub/B.va    `include "C.va"
    dir/sub/C.va    // desired file
    dir/C.va        // stray file

    "sub/B.va" will only find the desired file if "dir" is manually
    added to the search path. "C.va" would then always find the stray
    file, unless "dir/sub" had been previously added to the search
    path. Either of these actions could have unintended consequences.

This commit addresses these limitations.

ADMS will now first look for files relative to the directory of the
file containing the include directives and will behave the same way
regardless of the current working directory or how many nested include
directives are used, without the need to specify additional user-defined
search paths.
@ngwood
Update NEWS
442e164
@ngwood
Fix memory leak
6c90107
@ngwood
Move function prototype
3327fa5
@ngwood
Copy link
Contributor Author

ngwood commented Jan 23, 2022

I'm not 100% sure on whether this will work on Windows yet. Also, the dirname function had already been defined statically in admsXml.c meaning it's not defined in library admsElement like almost all other functions of this kind are. admsXml.c is used to create admsXml; but if you ever wanted to link against the admsPreprocessor library elsewhere you'd run into problems. Is it worth relocating the dirname and dependent functions? They could be made part of mkelements.pl. I'm thinking it would be safer and more consistent. Could we rename dirname to adms_dirname at the same time perhaps, as dirname is a standard library (string.h) function name?

@ngwood
Copy link
Contributor Author

ngwood commented Jan 23, 2022

Is it possible to check this builds correctly on Windows?

@ngwood ngwood mentioned this pull request Jan 24, 2022
Open
ngwood added 4 commits January 24, 2022 17:08
@ngwood
Append adms_ to some static function names
eb49203
@ngwood
Move renamed functions into libadmsElement
8b65ee9 
The adms_dirname function needs to be used in preprocessorLex.l, but
it is currently a static function declared in admsXml.c. Making it
accessible from libadmsElement allows both files to use it.

The dependencies, adms_split_new and adms_free_strlist, have also
been relocated, as well as the related functions adms_basename and
adms_filename, for consistency.
@ngwood
Undo changes to ADMS libraries
06ff2d6 
This reverts commit 8b65ee9.
This reverts commit eb49203.
This reverts commit 3327fa5.
@ngwood
Add static dirname function to preprocessorLex.l
71c6d35 
I have copied the static function dirname from admsXml.c along with its
two dependencies. Ideally, both instances of this function should be
cleaned up. At this stage, I just want to get include directives to
evaluate relative paths correctly.
@ngwood
Copy link
Contributor Author

ngwood commented Jan 25, 2022

Commit 71c6d35 contains all changes to in the pull request to preprocessorLex.l. The common static functions in preprocessorLex.l and admsXml.c can be cleaned up together at a later stage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@ngwood