[Lldb-commits] [PATCH] D65397: Qualify includes of Properties[Enum].inc files. NFC

Mon Jul 29 07:37:28 PDT 2019

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.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65397/new/

https://reviews.llvm.org/D65397