[PATCH] D14602: Switch lto codegen to using diagnostic handlers

Duncan P. N. Exon Smith via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 12 07:09:49 PST 2015


> On 2015-Nov-12, at 07:04, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
> 
> 
>> On 2015-Nov-11, at 18:55, Yunzhong Gao <Yunzhong_Gao at playstation.sony.com> wrote:
>> 
>> ygao created this revision.
>> ygao added subscribers: rafael, dexonsmith, pcc, joker.eph, llvm-commits.
>> 
>> With r252791, the C api will install a default diagnostic handler to print to sLastErrorString, therefore
>> we no longer need to pass around this sLastErrorString to any the LTOCodeGenerator methods.
>> 
>> This patch removes the std::string& argument from a number of C++ LTO API calls and instead
>> makes them use the installed diagnostic handler. This would also improve consistency of diagnostic
>> handling infrastructure: if an LTO client used lto_codegen_set_diagnostic_handler() to install a
>> custom error handler, we do not want some error messages to go through the custom error handler,
>> and some other error messages to go into sLastErrorString.
> 
> Sounds awesome.
> 
>> The part that I am not entirely sure about is what should happen when there is no installed
>> diagnostic handler at all. In this patch, the fall-back is to print the message to stderr. For this case
>> to happen, if the client is using the C api, the client must have explicitly called
>> lto_codegen_set_diagnostic_handler(nullptr), and maybe the intention is to not print any error at all?
>> I am not sure.
> 
> Since `lto_codegen_set_diagnostic_handler()` is the only interface for
> diagnostics, I think it should set the diagnostic handler back to the
> default diagnostic handler, not to stderr.

Willing to be convinced otherwise though.  We certainly don't rely on
the behaviour of `lto_codegen_set_diagnostic_handler()`, so I'm just
trying to guess what will feel like more consistent API.

>> http://reviews.llvm.org/D14602
>> 
>> Files:
>> include/llvm/LTO/LTOCodeGenerator.h
>> lib/LTO/LTOCodeGenerator.cpp
>> tools/llvm-lto/llvm-lto.cpp
>> tools/lto/lto.cpp
>> 
>> <D14602.40000.patch>
> 



More information about the llvm-commits mailing list