[cfe-commits] r43052 - in /cfe/trunk: Driver/ASTConsumers.h Driver/SerializationTest.cpp Driver/clang.cpp include/clang/Basic/IdentifierTable.h

[Chris Lattner]

> URL: http://llvm.org/viewvc/llvm-project?rev=43052&view=rev
> Log:
> Started work on clang object serialization.  Experimental
> serialization logic as well as driver code is now in
> Driver/SerializationTest.cpp.  The status of this code is that it
> should be used by no clients.
>
> Added --test-pickling option to driver to run the serialization code.
>
> Modified IdentifierInfo and IdentifierTable to have friend classes
> that permit object serialization.  Such friendship may not be needed
> in the final design.

cool, everyone likes pickles.  Some comments:

> +
> +using namespace clang;

Ok.

> +using llvm::BitstreamWriter;
> +using std::cerr;
> +using std::cout;
> +using std::endl;
> +using std::flush;

None of these are really needed.  BitstreamWriter is only mentioned  
twice (so adding llvm:: prefix isn't a big deal), cerr/cout should be  
avoided (ok for prototype code though), endl should never be used,  
and you never use endl/flush.

> +namespace {
> +class SerializationTest : public ASTConsumer {
> +  IdentifierTable* IdTable;
> +  unsigned MainFileID;
> +public:
> +  void Initialize(ASTContext& Context, unsigned mainFileID) {
> +    IdTable = &Context.Idents;
> +    MainFileID = mainFileID;
> +    RunSerializationTest();

fwiw, this will call RunSerializationTest before the file is parsed.  
You probably want to do this stuff in the SerializationTest dtor like  
the rewriter test does.

> +  Writer(std::ostream& out) : Stream(Buffer), Out(out) {
> +    Buffer.reserve(256*1024);
> +
> +    // Emit the file header.
> +    Stream.Emit((unsigned)'B', 8);
> +    Stream.Emit((unsigned)'C', 8);
> +    Stream.Emit(0xC, 4);
> +    Stream.Emit(0xF, 4);
> +    Stream.Emit(0xE, 4);
> +    Stream.Emit(0x0, 4);

Cute :)

> +  inline void operator()(unsigned X, unsigned bits, bool VBR=false) {
> +    if (VBR) Stream.Emit(X,bits);
> +    else Stream.Emit(X,bits);
> +  }

It looks like one of these should be EmitVBR or something.

> +// 
> ===------------------------------------------------------------------- 
> ---===//
> +// Serialization "Driver" code.
> +// 
> ===------------------------------------------------------------------- 
> ---===//
> +
> +void SerializationTest::RunSerializationTest() {
> +  std::string ErrMsg;
> +  llvm::sys::Path Filename = llvm::sys::Path::GetTemporaryDirectory 
> (&ErrMsg);
> +
> +  if (Filename.isEmpty()) {
> +    cerr << "Error: " << ErrMsg << "\n";
> +    return;
> +  }
> +
> +  Filename.appendComponent("test.cfe_bc");

Eventually, how about just ".ast"?

>
> +template<> template<typename Introspector>
> +struct IntrospectionTrait<clang::IdentifierInfo>::Ops<Introspector> {
> +  static void Introspect(clang::IdentifierInfo& X, Introspector& I) {
> +//    I(X.getTokenID());
> +    I(X.getBuiltinID(),9); // FIXME: do 9 bit specialization.
> +//    I(X.getObjCKeywordID());
> +    I(X.hasMacroDefinition());
> +    I(X.isExtensionToken());
> +    I(X.isPoisoned());
> +    I(X.isOtherTargetMacro());
> +    I(X.isCPlusPlusOperatorKeyword());
> +    I(X.isNonPortableBuiltin());
> +  }
> +};
> +
> +template<> template<>
> +struct IntrospectionTrait<clang::IdentifierTable>::Ops<Writer> {
> +  static void Introspect(clang::IdentifierTable& X, Writer& W) {
> +    W.EnterSubblock<clang::IdentifierTable>(1);
> +/*
> +    for (clang::IdentifierTable::iterator I = X.begin(), E = X.end();
> +         I != E; ++I)
> +      W(I->getValue());
> +   */
> +    W.ExitBlock();
> +  }
> +};

Nice!

> @@ -60,6 +60,7 @@
>    WarnDeadStores,               // Run DeadStores checker on  
> parsed ASTs.
>    WarnDeadStoresCheck,          // Check diagnostics for  
> "DeadStores".
>    WarnUninitVals,               // Run UnitializedVariables checker.
> +  TestSerialization,            // Run experimental serialization  
> code.
>    ParsePrintCallbacks,          // Parse and print each callback.
>    ParseSyntaxOnly,              // Parse and perform semantic  
> analysis.
>    ParseNoop,                    // Parse with noop callbacks.
> @@ -100,6 +101,8 @@
>                          "Flag warnings of stores to dead  
> variables."),
>               clEnumValN(WarnUninitVals, "warn-uninit-values",
>                          "Flag warnings of uses of unitialized  
> variables."),
> +             clEnumValN(TestSerialization, "test-pickling",
> +                        "Run prototype serializtion code."),
>               clEnumValN(EmitLLVM, "emit-llvm",
>                          "Build ASTs then convert to LLVM, emit .ll  
> file"),
>               clEnumValN(RewriteTest, "rewrite-test",

This is getting a bit crazy/unmaintainable.  When convenient, can you  
please investigate making the various clang modes dynamically  
registratable using the "llvm/Support/Registry.h" mechanism?  It  
would be very nice for clang to just get new options when new .o  
files are linked in (this also gives support for dynamically loadable  
toys).

Thanks Ted,

-Chris