Subzero: Reduce the amount of #ifdef'd code.

src/IceAssembler.h

Index: src/IceAssembler.h
diff --git a/src/IceAssembler.h b/src/IceAssembler.h
index 69faa95f98809635ecb66ffede847d95310957a4..252deeb1bad16f2baa47c89f64f2e94677573bc2 100644
--- a/src/IceAssembler.h
+++ b/src/IceAssembler.h
@@ -63,49 +63,45 @@ public:
   intptr_t size() const { return Cursor - Contents; }
   uintptr_t contents() const { return Contents; }
 
-// To emit an instruction to the assembler buffer, the EnsureCapacity helper
-// must be used to guarantee that the underlying data area is big enough to
-// hold the emitted instruction. Usage:
-//
-//     AssemblerBuffer buffer;
-//     AssemblerBuffer::EnsureCapacity ensured(&buffer);
-//     ... emit bytes for single instruction ...
+  // To emit an instruction to the assembler buffer, the EnsureCapacity helper
+  // must be used to guarantee that the underlying data area is big enough to
+  // hold the emitted instruction. Usage:
+  //
+  //     AssemblerBuffer buffer;
+  //     AssemblerBuffer::EnsureCapacity ensured(&buffer);
+  //     ... emit bytes for single instruction ...
 
-#ifndef NDEBUG
   class EnsureCapacity {
     EnsureCapacity(const EnsureCapacity &) = delete;
     EnsureCapacity &operator=(const EnsureCapacity &) = delete;
 
   public:
-    explicit EnsureCapacity(AssemblerBuffer *Buffer);
+    explicit EnsureCapacity(AssemblerBuffer *Buffer) : Buffer(Buffer), Gap(0) {
+      if (Buffer->cursor() >= Buffer->limit())
+        Buffer->extendCapacity();
+#ifdef NDEBUG

[Karl, 2015/06/22 15:41:06]

Why not create a boolean (inline) function in IceDefs.h, and then use

if (IsNdebug()) return;

Or, create a constexpr and use that as the test?

[John, 2015/06/22 17:13:46]

I agree with Karl on this one. Perhaps IceDefs could have something like:

constexpr bool nDebug() {
#if NDEBUG
  return true;
#else
  return false;
#endif  // NDEBUG
}

constexpr bool allowDump() {
if ALLOW_DUMP
  return true;
#else
  return false;
#endif  // ALLOW_DUMP
}

so that we could avoid these macros everywhere.

Additionally, it would be nice to have something like


class BuildTraits {
private:
  template <class Debug, class Release, bool>
  class TypePickerImpl {
  public:
    using Type = Release;
  };

  template <class Debug, class Release>
  class TypePickerImpl<Debug, Release, false> {
  public:
    using Type = Debug;
  };

public:
  static constexpr bool ReleaseBuild = nDebug();
  static constexpr bool AllowDump = allowDump();

  // ReleaseOrDebug is bad... perhaps SecondIfDebug?
  template <class Release, class Debug>
  using ReleaseOrDebug = typename
      TypePickerImpl<Debug, Release, ReleaseBuild>::Type;
};

Which means the ifs would become

if (BuildTraits::AllowDump) { /* do work */}

[Jim Stichnoth, 2015/06/23 21:42:07]

Did both, in IceDefs.h:
  constexpr bool buildAllowsAsserts() { ... }
  constexpr bool buildAllowsExtraValidation() { ... }
Also:
  constexpr bool buildAllowsDump() { ... }

+      return;
+#endif // NDEBUG
+      validate(Buffer);
+    }
     ~EnsureCapacity();
 
   private:
     AssemblerBuffer *Buffer;
     intptr_t Gap;
 
+    void validate(AssemblerBuffer *Buffer);
     intptr_t computeGap() { return Buffer->capacity() - Buffer->size(); }
   };
 
   bool HasEnsuredCapacity;
-  bool hasEnsuredCapacity() const { return HasEnsuredCapacity; }
-#else  // NDEBUG
-  class EnsureCapacity {
-    EnsureCapacity(const EnsureCapacity &) = delete;
-    EnsureCapacity &operator=(const EnsureCapacity &) = delete;
-
-  public:
-    explicit EnsureCapacity(AssemblerBuffer *Buffer) {
-      if (Buffer->cursor() >= Buffer->limit())
-        Buffer->extendCapacity();
-    }
-  };
-
-  // When building the C++ tests, assertion code is enabled. To allow
-  // asserting that the user of the assembler buffer has ensured the
-  // capacity needed for emitting, we add a dummy method in non-debug mode.
-  bool hasEnsuredCapacity() const { return true; }
+  bool hasEnsuredCapacity() const {
+#ifdef NDEBUG

[Karl, 2015/06/22 15:41:06]

same here.

[John, 2015/06/22 17:13:46]

This makes me nervous that, with NDEBUG, some compiler might complain about
unreachable code:

return true;
return HasEnsuredCapacity;

+    // Disable the actual check in non-debug mode.
+    return true;
 #endif // NDEBUG
+    return HasEnsuredCapacity;
+  }
 
   // Returns the position in the instruction stream.
   intptr_t getPosition() const { return Cursor - Contents; }
@@ -124,7 +120,7 @@ private:
   // The limit is set to kMinimumGap bytes before the end of the data area.
   // This leaves enough space for the longest possible instruction and allows
   // for a single, fast space check per instruction.
-  static const intptr_t kMinimumGap = 32;
+  static constexpr intptr_t kMinimumGap = 32;
 
   uintptr_t Contents;
   uintptr_t Cursor;