p4lang / p4c

P4_16 reference compiler
https://p4.org/
Apache License 2.0
665 stars 440 forks source link

SourceInfo initializiation is inconsistent. #4832

Open fruffy opened 1 month ago

fruffy commented 1 month ago

The source info object can be initialized using two approaches:

Using an explicit filename, line number, column number, and fragment: https://github.com/p4lang/p4c/blob/main/lib/source_file.h#L129

Using the Sources object: https://github.com/p4lang/p4c/blob/main/lib/source_file.h#L139

The first initialized method is used by the JSON parser, the second by the conventional Bison parser. Both constructors set different fields, which causes problems using the SourceInfo methods. There should only be one way to initialize Source Info.

Personally, I do not see a reason why a Sources pointer needs to be preserved. SourceInfo should contain all the required information on its own.

ChrisDodd commented 1 month ago

The explicit filename/line stuff used by the json parser is broken and useless, as it is never read by anyone, except when redumping json. It should be removed and replaced by something that is compatible with the "normal" SourceInfo used by all the rest of the code. It was a temporary hack added at some point.

fruffy commented 1 month ago

The explicit filename/line stuff used by the json parser is broken and useless, as it is never read by anyone, except when redumping json. It should be removed and replaced by something that is compatible with the "normal" SourceInfo used by all the rest of the code. It was a temporary hack added at some point.

I tried replacing the JSON instantiations with a version that only uses Sources, but setting that up seems complicated. We need to mock that singleton somehow.