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

[Ken Dyck]

On Wednesday, October 21, 2009 2:23 AM, Chris Lattner wrote:
> On Oct 20, 2009, at 9:22 AM, Ken Dyck wrote:
> > Attached are patches for the stdint.h/InitPreprocessor.cpp 
> > modifications and some new tests that exercise preprocessor 
> > initialization.
> 
> 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?

More of an unrelated fix. Neither "long unsigned int" nor "long long
unsigned int" are listed among the acceptable type specifiers in section
6.7.2 of C99 or section 7.1.5.2 of C++98. Clang obviously must handle
them correctly, but I figured changing them could only improve
portability.

> 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) :)

Is the attached patch what you have in mind? Or would you like to see a
more aggressive refactoring to remove all of the literal type names and
constant suffixes from InitPreprocessor.cpp?

> 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)?

There aren't any additional user macros, but there are a few more
implementation macros. Some are for the SIG_ATOMIC definitions, which
were previously hard-coded as 32-bit integers. There are also some new
SIGNED macros so stdint.h can define the correct limits for the types
that can can be either signed or unsigned.

> 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.

How about two tests: one to test stdint.h and the other to test basic
preprocessor initialization? That is what is in the attached patch,
anyways.

The patch corresponds to the current stdint.h, so it can be applied
safely now, with (or without) the InitPreprocessor.cpp changes in the
other patch. This will provide a baseline for evaluating further changes
to stdint.h and InitPreprocessor.cpp.

Cheers,
-Ken
-------------- next part --------------
A non-text attachment was scrubbed...
Name: InitPreprocessor-simplify.r84758.patch
Type: application/octet-stream
Size: 1617 bytes
Desc: InitPreprocessor-simplify.r84758.patch
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20091022/0226bc4b/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: cpp-tests-pre-stdint-mods.r84758.patch
Type: application/octet-stream
Size: 77337 bytes
Desc: cpp-tests-pre-stdint-mods.r84758.patch
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20091022/0226bc4b/attachment-0001.obj>