Refactor Standard JSON Input/Output Documentation#16562
Refactor Standard JSON Input/Output Documentation#16562matheusaaguiar wants to merge 1 commit intofixRevertStringFlagInconsistentHandlingfrom
Conversation
matheusaaguiar
changed the base branch from
develop
to
fixRevertStringFlagInconsistentHandling
matheusaaguiar
added
has dependencies
There was a problem hiding this comment.
This is nice, I always wanted to have a proper docs for these rather than just a huge code block, but I'm not sure this PR is really the way to go.
It rewrites everything in one go, but there's just too much detail accumulated over the years. I already see a few things that need adjustment and it'll be a pain to make sure that this detail is not lost and stays accurate throughout multiple rounds of review fixes over the whole thing.
If you want to tackle it, I'd suggest doing it in stages. For example the description of outputs and output selection seems like good starting point. Settings would also be a nice self-contained part that could be done separately. Then we could finally do the overall structure.
It would also be best to start with only taking out the existing info and reformatting it into tables, with only just enough changes for it to still make sense in that form. Then have a separate PR with any extra adjustments like correcting inaccuracies, making things read better, adding missing info, etc. This way those initial PRs would be much lighter to review as it would be enough to verify that the content is still the same without going into detailed discussions about it.
There was a problem hiding this comment.
I don't think it makes sense to add this but leave the comments in the JSON example as is. Doing this will generate more work for us because we'll have duplicate information.
| urls | ||
| ^^^^ | ||
|
|
||
| URL(s) to the source file. | ||
| Required unless ``content`` is used |
There was a problem hiding this comment.
You're missing a section for the content field.
| ast | ||
| ^^^ | ||
|
|
||
| Required if language is set to "SolidityAST". |
There was a problem hiding this comment.
We should have a table showing which fields in sources are allowed for which language. Or make it a part of the table that shows top-level fields.
| TODO: | ||
| ".data": { ... }, // optional | ||
| "sourceList": [ ... ] // optional (if no ``source`` node was defined in any ``.code`` object) |
There was a problem hiding this comment.
Documenting it would be a whole separate task (we should really have a JSON schema for this). I'd show it it only as a part of the JSON example.
BTW, we'll likely need a separate example for each language.
| +------------------+----------+-------------------------------------------------------------------------+ | ||
| | ``viaSSACFG`` | boolean | Experimental. Turn on SSA CFG-based code generation via the IR | | ||
| | | | (implies ``viaIR: true``). False by default. | | ||
| +------------------+----------+-------------------------------------------------------------------------+ | ||
|
|
||
| Optimizer Settings | ||
| ^^^^^^^^^^^^^^^^^^ | ||
|
|
||
| The ``optimizer`` object controls how aggressively the compiler optimizes the generated bytecode. |
There was a problem hiding this comment.
These tables should be exhaustive. For this to work as a reference there needs to be one where you can see all the allowed fields. For example, the one here should include the optimizer field showing that it's a dict, even if it has a separate section dedicated to it.
| | | | under certain circumstances. Always runs, even with | | ||
| | | | optimization disabled, unless explicitly turned off. | | ||
| +--------------------------------------------+---------+---------------------------------------------------------------+ | ||
| | ``yul`` | True | Yul optimizer. Used to optimize the IR produced by the Yul | |
There was a problem hiding this comment.
This is missing some nuance that was in the previous docs. The default is not static. It's True but only when the optimizer is enabled.
| - irOptimizedAst - AST of intermediate representation after optimization (experimental) | ||
| - storageLayout - Slots, offsets and types of the contract's state variables in storage | ||
| - transientStorageLayout - Slots, offsets and types of the contract's state variables in transient storage | ||
| - evm.assembly - New assembly format |
There was a problem hiding this comment.
These are identifiers so we should present them as such.
| - evm.assembly - New assembly format | |
| - ``evm.assembly`` - New assembly format |
This should really be a table too though. Maybe even with some nested cells for each nesting level.
| - evm.bytecode.sourceMap - Source mapping (useful for debugging) | ||
| - evm.bytecode.linkReferences - Link references (if unlinked object) | ||
| - evm.bytecode.generatedSources - Sources generated by the compiler | ||
| - evm.deployedBytecode* - Deployed bytecode (has all the options that evm.bytecode has) |
There was a problem hiding this comment.
| - evm.deployedBytecode* - Deployed bytecode (has all the options that evm.bytecode has) | |
| - evm.deployedBytecode.* - Deployed bytecode (has all the options that evm.bytecode has) |
But probably would be better to just list them. Or to have a separate table listing the bytecode outputs.
There was a problem hiding this comment.
I see you already have them in a table in output description. I think that it would be best not to list outputs here but just say how they work, but then just link to that section. It would also reinforce the fact that these lists are not independent. Everything here should have a corresponding output.
| | ``contracts`` | object | Yes | This contains the contract-level outputs. | | ||
| | | | | It can be limited/filtered by the outputSelection settings. | |
There was a problem hiding this comment.
Is this actually mandatory? I think we don't include it when there are no contracts.
| Error Types | ||
| ^^^^^^^^^^^ | ||
|
|
||
| 1. ``JSONError``: JSON input doesn't conform to the required format, e.g. input is not a JSON object, the language is not supported, etc. |
There was a problem hiding this comment.
Let's make this a table too.
2 participants
Tentatively improve readability and organization of the JSON Input/Output docs.
Depends on #16466,