[PATCH] D89775: [lld][ELF][test] Add additional test coverage for LTO

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 22 05:26:51 PDT 2020


grimar added inline comments.


================
Comment at: lld/test/ELF/lto/bitcode-wrapper.ll:27
+
+; ERR1: error: [[FILE]]: Invalid bitcode wrapper header
+
----------------
jhenderson wrote:
> grimar wrote:
> > jhenderson wrote:
> > > grimar wrote:
> > > > aside: we probably want to lowercase this message.
> > > I took a quick look, and this is just one of several error messages within the BitcodeReader and BitcodeAnalyzer files, and most of them use upper-case, so that would probably want to be part of a much wider change. Think we'd need to discuss the preferred style of the library with whoever owns/maintains it before doing that.
> > > Think we'd need to discuss the preferred style of the library with whoever owns/maintains it before doing that.
> > 
> > perhaps we could just lower case all error messages on the lld side.
> I thought about that, but that would risk us occasionally converting strings to lower-case that shouldn't be (e.g. file paths, names, accronyms like ELF or DWARF etc).
I meant to lower case just the first character. Perhaps it is a low risk here. Though probably
a more safe way is to convert particular error messages. I.e. if it comes from BitcodeReader/BitcodeAnalyzer we could do it in case if we can't just switch to lower case right there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89775



More information about the llvm-commits mailing list