swb: simplify write barrier macros

Apply template traits to field types to simplify write barrier field
annotations. Now most annotations are Field or FieldNoBarrier (for pointers
known not needing write barrier).

Replaced `WB_ITEM_TYPE` with `Field`.

Author: Jianchun Xu

lib/Common/DataStructures/BaseDictionary.h

@@ -81,8 +81,8 @@ namespace JsUtil
         typedef typename AllocatorInfo<TAllocator, TValue>::AllocatorType AllocatorType;
         typedef SizePolicy CurrentSizePolicy;
         typedef Entry<
-                    WB_ITEM_TYPE(TKey, TAllocator),
-                    WB_ITEM_TYPE(TValue, TAllocator)> EntryType;
+                    Field(TKey, TAllocator),
+                    Field(TValue, TAllocator)> EntryType;
 
         template<class TDictionary> class EntryIterator;
         template<class TDictionary> class BucketEntryIterator;
@@ -92,17 +92,17 @@ namespace JsUtil
         friend class Js::RemoteDictionary<BaseDictionary>;
         template <typename ValueOrKey> struct ComparerType { typedef Comparer<ValueOrKey> Type; }; // Used by diagnostics to access Comparer type
 
-        Pointer(int, TAllocator) buckets;
-        Pointer(EntryType, TAllocator) entries;
-        PointerNoBarrier(AllocatorType) alloc;
+        Field(int*, TAllocator) buckets;
+        Field(EntryType*, TAllocator) entries;
+        FieldNoBarrier(AllocatorType*) alloc;
         Field(int) size;
         Field(uint) bucketCount;
         Field(int) count;
         Field(int) freeList;
         Field(int) freeCount;
 
 #if PROFILE_DICTIONARY
-        PointerNoBarrier(DictionaryStats) stats;
+        FieldNoBarrier(DictionaryStats*) stats;
 #endif
         enum InsertOperations
         {
@@ -1569,7 +1569,7 @@ namespace JsUtil
     class SynchronizedDictionary: protected BaseDictionary<TKey, TValue, TAllocator, SizePolicy, Comparer, Entry>
     {
     private:
-        PointerNoBarrier(SyncObject) syncObj;
+        FieldNoBarrier(SyncObject*) syncObj;
 
         typedef BaseDictionary<TKey, TValue, TAllocator, SizePolicy, Comparer, Entry> Base;
     public:

lib/Common/Memory/RecyclerPointers.h

@@ -63,40 +63,17 @@ struct AllocatorWriteBarrierPolicy<RecyclerNonLeafAllocator, T> { typedef _write
 template <>
 struct AllocatorWriteBarrierPolicy<RecyclerNonLeafAllocator, int> { typedef _no_write_barrier_policy Policy; };
 
-// Choose WriteBarrierPtr or NoWriteBarrierPtr based on Policy
+// Choose write barrier Field type: T unchanged, or WriteBarrierPtr based on Policy.
 //
 template <class T, class Policy>
-struct _WriteBarrierPtrPolicy { typedef NoWriteBarrierPtr<T> Ptr; };
+struct _WriteBarrierFieldType { typedef T Type; };
 template <class T>
-struct _WriteBarrierPtrPolicy<T, _write_barrier_policy> { typedef WriteBarrierPtr<T> Ptr; };
+struct _WriteBarrierFieldType<T*, _write_barrier_policy> { typedef WriteBarrierPtr<T> Type; };
 
-// Choose WriteBarrierPtr or NoWriteBarrierPtr based on Allocator and T* type
-//
 template <class T,
           class Allocator = Recycler,
-          class Policy = typename AllocatorWriteBarrierPolicy<Allocator, T*>::Policy>
-struct WriteBarrierPtrTraits { typedef typename _WriteBarrierPtrPolicy<T, Policy>::Ptr Ptr; };
-
-// Choose WriteBarrierPtr type if Allocator is recycler type and element type T
-// is a pointer type, otherwise use type T unchanged.
-//
-// Used to wrap array item type when write barrier is needed (wraps pointer
-// item type with WriteBarrierPtr).
-//
-template <class T, class Policy>
-struct _ArrayItemTypeTraits
-{
-    typedef T Type;
-};
-template <class T>
-struct _ArrayItemTypeTraits<T*, _write_barrier_policy>
-{
-    typedef WriteBarrierPtr<T> Type;
-};
-template <class T,
-          class Allocator,
           class Policy = typename AllocatorWriteBarrierPolicy<Allocator, T>::Policy>
-struct WriteBarrierArrayItemTraits { typedef typename _ArrayItemTypeTraits<T, Policy>::Type Type; };
+struct WriteBarrierFieldTypeTraits { typedef typename _WriteBarrierFieldType<T, Policy>::Type Type; };
 
 // ArrayWriteBarrier behavior
 //

lib/Common/Memory/WriteBarrierMacros.h

@@ -28,38 +28,30 @@
 
 #define SAVE_WRITE_BARRIER_MACROS() \
     PushMacro("Field") \
-    PushMacro("PointerNoBarrier") \
-    PushMacro("Pointer") \
-    PushMacro("WB_ITEM_TYPE")
+    PushMacro("FieldNoBarrier")
 
 #define RESTORE_WRITE_BARRIER_MACROS() \
     PopMacro("Field") \
-    PopMacro("PointerNoBarrier") \
-    PopMacro("Pointer") \
-    PopMacro("WB_ITEM_TYPE")
+    PopMacro("FieldNoBarrier")
 
 #endif
 
 #ifdef FORCE_USE_WRITE_BARRIER
 SAVE_WRITE_BARRIER_MACROS()
 #undef Field
-#undef PointerNoBarrier
-#undef Pointer
-#undef WB_ITEM_TYPE
+#undef FieldNoBarrier
 #endif
 
 // TODO: Turn off these annotations on Win32
 #if defined(__clang__) || defined(FORCE_USE_WRITE_BARRIER)
 // Various macros for defining field attributes
-#define Field(type) NoWriteBarrierField<type>
-#define PointerNoBarrier(type) NoWriteBarrierPtr<type>
-#define Pointer(type, ...) typename WriteBarrierPtrTraits<type, ##__VA_ARGS__>::Ptr
-#define WB_ITEM_TYPE(type, ...) typename WriteBarrierArrayItemTraits<type, ##__VA_ARGS__>::Type
+#define Field(type, ...) \
+    typename WriteBarrierFieldTypeTraits<type, ##__VA_ARGS__>::Type
+#define FieldNoBarrier(type) \
+    typename WriteBarrierFieldTypeTraits<type, _no_write_barrier_policy, _no_write_barrier_policy>::Type
 #else
-#define Field(type) type
-#define PointerNoBarrier(type) type*
-#define Pointer(type, ...) type*
-#define WB_ITEM_TYPE(type, ...) type
+#define Field(type, ...) type
+#define FieldNoBarrier(type) type
 #endif
 
 #undef FORCE_USE_WRITE_BARRIER

lib/Runtime/Base/AuxPtrs.h

@@ -28,7 +28,7 @@ namespace Js
         static const uint8 MaxCount;
         Field(uint8) count;                 // always saving maxCount
         Field(FieldsEnum) type[_MaxCount];  // save instantiated pointer enum
-        Pointer(void) ptr[_MaxCount];       // save instantiated pointer address
+        Field(void*) ptr[_MaxCount];        // save instantiated pointer address
         AuxPtrsFix();
         AuxPtrsFix(AuxPtrsFix<FieldsEnum, 16>* ptr16); // called when promoting from AuxPtrs16 to AuxPtrs32
         void* Get(FieldsEnum e);
@@ -53,7 +53,7 @@ namespace Js
         Field(uint8) count;                                      // save instantiated pointers count
         Field(uint8) capacity;                                   // save number of pointers can be hold in current instance of AuxPtrs
         Field(uint8) offsets[static_cast<int>(FieldsEnum::Max)]; // save position of each instantiated pointers, if not instantiate, it's invalid
-        Pointer(void) ptrs[1];                                   // instantiated pointer addresses
+        Field(void*) ptrs[1];                                    // instantiated pointer addresses
         AuxPtrs(uint8 capacity, AuxPtrs32* ptr32);               // called when promoting from AuxPtrs32 to AuxPtrs
         AuxPtrs(uint8 capacity, AuxPtrs* ptr);                   // called when expanding (i.e. promoting from AuxPtrs to bigger AuxPtrs)
         void* Get(FieldsEnum e);

lib/Runtime/Base/CompactCounters.h

@@ -37,7 +37,7 @@ namespace Js
         mutable Field(bool) bgThreadCallStarted;
         Field(bool) isCleaningUp;
 #endif
-        Pointer(Fields) fields;
+        Field(Fields*) fields;
 
         CompactCounters() { }
         CompactCounters(T* host)

lib/Runtime/Base/FunctionBody.h

@@ -1351,7 +1351,7 @@ namespace Js
 
         typedef AuxPtrs<FunctionProxy, AuxPointerType> AuxPtrsT;
         friend AuxPtrsT;
-        Pointer(AuxPtrsT) auxPtrs;
+        Field(AuxPtrsT*) auxPtrs;
         void* GetAuxPtr(AuxPointerType e) const;
         void* GetAuxPtrWithLock(AuxPointerType e) const;
         void SetAuxPtr(AuxPointerType e, void* ptr);
@@ -1451,17 +1451,17 @@ namespace Js
 
     protected:
         // Static method(s)
-        static void SetDisplayName(const char16* srcName, Pointer(const char16)* destName, uint displayNameLength, ScriptContext * scriptContext, SetDisplayNameFlags flags = SetDisplayNameFlagsNone);
+        static void SetDisplayName(const char16* srcName, Field(const char16*)* destName, uint displayNameLength, ScriptContext * scriptContext, SetDisplayNameFlags flags = SetDisplayNameFlagsNone);
         static bool SetDisplayName(const char16* srcName, const char16** destName, uint displayNameLength, ScriptContext * scriptContext, SetDisplayNameFlags flags = SetDisplayNameFlagsNone);
         static bool IsConstantFunctionName(const char16* srcName);
 
     protected:
-        PointerNoBarrier(ScriptContext)  m_scriptContext;   // Memory context for this function body
-        Pointer(Utf8SourceInfo) m_utf8SourceInfo;
+        FieldNoBarrier(ScriptContext*) m_scriptContext;   // Memory context for this function body
+        Field(Utf8SourceInfo*) m_utf8SourceInfo;
         // WriteBarrier-TODO: Consider changing this to NoWriteBarrierPtr, and skip tagging- also, tagging is likely unnecessary since that pointer in question is likely not resolvable
-        PointerNoBarrier(FunctionProxyPtr) m_referenceInParentFunction; // Reference to nested function reference to this function in the parent function body (tagged to not be actual reference)
-        Pointer(ScriptFunctionType) deferredPrototypeType;
-        Pointer(ProxyEntryPointInfo) m_defaultEntryPointInfo; // The default entry point info for the function proxy
+        FieldNoBarrier(FunctionProxyPtr*) m_referenceInParentFunction; // Reference to nested function reference to this function in the parent function body (tagged to not be actual reference)
+        Field(ScriptFunctionType*) deferredPrototypeType;
+        Field(ProxyEntryPointInfo*) m_defaultEntryPointInfo; // The default entry point info for the function proxy
 
         Field(uint) m_functionNumber;  // Per thread global function number
 
@@ -1771,7 +1771,7 @@ namespace Js
         Field(bool) m_utf8SourceHasBeenSet;          // start of UTF8-encoded source
         Field(uint) m_sourceIndex;             // index into the scriptContext's list of saved sources
 #if DYNAMIC_INTERPRETER_THUNK
-        PointerNoBarrier(void) m_dynamicInterpreterThunk;  // Unique 'thunk' for every interpreted function - used for ETW symbol decoding.
+        FieldNoBarrier(void*) m_dynamicInterpreterThunk;  // Unique 'thunk' for every interpreted function - used for ETW symbol decoding.
 #endif
         Field(uint) m_cbStartOffset;         // pUtf8Source is this many bytes from the start of the scriptContext's source buffer.
 
@@ -1783,10 +1783,10 @@ namespace Js
 
         ULONG m_lineNumber;
         ULONG m_columnNumber;
-        Pointer(const char16) m_displayName;  // Optional name
+        Field(const char16*) m_displayName;  // Optional name
         uint m_displayNameLength;
-        Pointer(PropertyRecordList) m_boundPropertyRecords;
-        Pointer(NestedArray) nestedArray;
+        Field(PropertyRecordList*) m_boundPropertyRecords;
+        Field(NestedArray*) nestedArray;
 
     public:
 #if DBG
@@ -1988,16 +1988,16 @@ namespace Js
                 friend class ByteCodeBufferBuilder;
 
             public:
-                PointerNoBarrier(SmallSpanSequence) pSpanSequence;
+                FieldNoBarrier(SmallSpanSequence*) pSpanSequence;
 
                 RegSlot         frameDisplayRegister;   // this register slot cannot be 0 so we use that sentinel value to indicate invalid
                 RegSlot         objectRegister;         // this register slot cannot be 0 so we use that sentinel value to indicate invalid
-                Pointer(ScopeObjectChain) pScopeObjectChain;
-                Pointer(ByteBlock) m_probeBackingBlock; // NULL if no Probes, otherwise a copy of the unmodified the byte-codeblock //Delay
+                Field(ScopeObjectChain*) pScopeObjectChain;
+                Field(ByteBlock*) m_probeBackingBlock;  // NULL if no Probes, otherwise a copy of the unmodified the byte-codeblock //Delay
                 int32 m_probeCount;             // The number of installed probes (such as breakpoints).
 
                 // List of bytecode offset for the Branch bytecode.
-                Pointer(AuxStatementData) m_auxStatementData;
+                Field(AuxStatementData*) m_auxStatementData;
 
                 SourceInfo():
                     frameDisplayRegister(0),
@@ -2012,18 +2012,18 @@ namespace Js
             };
 
     private:
-        Pointer(ByteBlock) byteCodeBlock;               // Function byte-code for script functions
-        Pointer(FunctionEntryPointList) entryPoints;
-        Pointer(Var) m_constTable;
-        Pointer(void*) inlineCaches;
+        Field(ByteBlock*) byteCodeBlock;                // Function byte-code for script functions
+        Field(FunctionEntryPointList*) entryPoints;
+        Field(Var*) m_constTable;
+        Field(void**) inlineCaches;
         InlineCachePointerArray<PolymorphicInlineCache> polymorphicInlineCaches; // Contains the latest polymorphic inline caches
-        Pointer(PropertyId) cacheIdToPropertyIdMap;
+        Field(PropertyId*) cacheIdToPropertyIdMap;
 
 #if DBG
 #define InlineCacheTypeNone         0x00
 #define InlineCacheTypeInlineCache  0x01
 #define InlineCacheTypeIsInst       0x02
-            Pointer(byte) m_inlineCacheTypes;
+            Field(byte*) m_inlineCacheTypes;
 #endif
     public:
         PropertyId * GetCacheIdToPropertyIdMap()
@@ -2149,7 +2149,7 @@ namespace Js
 #ifdef IR_VIEWER
         // whether IR Dump is enabled for this function (used by parseIR)
         bool m_isIRDumpEnabled : 1;
-        Pointer(Js::DynamicObject) m_irDumpBaseObject;
+        Field(Js::DynamicObject*) m_irDumpBaseObject;
 #endif /* IR_VIEWER */
 
         Field(uint8) bailOnMisingProfileCount;
@@ -2181,14 +2181,14 @@ namespace Js
         // copied in FunctionBody::Clone
         //
 
-        PointerNoBarrier(Js::ByteCodeCache) byteCodeCache;  // Not GC allocated so naked pointer
+        FieldNoBarrier(Js::ByteCodeCache*) byteCodeCache;   // Not GC allocated so naked pointer
 #ifdef ENABLE_DEBUG_CONFIG_OPTIONS
         static bool shareInlineCaches;
 #endif
-        Pointer(FunctionEntryPointInfo) defaultFunctionEntryPointInfo;
+        Field(FunctionEntryPointInfo*) defaultFunctionEntryPointInfo;
 
 #if ENABLE_PROFILE_INFO
-        Pointer(DynamicProfileInfo) dynamicProfileInfo;
+        Field(DynamicProfileInfo*) dynamicProfileInfo;
 #endif
 
 

tools/RecyclerChecker/RecyclerChecker.cpp

@@ -14,11 +14,16 @@
 using namespace clang;
 using namespace std;
 
+template <size_t N>
+bool StartsWith(const string& s, const char (&prefix)[N])
+{
+    return s.compare(0, string::npos, prefix, N - 1) == 0;
+}
 
 namespace
 {
-
 class MainVisitor;
+
 class CheckAllocationsInFunctionVisitor :
         public RecursiveASTVisitor<CheckAllocationsInFunctionVisitor>
 {
@@ -80,8 +85,9 @@ class MainVisitor:
             else
             {
                 auto fieldTypeName = qualType.getAsString();
-                if (fieldTypeName.find("WriteBarrierPtr") == 0)  // starts with
+                if (StartsWith(fieldTypeName, "typename WriteBarrierFieldTypeTraits"))
                 {
+                    // Note this only indicates the class is write-barrier annotated
                     hasBarrieredField = true;
                 }
                 else if (type->isCompoundType())