Improve documentation for the tools for generating size checks, based on comments on:

http://codereview.chromium.org/5730003/

Also automated handling of long and unsigned long as suggested in review comments for that CL.

BUG=None
TEST=Reran generate_ppapi_size_checks.py;  produced the same output.

Review URL: http://codereview.chromium.org/6014007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@69950 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 82 insertions, 37 deletions

diff --git a/ppapi/tests/clang/README b/ppapi/tests/clang/README
index 17abb83..64dd332 100644
--- a/ppapi/tests/clang/README
+++ b/ppapi/tests/clang/README
@@ -1,32 +1,57 @@
-This is a directory for Clang plugins that are designed to do analysis and/or manipulation of PPAPI code.  Clang is an open-source C front-end that allows you to parse C, C++, or Objective-C code in to an abstract syntax tree (or AST) for processing.
-
-To use these plugins, you will need to get Clang.  Clang is rapidly changing, so you may want to download and build it yourself.  See the instructions here:
+This is a directory for Clang plugins that are designed to do analysis and/or
+manipulation of PPAPI code.  Clang is an open-source C front-end that allows
+you to parse C, C++, or Objective-C code in to an abstract syntax tree (or AST)
+for processing.  This README assumes that you are working in a check-out of
+chromium.
+
+To use these plugins, you will need to get Clang.  Clang is rapidly changing,
+so you may want to download and build it yourself.  See the instructions here:
 - http://clang.llvm.org/get_started.html
 
-To build the plugins, use the Makefile in this directory.  If you want the provided Makefile to work out-of-the-box, in step 2 of the instructions at the above URL, you should do the following:
+To build the plugins, use the Makefile in this directory.  If you want the
+provided Makefile to work out-of-the-box, in step 2 of the instructions at the
+above URL, you should do the following:
 > mkdir ~/llvm
 > cd ~/llvm
-Now continue with the svn co command to check out llvm in ~/llvm/llvm.
+Now continue with the svn co command to check out llvm in ~/llvm/llvm.  If you
+choose to build llvm in another location, you can use environment variables to
+force the Makefile to find your build of clang.  See the Makefile for details.
 
-To run a plugin, use clang with the -cc1 -load and -plugin flags and an otherwise normal build line.  For example, to run liBPrintNamesAndSizes.so, if you currently build like this:
+To run a plugin, use clang with the -cc1 -load and -plugin flags and an
+otherwise normal build line.  For example, to run liBPrintNamesAndSizes.so, if
+you currently build like this:
 g++ (build_options)
 Run this from the command-line instead:
-clang -cc1 -load ppapi/tests/clang/libPrintNamesAndSizes.so -plugin PrintNamesAndSizes (build_options)
+clang -cc1 -load ppapi/tests/clang/libPrintNamesAndSizes.so \
+  -plugin PrintNamesAndSizes (build_options)
 
 Plugins:
-PrintNamesAndSizes : print_names_and_sizes.cc
-Print information about all top-level type definitions.  You probably won't need to run it by itself;  instead see generate_ppapi_size_checks.py, which uses the plugin.
-
-Example command-line:
-  python generate_ppapi_size_checks.py --ppapi-root=/usr/local/google/chrome_build/src/ppapi
-  python generate_ppapi_size_checks.py --help
-
-
-FindAffectedInterfaces : find_affected_interfaces.cc
-Given typenames as parameters, print out all types that are affected (including function pointer types and structs containing affected function pointer types) if the given type(s) change.  This is meant to be used for determining what interfaces are affected by a change to a struct.
-
-Example command-line:
-  clang -cc1 -load ppapi/tests/clang/libFindAffectedInterfaces.so -plugin FindAffectedInterfaces -I. ppapi/tests/all_includes.h -plugin-arg-FindAffectedInterfaces "struct PP_VideoCompressedDataBuffer_Dev"
-  clang -cc1 -load tests/clang/libFindAffectedInterfaces.so -plugin FindAffectedInterfaces -I../ tests/all_c_includes.h -plugin-arg-FindAffectedInterfaces "struct PP_VideoCompressedDataBuffer_Dev,struct PP_Var"
-
-(This assumes that clang is in your path and you are running the plugin from the ppapi subdirectory in a chrome checkout).
+  PrintNamesAndSizes : print_names_and_sizes.cc
+  Print information about all top-level type definitions.  You probably won't
+  need to run it by itself;  instead see generate_ppapi_size_checks.py, which
+  uses the plugin.  See print_names_and_sizes.cc for more detail on the plugin.
+
+  Example command-line:
+    python generate_ppapi_size_checks.py \
+        --ppapi-root=/usr/local/google/chrome_build/src/ppapi
+    python generate_ppapi_size_checks.py --help
+
+
+  FindAffectedInterfaces : find_affected_interfaces.cc
+  Given typenames as parameters, print out all types that are affected
+  (including function pointer types and structs containing affected function
+  pointer types) if the given type(s) change.  This is meant to be used for
+  determining what interfaces are affected by a change to a struct.
+
+  Example command-line:
+    clang -cc1 -load ppapi/tests/clang/libFindAffectedInterfaces.so \
+        -plugin FindAffectedInterfaces -I. ppapi/tests/all_includes.h \
+        -plugin-arg-FindAffectedInterfaces \
+        "struct PP_VideoCompressedDataBuffer_Dev"
+    clang -cc1 -load tests/clang/libFindAffectedInterfaces.so \
+        -plugin FindAffectedInterfaces -I../ tests/all_c_includes.h \
+        -plugin-arg-FindAffectedInterfaces \
+        "struct PP_VideoCompressedDataBuffer_Dev,struct PP_Var"
+
+(This assumes that clang is in your path and you are running the plugin from
+ the ppapi subdirectory in a chrome checkout).
diff --git a/ppapi/tests/clang/print_names_and_sizes.cc b/ppapi/tests/clang/print_names_and_sizes.cc
index 18aee47..e2c2a60 100644
--- a/ppapi/tests/clang/print_names_and_sizes.cc
+++ b/ppapi/tests/clang/print_names_and_sizes.cc
@@ -6,11 +6,13 @@
 // structs, enums, and typedefs in the input file.
 
 #include <cstdio>
+#include <cstring>
 #include <string>
 
 #include "clang/AST/AST.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/CharUnits.h"
+#include "clang/AST/Decl.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Frontend/CompilerInstance.h"
@@ -20,19 +22,19 @@ namespace {
 
 const char* const kTypedefName = "Typedef";
 const char* const kDelim = ",";
-const char* const kHasPointer = "HasPointer";
-const char* const kNoPointer = "NoPointer";
+const char* const kArchDependent = "ArchDependentSize";
+const char* const kNotArchDependent = "NotArchDependentSize";
 
 // This class consumes a Clang-parsed AST and prints out information about types
 // defined in the global namespace.  Specifically, for each type definition
 // encountered, it prints:
-// "kind,name,size,has_pointer,source_file,first_line,last_line\n"
+// "kind,name,size,arch_dependent,source_file,first_line,last_line\n"
 // Where:
 // - kind:  The Clang TypeClassName (Record, Enum, Typedef, Union, etc)
 // - name:  The unmangled string name of the type.
 // - size:  The size in bytes of the type.
-// - has_pointer:  'HasPointer' if the type is or has a pointer.  Otherwise,
-//                 'NoPointer'.
+// - arch_dependent:  'ArchDependentSize' if the type has architecture-dependent
+//                    size, NotArchDependentSize otherwise.
 // - source_file:  The source file in which the type is defined.
 // - first_line:  The first line of the definition (counting from 0).
 // - last_line:  The last line of the definition (counting from 0).
@@ -47,29 +49,47 @@ class PrintNamesAndSizesConsumer : public clang::ASTConsumer {
   // delete it.
   clang::SourceManager* source_manager_;
 
-  // Return true iff the type is a pointer or contains a pointer.  This is
-  // important because these types may be different sizes on 32-bit vs 64-bit
-  // platforms.  Structs, enums, and unions that do NOT contain pointers are
+  // Return true if the type contains types that differ in size between 32-bit
+  // and 64-bit architectures.  This is true for types that are typedefed to a
+  // pointer, long, or unsigned long and also any types that contain an
+  // architecture-dependent type.  Note that this is a bit overly restrictive;
+  // some unions may be consistent size on 32-bit and 64-bit architectures
+  // despite containing one of these types.  But it's not an important enough
+  // issue to warrant coding the special case.
+  // Structs, enums, and unions that do NOT contain arch-dependent types are
   // crafted to be the same size on 32-bit and 64-bit platforms by convention.
-  bool HasPointer(const clang::Type& type) {
+  bool HasArchDependentSize(const clang::Type& type) {
     if (type.isPointerType()) {
       return true;
+    } else if (const clang::BuiltinType* builtin =
+        dyn_cast<clang::BuiltinType>(&type)) {
+      if ((builtin->getKind() == clang::BuiltinType::Long) ||
+          (builtin->getKind() == clang::BuiltinType::ULong)) {
+        return true;
+      }
+    } else if (const clang::ArrayType* array =
+        dyn_cast<clang::ArrayType>(&type)) {
+      // If it's an array, it has architecture-dependent size if its elements
+      // do.
+      return HasArchDependentSize(*(array->getElementType().getTypePtr()));
+    } else if (const clang::TypedefType* typedef_type =
+        dyn_cast<clang::TypedefType>(&type)) {
+      return HasArchDependentSize(*(typedef_type->desugar().getTypePtr()));
     } else if (const clang::RecordType* record =
         dyn_cast<clang::RecordType>(&type)) {
       // If it's a struct or union, iterate through the fields.  If any of them
-      // contain is or has a pointer, then we have one too.
+      // has architecture-dependent size, then we do too.
       const clang::RecordDecl* decl = record->getDecl();
       clang::RecordDecl::field_iterator iter(decl->field_begin());
       clang::RecordDecl::field_iterator end(decl->field_end());
       for (; iter != end; ++iter) {
-        if (HasPointer(*(iter->getType().getTypePtr()))) {
+        if (HasArchDependentSize(*(iter->getType().getTypePtr()))) {
           return true;
         }
       }
-      // It's a struct or union, but contains no pointers.
+      // It's a struct or union, but contains no architecture-dependent members.
       return false;
     }
-    // It's not a pointer, a struct, or a union.
     return false;
   }
 
@@ -84,7 +104,7 @@ class PrintNamesAndSizesConsumer : public clang::ASTConsumer {
                 kind.c_str(),
                 name.c_str(),
                 size.getQuantity(),
-                HasPointer(type) ? kHasPointer : kNoPointer,
+                HasArchDependentSize(type) ? kArchDependent : kNotArchDependent,
                 presumed_begin.getFilename(),
                 presumed_begin.getLine(),
                 presumed_end.getLine());