YosysHQ / arachne-pnr

Place and route tool for FPGAs
MIT License
413 stars 73 forks source link

Include associated headers first #95

Closed doctaweeks closed 6 years ago

doctaweeks commented 6 years ago

This prevents missing dependencies in the header files.

cliffordwolf commented 6 years ago

Which header is missing? This looks like what you'd actually want to do is add missing #include statements to the *.hh files.

doctaweeks commented 6 years ago

Nothing is missing. It's a change to prevent masking missing dependencies in header files in the future. Currently, it possible to introduce a change that inadvertently includes a dependency of a header file by way of another header file or a source file rather than including it in the header file where it's needed.

mithro commented 6 years ago

FYI - https://github.com/include-what-you-use/include-what-you-use

cliffordwolf commented 6 years ago

@doctaweeks I don't understand what you mean. Is this about *.d files? In what way are they different when you include the headers in a different order? If everything else is correct, then changing the order of #include statements should not have any consequences. If it makes a difference then this is very likely a symptom of another problem (like missing #include statements in header files), in which case the other problem should be fixed.

doctaweeks commented 6 years ago

If it makes a difference then this is very likely a symptom of another problem (like missing #include statements in header files), in which case the other problem should be fixed.

@cliffordwolf Agreed. However, the current order means the scenario you describe could be masked if the header file is not included first in the source file. For example, if configuration.hh needed string for some reason it should be included there but say the author forgets. You would expect to get an error when compiling configuration.cc because string is missing in the associated header. However, chipdb.hh already includes string and appears before configuration.hh so it will mask the problem.

Include order local to global prevents masking problems like this.