1411552 - Improve the warning about attempting to overwrite prefs with a pref of the wrong type

Status: RESOLVED FIXED

(Core :: Preferences: Backend, enhancement)

Description

[Jonathan Watt [:jwatt]]

The warning is currently in the form "Trying to overwrite value of default pref %s with the wrong type". This could be improved by:

 a) making it clear this attempt will be IGNORED
 b) mentioning the types involved

Comment 1

[Jonathan Watt [:jwatt]]

Attachment: patch

Comment 2

[Nicholas Nethercote [inactive]]

Comment on attachment 8921847 [details] [diff] [review]
patch

Review of attachment 8921847 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with the nits below addressed.

Also, I've reformatted libpref/ with clang-format, so please do that (`./mach clang-format` will do the current patch) if you haven't already.

::: modules/libpref/Preferences.cpp
@@ +164,5 @@
>    Bool = 3,
>  };
>  
> +#ifdef DEBUG
> +const char* PrefTypeToString(PrefType aType)

Return type on its own line (clang-format should do that for you).

@@ +166,5 @@
>  
> +#ifdef DEBUG
> +const char* PrefTypeToString(PrefType aType)
> +{
> +  static const char* kInvalidStr = "INVALID";

No need for the named constants, just do `return "INVALID"`, etc., below.

@@ +181,5 @@
> +    return kIntStr;
> +  case PrefType::Bool:
> +    return kBoolStr;
> +  default:
> +    MOZ_ASSERT_UNREACHABLE("Unhandled enum value");

The comment for MOZ_ASSERT_UNREACHABLE says

> Unconditional assert in debug builds for (assumed) unreachable code paths
> that have a safe return without crashing in release builds.

The code currently doesn't have a safe return in release builds. You could either add `return "???";` after the MOZ_ASSERT_UNREACHABLE, or you could change it to MOZ_CRASH.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/49451ebb746c
Improve the warning about attempting to overwrite prefs with a pref of the wrong type. r=njn

Comment 4

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/49451ebb746c