[Lldb-commits] [PATCH] D65397: Qualify includes of Properties[Enum].inc files. NFC
Sam McCall via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Mon Jul 29 07:37:14 PDT 2019
sammccall created this revision.
sammccall added reviewers: JDevlieghere, labath, chandlerc.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
labath added a comment.
I am not sure about the consistency argument (on the other CL), as it seems to me that most/all llvm .inc files *which are produced by tblgen" are included by via just their bare names. However, these files generally have unique names, which is usually achieved by just embedding (a derivative of) the folder name into the .inc file name (e.g. AArch64GenAsmWriter.inc, AMDGPUGenAsmWriter.inc, ...). So, I think the most consistent approach here would be to rename lldb files into something like `TargetProperties.inc`, `CoreProperties.inc` etc.
Consistency considerations aside, I do think that having a bunch of files called `Properties.inc` is a bit confusing, and it would be good to disambiguate between them. I'm fairly ambivalent about whether that is achieved by prefixing the include directives with the folder name, or by renaming the files so that they have unique names.
This is a bit more explicit, and makes it possible to build LLDB without
varying the -I lines per-directory.
(The latter is useful because many build systems only allow this to be
configured per-library, and LLDB is insufficiently layered to be split into
multiple libraries on stricter build systems).
(My comment on D65185 <https://reviews.llvm.org/D65185> has some more context)
rG LLVM Github Monorepo
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 8846 bytes
Desc: not available
More information about the lldb-commits