-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
Expose "parsingAndImporting" setting to user #12199
Conversation
a255e20
to
70a7c70
Compare
I don't understand - why do we need a new "stop after" setting? I thought the idea would be to just fill the absolute path during parsing and before actually trying to load the file? |
The ticket specified
and I figured we have that internal step already so just exposing it would be the easiest solution.. and I figured it makes sense to expose more steps as necessary.. |
I think that, unless it is complicated to do, we should expose more useful data earlier. |
1cda14c
to
0a6852b
Compare
@@ -1102,12 +1116,8 @@ StringMap CompilerStack::loadMissingSources(SourceUnit const& _ast, std::string | |||
{ | |||
solAssert(!import->path().empty(), "Import path cannot be empty."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you check for import->annotation().absolutePath
instead now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It turns out: No. The path can actually be empty after we resolved it. What was previously "." becomes empty. But we still have the assert pre-resolve stage as it was originally...
Changelog.md
Outdated
@@ -8,6 +8,7 @@ Compiler Features: | |||
* Commandline Interface: Accept nested brackets in step sequences passed to ``--yul-optimizations``. | |||
* Commandline Interface: Add ``--debug-info`` option for selecting how much extra debug information should be included in the produced EVM assembly and Yul code. | |||
* Commandline Interface: Use different colors when printing errors, warnings and infos. | |||
* General: Absolute paths of imports are now set in the ``parsing`` stage. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could use "now" in every entry, so I think it is better to not use it at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh and as category, maybe "JSON AST"? Because that is what is relevant to the user reading the changelog.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The option is available using the command line as well though?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, but the json ast is a generic thing that is available to all ways to access the compiler.
0a6852b
to
3a5142a
Compare
Tests failing |
3a5142a
to
6a2ea7b
Compare
for (auto const& import: ASTNode::filteredNodes<ImportDirective>(source.ast->nodes())) | ||
{ | ||
solAssert(!import->path().empty(), "Import path cannot be empty."); | ||
|
||
// The current value of `path` is the absolute path as seen from this source file. | ||
// We first have to apply remappings before we can store the actual absolute path | ||
// as seen globally. | ||
import->annotation().absolutePath = | ||
applyRemapping( | ||
util::absolutePath( | ||
import->path(), | ||
path | ||
), | ||
path | ||
); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for (auto const& import: ASTNode::filteredNodes<ImportDirective>(source.ast->nodes())) | |
{ | |
solAssert(!import->path().empty(), "Import path cannot be empty."); | |
// The current value of `path` is the absolute path as seen from this source file. | |
// We first have to apply remappings before we can store the actual absolute path | |
// as seen globally. | |
import->annotation().absolutePath = | |
applyRemapping( | |
util::absolutePath( | |
import->path(), | |
path | |
), | |
path | |
); | |
} | |
for (auto const& import: ASTNode::filteredNodes<ImportDirective>(source.ast->nodes())) | |
{ | |
solAssert(!import->path().empty(), "Import path cannot be empty."); | |
// The current value of `path` is the absolute path as seen from this source file. | |
// We first have to apply remappings before we can store the actual absolute path | |
// as seen globally. | |
import->annotation().absolutePath = applyRemapping(util::absolutePath( | |
import->path(), | |
path | |
), path); | |
} |
139b5cd
to
f860891
Compare
f860891
to
1ed2977
Compare
{ | ||
"ast": | ||
{ | ||
"absolutePath": "/project/../C.sol", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah ok, it was named like that in the input json.
{ | ||
"ast": | ||
{ | ||
"absolutePath": "/project/../C.sol", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah ok, it was named like that in the input json.
1ed2977
to
fc224f7
Compare
fixes #11756