-
Notifications
You must be signed in to change notification settings - Fork 46
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
Parser for includes. #132
Comments
Hi threonorm, Jamey is on vacation with limited email access, so your question may not be answered in time. It sounds like the error is due to some hacky-ness in bsvdependencies.py , would you be able suggest a fix for this? I think you are correct that none of these two cases should generate a dependency. |
Hi hanw, thanks for your answer! I have a "fix" where I just consider every commented dependency to be not a dependency, this work for my use of the bsvdependencies functions, but I suspect that Jamey uses it in a different way somewhere else where he actually wants to keep this feature. (The way the code is written make me believe that this is really a feature) For this reason, I don't want to pull request yet, waiting for his opinion is definitely a good idea :). |
I guess it would be less Jaclyn if bsvpreprocess returned a list of included files as well as the expanded source code. Do you have time to make that change? Then it could ignore all commented out includes. |
autocorrect strikes again :) |
In case you did not see the comments I made in one of the chat channels:
I think this is an artifact of the interaction between bsvpreprocess and
bsvdepend. It's a bit too hackish, now, so probably it would be better if
bsvpreprocess returned a list of the included files along with the
preprocessed text and then we do not have to treat commented out includes
specially.
Either that, or we need a much more unique tag for the commented includes.
How about a commented out attribute?
// (* included_file="filename.bsvi" *)
It's too bad the BSV compiler does not support the equivalent of C/C++
#line directive, which is used for just this purpose.
If you have a chance to make a pull request along either of these lines I
would greatly appreciate it.
Happy holidays!
…On Fri, Dec 2, 2016 at 3:03 PM threonorm ***@***.***> wrote:
Hi hanw, thanks for your answer!
I have a "fix" where I just consider every commented dependency to be not
a dependency, this work for my use of the bsvdependencies functions, but I
suspect that Jamey uses it in a different way somewhere else where he
actually wants to keep this feature. (The way the code is written make me
believe that this is really a feature)
For this reason, I don't want to pull request yet, waiting for his opinion
is definitely a good idea :).
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#132 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACU3s-ktGy7VqAi3VAL82uMkgZU-4Kluks5rEHmjgaJpZM4LC4WU>
.
|
Hello Jamey,
I am not sure I understand the problem properly : I don't understand what is
this special kind of include for and where/what it used for. (What are those include that are commented, but still considered as include?)
Though if abstractly speaking you just need a special kind of include, only
seen by connectal build system, and added by connectal automatically somehow, I
think I agree with your second suggestion, I can modify bsvdependencies to
special pick something like //CONNECTALINCLUDE "nameoffile"
I can definitely make the one line change in bsvdependencies, but I would
break everything by doing so, because I don't know where I should change
things as well. (I guess some script add those special include for connectal
somewhere : I think those special include are not hand written, are they ? And
if they are, it means that I will break every project that is using them.).
Thomas
Excerpts from Jamey Hicks's message of 2016-12-30 08:10:02 -0500:
… In case you did not see the comments I made in one of the chat channels:
I think this is an artifact of the interaction between bsvpreprocess and
bsvdepend. It's a bit too hackish, now, so probably it would be better if
bsvpreprocess returned a list of the included files along with the
preprocessed text and then we do not have to treat commented out includes
specially.
Either that, or we need a much more unique tag for the commented includes.
How about a commented out attribute?
// (* included_file="filename.bsvi" *)
It's too bad the BSV compiler does not support the equivalent of C/C++
#line directive, which is used for just this purpose.
If you have a chance to make a pull request along either of these lines I
would greatly appreciate it.
Happy holidays!
On Fri, Dec 2, 2016 at 3:03 PM threonorm ***@***.***> wrote:
> Hi hanw, thanks for your answer!
>
> I have a "fix" where I just consider every commented dependency to be not
> a dependency, this work for my use of the bsvdependencies functions, but I
> suspect that Jamey uses it in a different way somewhere else where he
> actually wants to keep this feature. (The way the code is written make me
> believe that this is really a feature)
>
> For this reason, I don't want to pull request yet, waiting for his opinion
> is definitely a good idea :).
>
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub
> <#132 (comment)>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ACU3s-ktGy7VqAi3VAL82uMkgZU-4Kluks5rEHmjgaJpZM4LC4WU>
> .
>
|
Depending on how we comment an include :
the script for dependency tracking consider that foo is a dependency, but not bar. When I expect the two to not be dependencies (as they are commented).
I guess connectal used the first form of comment as a feature to signal weird dependies invisible to bluespec but visible to connectal?
The code involved is in bsvdependencies.py:
I am not sure why this special case. I don't have the full picture.
If connectal really need to do this kind of commented include that should not be seen by BSV, perhaps connectal could use a special form of include like `includeConnectal, that would be in the comment section?
The text was updated successfully, but these errors were encountered: