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

Add file name to CodePosition #134

Open
tictacmenthe opened this issue Aug 26, 2020 · 5 comments
Open

Add file name to CodePosition #134

tictacmenthe opened this issue Aug 26, 2020 · 5 comments

Comments

@tictacmenthe
Copy link

Hi,

I am using HdlConvertor for a project, and I need to be able to know from which file comes which module, but `include directives clearly break this as all the included files are put in the same HdlContext, with no information (as far as I know) of where the module is.

  • Is there a way to disable the preprocessor/ignore includes ? Or a way to know which include resulted in which HdlModuleDef/other structures?

I have an alternative to this (parse includes, use hdlconvertor on the included files, then remove the results from the main HdlContext), but having a way to do it may be useful as a feature of the library.

Thanks.

@Nic30
Copy link
Owner

Nic30 commented Aug 26, 2020

Hello,

the feature is not completed yet. There is an information about origin of the line in preprocessor
https://github.com/Nic30/hdlConvertor/blob/master/include/hdlConvertor/verilogPreproc/out_buffer.h#L32
But it is not propagated to HDL AST. It is easy to do. However we need std::shared_ptr for file names if we want to add it to every code position object, in order to prevent unnecessary copy of string for each node in expressions, because it would be slow.
https://github.com/Nic30/hdlConvertor/blob/master/include/hdlConvertor/hdlAst/codePosition.h#L13

@Nic30 Nic30 changed the title Disable Preprocessor Add file name to CodePosition Aug 26, 2020
@Thomasb81
Copy link
Contributor

I have an alternative to this (parse includes, use hdlconvertor on the included files, then remove the results from the main HdlContext), but having a way to do it may be useful as a feature of the library.

This may work for your usecase. But will not work in general : The content of an included file may not be legal verilog or systemVerilog source code.
So parsing included file may lead the tool to report invalid source code.

Disabling the consideration of include preprocessor directive is not a good idea.

@Nic30
Copy link
Owner

Nic30 commented Aug 27, 2020

As @Thomasb81 mentioned content of include file may not be Verilog code. But there is yet an another problem. The actual file name and file name which can be overriden using line directive are two different things I expect you want the first thing, but current implementation is the first thing.

Do you know if std::string uses copy-on-write on all platforms?
Do you know if there is something like PyUnicode_FromString which can share Python str instances for same input string?

Once I do have answer for this questions the implementation is matter of 1h.
If anyone can prepare tests it would be nice.

@tictacmenthe
Copy link
Author

@Thomasb81 you are completely right.
But then doesn't that mean the file that includes the illegal verilog/sv code also illegal ?
I do not see what is the difference then.

But yes just adding the source file to position would work best.
My alternative wasn't a proposition for the library, it was just what I'll be doing now at work.

@Nic30
Copy link
Owner

Nic30 commented Aug 27, 2020

Include directive can be almost everywhere that is why the included file can contain only fragment of the code (e.g. an array initialization) which is not a valid verilog because it misses the surrounding code.

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

No branches or pull requests

3 participants