[llvm-commits] [llvm] r75027 - in /llvm/trunk: include/llvm/CodeGen/ include/llvm/Support/ lib/Target/Alpha/ lib/Target/Alpha/AsmPrinter/ lib/Target/IA64/ lib/Target/IA64/AsmPrinter/ lib/Target/MSP430/ lib/Target/Mips/AsmPrinter/ lib/Target/PIC16/ lib/Target/Sparc/AsmPrinter/ lib/Target/X86/ lib/Target/XCore/ utils/TableGen/

[Chris Lattner]

On Jul 8, 2009, at 1:46 PM, Török Edwin wrote:
> Thanks for reviewing my commit, looks like I got it wrong in many  
> places.
> At least error handling should be more consistent now, asserts that  
> are
> meant to be assert are ;)

Well I only listed the ones I disagreed with, you got many more right :)

>
>>
>>> +++ llvm/trunk/include/llvm/CodeGen/MachineCodeEmitter.h Wed Jul  8
>>> 14:04:27 2009
>>> @@ -18,6 +18,8 @@
>>> #define LLVM_CODEGEN_MACHINECODEEMITTER_H
>>>
>>> #include "llvm/Support/DataTypes.h"
>>> +#include "llvm/Support/ErrorHandling.h"
>>> +#include "llvm/Support/raw_ostream.h"
>>>
>>
>> Please don't include headers in other headers unless you cannot avoid
>> it.
>>
>>
>
> I included it because then I don't need to include it in all the
> *ISelDAGToDAG.cpp files.
> I removed it now, and included the necessary headers in each .cpp  
> file.

Why do they need raw_ostream?

>>>    case GlobalValue::DLLExportLinkage:
>>> -      cerr << "DLLExport linkage is not supported by this target! 
>>> \n";
>>> -      abort();
>>> +      llvm_report_error("DLLExport linkage is not supported by this
>>> target!");
>>>
>>
>> assertions. :)
>>
>
> Thanks, I changed them. Quite a lot of copy+pasting between the  
> targets,
> isn't it?

Yes, the asmprinters are copy and paste hell, I'm trying to help a bit  
with the mc stuff.
>>
>> Thanks Edwin, most of this looks great.  Several of these below are
>> assertions that have a message: the preferred style is something  
>> like:
>>
>> #ifndef NDEBUG
>>   cerr << "WHATEVER\n";
>> #endif
>>   unreachable();
>>
>
> Ok, I changed LLVM_UNREACHABLE to do that.
> I also used this style when a more complicated message was printed.

Sounds good.

Thanks Edwin,

-Chris