[cfe-dev] patch for generalizing stdint for n-bit bytes

Chris Lattner clattner at apple.com
Tue Oct 20 23:23:17 PDT 2009


On Oct 20, 2009, at 9:22 AM, Ken Dyck wrote:
>>> If it helps, I'd be happy to separate the new test cases and the
>>> raw_ostream conversion into separate patches.
>>
>> Yes, that would probably be very helpful!
>
> Attached are patches for the stdint.h/InitPreprocessor.cpp  
> modifications and some new tests that exercise preprocessor  
> initialization.

Hi Ken,

This is very interesting work!

First, TargetInfo.h/cpp: these look great.  I applied these as r  
except for this hunk:

+++ lib/Basic/TargetInfo.cpp	(working copy)
@@ -67,12 +67,63 @@
    case SignedInt:        return "int";
    case UnsignedInt:      return "unsigned int";
    case SignedLong:       return "long int";
-  case UnsignedLong:     return "long unsigned int";
+  case UnsignedLong:     return "unsigned long int";
    case SignedLongLong:   return "long long int";
-  case UnsignedLongLong: return "long long unsigned int";
+  case UnsignedLongLong: return "unsigned long long int";
    }
  }

Is this needed or just cleanup?


Please propose a patch that just uses the new methods you added to  
simplify InitPreprocessor without changing the other behavior.  That  
will make it easier to review the patch and it will be more obviously  
correct (and can thus be applied without thinking) :)


The changes to stdint.h are difficult to understand from the patch,  
but look basically reasonable to me.  In the next round of patches,  
I'll look closer.  Just to verify, on boring targets where bytes are 8  
bits, no extra definitions will come out of it (other than the new  
WIDTH macros)?

The testcases are very nice: please merge them together into one test  
that uses multiple RUN lines and uses a different -check-prefix to  
FileCheck.  Also, the RUN lines don't need to fit into 80 columns,  
please use "clang-cc -E ... | FileCheck %s" instead of using %t.

A minor issue, please don't use "const TargetInfo& TI", please use  
"const TargetInfo &TI".

-Chris



More information about the cfe-dev mailing list