Closed Bug 1448490 Opened 2 years ago Closed 2 years ago

Make layers id strongly-typed

Categories

(Core :: Graphics: Layers, enhancement)

Other Branch
enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: kats, Assigned: kats)

References

Details

Attachments

(1 file)

We have a bajillion different uint64_t ids being passed around all over the codebase, and it's very easy to accidentally screw up which one you're passing where. I'd like to introduce a LayersId struct type that allows us to get better compile-time safety.
Comment on attachment 8962105 [details]
Bug 1448490 - Make the layers id a struct instead of a uint64_t.

https://reviewboard.mozilla.org/r/230952/#review236398


Code analysis found 8 defects in this patch:
 - 8 defects found by clang-tidy

You can run this analysis locally with:
 - `./mach static-analysis check path/to/file.cpp` (C/C++)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: gfx/layers/apz/src/APZCTreeManager.cpp:224
(Diff revision 1)
>    FocusState& mFocusState;
>    InputData& mEvent;
>    bool mMayChangeFocus;
>  };
>  
> -APZCTreeManager::APZCTreeManager(uint64_t aRootLayersId)
> +APZCTreeManager::APZCTreeManager(LayersId aRootLayersId)

Error: Bad implicit conversion constructor for 'apzctreemanager' [clang-tidy: mozilla-implicit-constructor]

::: gfx/layers/apz/src/APZCTreeManager.cpp:224
(Diff revision 1)
>    FocusState& mFocusState;
>    InputData& mEvent;
>    bool mMayChangeFocus;
>  };
>  
> -APZCTreeManager::APZCTreeManager(uint64_t aRootLayersId)
> +APZCTreeManager::APZCTreeManager(LayersId aRootLayersId)

Error: Bad implicit conversion constructor for 'apzctreemanager' [clang-tidy: mozilla-implicit-constructor]

::: gfx/layers/apz/src/APZCTreeManager.cpp:224
(Diff revision 1)
>    FocusState& mFocusState;
>    InputData& mEvent;
>    bool mMayChangeFocus;
>  };
>  
> -APZCTreeManager::APZCTreeManager(uint64_t aRootLayersId)
> +APZCTreeManager::APZCTreeManager(LayersId aRootLayersId)

Error: Bad implicit conversion constructor for 'apzctreemanager' [clang-tidy: mozilla-implicit-constructor]

::: gfx/layers/apz/src/APZCTreeManager.cpp:224
(Diff revision 1)
>    FocusState& mFocusState;
>    InputData& mEvent;
>    bool mMayChangeFocus;
>  };
>  
> -APZCTreeManager::APZCTreeManager(uint64_t aRootLayersId)
> +APZCTreeManager::APZCTreeManager(LayersId aRootLayersId)

Error: Bad implicit conversion constructor for 'apzctreemanager' [clang-tidy: mozilla-implicit-constructor]

::: gfx/layers/apz/src/APZCTreeManager.cpp:224
(Diff revision 1)
>    FocusState& mFocusState;
>    InputData& mEvent;
>    bool mMayChangeFocus;
>  };
>  
> -APZCTreeManager::APZCTreeManager(uint64_t aRootLayersId)
> +APZCTreeManager::APZCTreeManager(LayersId aRootLayersId)

Error: Bad implicit conversion constructor for 'apzctreemanager' [clang-tidy: mozilla-implicit-constructor]

::: gfx/layers/apz/src/APZCTreeManager.cpp:224
(Diff revision 1)
>    FocusState& mFocusState;
>    InputData& mEvent;
>    bool mMayChangeFocus;
>  };
>  
> -APZCTreeManager::APZCTreeManager(uint64_t aRootLayersId)
> +APZCTreeManager::APZCTreeManager(LayersId aRootLayersId)

Error: Bad implicit conversion constructor for 'apzctreemanager' [clang-tidy: mozilla-implicit-constructor]

::: gfx/layers/ipc/UiCompositorControllerParent.cpp:238
(Diff revision 1)
>  {
>    MOZ_ASSERT(aSize > 0);
>    return AllocShmem(aSize, ipc::SharedMemory::TYPE_BASIC, aMem);
>  }
>  
> -UiCompositorControllerParent::UiCompositorControllerParent(const uint64_t& aRootLayerTreeId)
> +UiCompositorControllerParent::UiCompositorControllerParent(const LayersId& aRootLayerTreeId)

Error: Bad implicit conversion constructor for 'uicompositorcontrollerparent' [clang-tidy: mozilla-implicit-constructor]

::: gfx/layers/ipc/UiCompositorControllerParent.cpp:238
(Diff revision 1)
>  {
>    MOZ_ASSERT(aSize > 0);
>    return AllocShmem(aSize, ipc::SharedMemory::TYPE_BASIC, aMem);
>  }
>  
> -UiCompositorControllerParent::UiCompositorControllerParent(const uint64_t& aRootLayerTreeId)
> +UiCompositorControllerParent::UiCompositorControllerParent(const LayersId& aRootLayerTreeId)

Error: Bad implicit conversion constructor for 'uicompositorcontrollerparent' [clang-tidy: mozilla-implicit-constructor]
Comment on attachment 8962105 [details]
Bug 1448490 - Make the layers id a struct instead of a uint64_t.

https://reviewboard.mozilla.org/r/230952/#review236442
Attachment #8962105 - Flags: review?(matt.woodrow) → review+
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5d9f32557d77
Make the layers id a struct instead of a uint64_t. r=mattwoodrow
https://hg.mozilla.org/mozilla-central/rev/5d9f32557d77
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fd0e344e9bce
Follow-up to fix compilation of disabled-by-default logging. r=me and DONTBUILD
You need to log in before you can comment on or make changes to this bug.