Other approach would be to switch from enum-based opaque types to
class-based opaque-types.

But that would require some good refactoring for Code::Flags.

--
Vyacheslav Egorov


On Mon, Aug 16, 2010 at 4:12 PM, <[email protected]> wrote:

> Reviewers: Mads Ager,
>
> Description:
> Give correct value ranges to enumeration types used as opaque types.
>
> This allows to remove special handling of GCC 4.4 (disabling of Value Range
> Propagation) from SConstruct.
>
> BUG=http://code.google.com/p/v8/issues/detail?id=830
>
> Please review this at http://codereview.chromium.org/3135022/show
>
> Affected files:
>  M SConstruct
>  M src/frames.h
>  M src/objects.h
>  M src/platform.h
>
>
> Index: SConstruct
> diff --git a/SConstruct b/SConstruct
> index
> 0abaeed7752200bd7ce93b16edc3f7d4a879c63d..2a39583f1c3e27fffc4cbbe920ed68d2443fe49e
> 100644
> --- a/SConstruct
> +++ b/SConstruct
> @@ -54,15 +54,8 @@ if ARM_TARGET_LIB:
>  else:
>   ARM_LINK_FLAGS = []
>
> -# TODO: Sort these issues out properly but as a temporary solution for gcc
> 4.4
> -# on linux we need these compiler flags to avoid crashes in the v8 test
> suite
> -# and avoid dtoa.c strict aliasing issues
> -if os.environ.get('GCC_VERSION') == '44':
> -    GCC_EXTRA_CCFLAGS = ['-fno-tree-vrp']
> -    GCC_DTOA_EXTRA_CCFLAGS = []
> -else:
> -    GCC_EXTRA_CCFLAGS = []
> -    GCC_DTOA_EXTRA_CCFLAGS = []
> +GCC_EXTRA_CCFLAGS = []
> +GCC_DTOA_EXTRA_CCFLAGS = []
>
>  ANDROID_FLAGS = ['-march=armv7-a',
>                  '-mtune=cortex-a8',
> Index: src/frames.h
> diff --git a/src/frames.h b/src/frames.h
> index
> 102244c9ba2f81879b823b69ebfe4fd6c46fa324..f89b07dae51d8a8225673ca6c529a457cf1255f8
> 100644
> --- a/src/frames.h
> +++ b/src/frames.h
> @@ -112,7 +112,9 @@ class StackFrame BASE_EMBEDDED {
>
>   // Opaque data type for identifying stack frames. Used extensively
>   // by the debugger.
> -  enum Id { NO_ID = 0 };
> +  // kIdMinValue and kIdMaxValue are specified to ensure that enumeration
> type
> +  // has correct value range (see Issue 830 for more details).
> +  enum Id { kIdMinValue = kMinInt, kIdMaxValue = kMaxInt, NO_ID = 0 };
>
>   // Copy constructor; it breaks the connection to host iterator.
>   StackFrame(const StackFrame& original) {
> Index: src/objects.h
> diff --git a/src/objects.h b/src/objects.h
> index
> d2f6d3559bf99d37b31b448a1e777a76d933002b..24e3bcd9b3ee2b26dc23e6f6d840ad9f4d3d82c0
> 100644
> --- a/src/objects.h
> +++ b/src/objects.h
> @@ -2762,7 +2762,11 @@ class Code: public HeapObject {
>  public:
>   // Opaque data type for encapsulating code flags like kind, inline
>   // cache state, and arguments count.
> -  enum Flags { };
> +  // kFlagsMinValue and kFlagsMaxValue are specified to ensure that
> +  // enumeration type has correct value range (see Issue 830 for more
> details).
> +  enum Flags {
> +    kFlagsMinValue = kMinInt, kFlagsMaxValue = kMaxInt
> +  };
>
>   enum Kind {
>     FUNCTION,
> Index: src/platform.h
> diff --git a/src/platform.h b/src/platform.h
> index
> d63ca5e617358f98510404b9abc9fd89af2bcdb4..1f8c641df614b6dbe2ab9be7972a3b30b9e276bc
> 100644
> --- a/src/platform.h
> +++ b/src/platform.h
> @@ -360,7 +360,12 @@ class ThreadHandle {
>  class Thread: public ThreadHandle {
>  public:
>   // Opaque data type for thread-local storage keys.
> -  enum LocalStorageKey {};
> +  // kLocalStorageKeyMinValue and kLocalStorageKeyMaxValue are specified
> +  // to ensure that enumeration type has correct value range (see Issue
> 830 for
> +  // more details).
> +  enum LocalStorageKey {
> +    kLocalStorageKeyMinValue = kMinInt, kLocalStorageKeyMaxValue = kMaxInt
> +  };
>
>   // Create new thread.
>   Thread();
>
>
>

-- 
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to