[llvm-commits] [llvm] r81316 - in /llvm/trunk: include/llvm/Support/TypeBuilder.h unittests/Support/TypeBuilderTest.cpp

Jeffrey Yasskin jyasskin at google.com
Tue Sep 8 23:12:05 PDT 2009


Hi Tanya. I think this should go into 2.6 because the errors it
prevents are really hard to diagnose (because there are few to no
assertions that contexts match).

On Tue, Sep 8, 2009 at 10:04 PM, Jeffrey Yasskin<jyasskin at google.com> wrote:
> Author: jyasskin
> Date: Wed Sep  9 00:04:01 2009
> New Revision: 81316
>
> URL: http://llvm.org/viewvc/llvm-project?rev=81316&view=rev
> Log:
> Make TypeBuilder's result depend on the LLVMContext it's passed.
> TypeBuilder was using a local static variable to cache its result. This made it
> ignore changes in its LLVMContext argument and always return a type constructed
> from the argument to the first call.
>
> Modified:
>    llvm/trunk/include/llvm/Support/TypeBuilder.h
>    llvm/trunk/unittests/Support/TypeBuilderTest.cpp
>
> Modified: llvm/trunk/include/llvm/Support/TypeBuilder.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/TypeBuilder.h?rev=81316&r1=81315&r2=81316&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/Support/TypeBuilder.h (original)
> +++ llvm/trunk/include/llvm/Support/TypeBuilder.h Wed Sep  9 00:04:01 2009
> @@ -50,15 +50,14 @@
>  ///   namespace llvm {
>  ///   template<bool xcompile> class TypeBuilder<MyType, xcompile> {
>  ///   public:
> -///     static const StructType *get() {
> -///       // Using the static result variable ensures that the type is
> -///       // only looked up once.
> -///       static const StructType *const result = StructType::get(
> -///         TypeBuilder<types::i<32>, xcompile>::get(),
> -///         TypeBuilder<types::i<32>*, xcompile>::get(),
> -///         TypeBuilder<types::i<8>*[], xcompile>::get(),
> +///     static const StructType *get(LLVMContext &Context) {
> +///       // If you cache this result, be sure to cache it separately
> +///       // for each LLVMContext.
> +///       return StructType::get(
> +///         TypeBuilder<types::i<32>, xcompile>::get(Context),
> +///         TypeBuilder<types::i<32>*, xcompile>::get(Context),
> +///         TypeBuilder<types::i<8>*[], xcompile>::get(Context),
>  ///         NULL);
> -///       return result;
>  ///     }
>  ///
>  ///     // You may find this a convenient place to put some constants
> @@ -72,9 +71,6 @@
>  ///   }
>  ///   }  // namespace llvm
>  ///
> -/// Using the static result variable ensures that the type is only looked up
> -/// once.
> -///
>  /// TypeBuilder cannot handle recursive types or types you only know at runtime.
>  /// If you try to give it a recursive type, it will deadlock, infinitely
>  /// recurse, or throw a recursive_init exception.
> @@ -106,9 +102,7 @@
>  template<typename T, bool cross> class TypeBuilder<T*, cross> {
>  public:
>   static const PointerType *get(LLVMContext &Context) {
> -    static const PointerType *const result =
> -      PointerType::getUnqual(TypeBuilder<T,cross>::get(Context));
> -    return result;
> +    return PointerType::getUnqual(TypeBuilder<T,cross>::get(Context));
>   }
>  };
>
> @@ -119,18 +113,14 @@
>  template<typename T, size_t N, bool cross> class TypeBuilder<T[N], cross> {
>  public:
>   static const ArrayType *get(LLVMContext &Context) {
> -    static const ArrayType *const result =
> -    ArrayType::get(TypeBuilder<T, cross>::get(Context), N);
> -    return result;
> +    return ArrayType::get(TypeBuilder<T, cross>::get(Context), N);
>   }
>  };
>  /// LLVM uses an array of length 0 to represent an unknown-length array.
>  template<typename T, bool cross> class TypeBuilder<T[], cross> {
>  public:
>   static const ArrayType *get(LLVMContext &Context) {
> -    static const ArrayType *const result =
> -      ArrayType::get(TypeBuilder<T, cross>::get(Context), 0);
> -    return result;
> +    return ArrayType::get(TypeBuilder<T, cross>::get(Context), 0);
>   }
>  };
>
> @@ -160,9 +150,7 @@
>  template<> class TypeBuilder<T, false> { \
>  public: \
>   static const IntegerType *get(LLVMContext &Context) { \
> -    static const IntegerType *const result = \
> -      IntegerType::get(Context, sizeof(T) * CHAR_BIT); \
> -    return result; \
> +    return IntegerType::get(Context, sizeof(T) * CHAR_BIT); \
>   } \
>  }; \
>  template<> class TypeBuilder<T, true> { \
> @@ -191,8 +179,7 @@
>  class TypeBuilder<types::i<num_bits>, cross> {
>  public:
>   static const IntegerType *get(LLVMContext &C) {
> -    static const IntegerType *const result = IntegerType::get(C, num_bits);
> -    return result;
> +    return IntegerType::get(C, num_bits);
>   }
>  };
>
> @@ -248,24 +235,12 @@
>  template<typename R, bool cross> class TypeBuilder<R(), cross> {
>  public:
>   static const FunctionType *get(LLVMContext &Context) {
> -    static const FunctionType *const result = create(Context);
> -    return result;
> -  }
> -
> -private:
> -  static const FunctionType *create(LLVMContext &Context) {
>     return FunctionType::get(TypeBuilder<R, cross>::get(Context), false);
>   }
>  };
>  template<typename R, typename A1, bool cross> class TypeBuilder<R(A1), cross> {
>  public:
>   static const FunctionType *get(LLVMContext &Context) {
> -    static const FunctionType *const result = create(Context);
> -    return result;
> -  }
> -
> -private:
> -  static const FunctionType *create(LLVMContext &Context) {
>     std::vector<const Type*> params;
>     params.reserve(1);
>     params.push_back(TypeBuilder<A1, cross>::get(Context));
> @@ -277,12 +252,6 @@
>  class TypeBuilder<R(A1, A2), cross> {
>  public:
>   static const FunctionType *get(LLVMContext &Context) {
> -    static const FunctionType *const result = create(Context);
> -    return result;
> -  }
> -
> -private:
> -  static const FunctionType *create(LLVMContext &Context) {
>     std::vector<const Type*> params;
>     params.reserve(2);
>     params.push_back(TypeBuilder<A1, cross>::get(Context));
> @@ -295,12 +264,6 @@
>  class TypeBuilder<R(A1, A2, A3), cross> {
>  public:
>   static const FunctionType *get(LLVMContext &Context) {
> -    static const FunctionType *const result = create(Context);
> -    return result;
> -  }
> -
> -private:
> -  static const FunctionType *create(LLVMContext &Context) {
>     std::vector<const Type*> params;
>     params.reserve(3);
>     params.push_back(TypeBuilder<A1, cross>::get(Context));
> @@ -316,12 +279,6 @@
>  class TypeBuilder<R(A1, A2, A3, A4), cross> {
>  public:
>   static const FunctionType *get(LLVMContext &Context) {
> -    static const FunctionType *const result = create(Context);
> -    return result;
> -  }
> -
> -private:
> -  static const FunctionType *create(LLVMContext &Context) {
>     std::vector<const Type*> params;
>     params.reserve(4);
>     params.push_back(TypeBuilder<A1, cross>::get(Context));
> @@ -338,12 +295,6 @@
>  class TypeBuilder<R(A1, A2, A3, A4, A5), cross> {
>  public:
>   static const FunctionType *get(LLVMContext &Context) {
> -    static const FunctionType *const result = create(Context);
> -    return result;
> -  }
> -
> -private:
> -  static const FunctionType *create(LLVMContext &Context) {
>     std::vector<const Type*> params;
>     params.reserve(5);
>     params.push_back(TypeBuilder<A1, cross>::get(Context));
> @@ -359,12 +310,6 @@
>  template<typename R, bool cross> class TypeBuilder<R(...), cross> {
>  public:
>   static const FunctionType *get(LLVMContext &Context) {
> -    static const FunctionType *const result = create(Context);
> -    return result;
> -  }
> -
> -private:
> -  static const FunctionType *create(LLVMContext &Context) {
>     return FunctionType::get(TypeBuilder<R, cross>::get(Context), true);
>   }
>  };
> @@ -372,12 +317,6 @@
>  class TypeBuilder<R(A1, ...), cross> {
>  public:
>   static const FunctionType *get(LLVMContext &Context) {
> -    static const FunctionType *const result = create(Context);
> -    return result;
> -  }
> -
> -private:
> -  static const FunctionType *create(LLVMContext &Context) {
>     std::vector<const Type*> params;
>     params.reserve(1);
>     params.push_back(TypeBuilder<A1, cross>::get(Context));
> @@ -388,12 +327,6 @@
>  class TypeBuilder<R(A1, A2, ...), cross> {
>  public:
>   static const FunctionType *get(LLVMContext &Context) {
> -    static const FunctionType *const result = create(Context);
> -    return result;
> -  }
> -
> -private:
> -  static const FunctionType *create(LLVMContext &Context) {
>     std::vector<const Type*> params;
>     params.reserve(2);
>     params.push_back(TypeBuilder<A1, cross>::get(Context));
> @@ -406,12 +339,6 @@
>  class TypeBuilder<R(A1, A2, A3, ...), cross> {
>  public:
>   static const FunctionType *get(LLVMContext &Context) {
> -    static const FunctionType *const result = create(Context);
> -    return result;
> -  }
> -
> -private:
> -  static const FunctionType *create(LLVMContext &Context) {
>     std::vector<const Type*> params;
>     params.reserve(3);
>     params.push_back(TypeBuilder<A1, cross>::get(Context));
> @@ -427,12 +354,6 @@
>  class TypeBuilder<R(A1, A2, A3, A4, ...), cross> {
>  public:
>   static const FunctionType *get(LLVMContext &Context) {
> -    static const FunctionType *const result = create(Context);
> -    return result;
> -  }
> -
> -private:
> -  static const FunctionType *create(LLVMContext &Context) {
>     std::vector<const Type*> params;
>     params.reserve(4);
>     params.push_back(TypeBuilder<A1, cross>::get(Context));
> @@ -449,12 +370,6 @@
>  class TypeBuilder<R(A1, A2, A3, A4, A5, ...), cross> {
>  public:
>   static const FunctionType *get(LLVMContext &Context) {
> -    static const FunctionType *const result = create(Context);
> -    return result;
> -  }
> -
> -private:
> -  static const FunctionType *create(LLVMContext &Context) {
>     std::vector<const Type*> params;
>     params.reserve(5);
>     params.push_back(TypeBuilder<A1, cross>::get(Context));
>
> Modified: llvm/trunk/unittests/Support/TypeBuilderTest.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/TypeBuilderTest.cpp?rev=81316&r1=81315&r2=81316&view=diff
>
> ==============================================================================
> --- llvm/trunk/unittests/Support/TypeBuilderTest.cpp (original)
> +++ llvm/trunk/unittests/Support/TypeBuilderTest.cpp Wed Sep  9 00:04:01 2009
> @@ -147,6 +147,18 @@
>                          false>::get(getGlobalContext())));
>  }
>
> +TEST(TypeBuilderTest, Context) {
> +  // We used to cache TypeBuilder results in static local variables.  This
> +  // produced the same type for different contexts, which of course broke
> +  // things.
> +  LLVMContext context1;
> +  EXPECT_EQ(&context1,
> +            &(TypeBuilder<types::i<1>, true>::get(context1))->getContext());
> +  LLVMContext context2;
> +  EXPECT_EQ(&context2,
> +            &(TypeBuilder<types::i<1>, true>::get(context2))->getContext());
> +}
> +
>  class MyType {
>   int a;
>   int *b;
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>




More information about the llvm-commits mailing list