1410765 - Three tiny libpref tweaks

Status: RESOLVED FIXED

(Core :: Preferences: Backend, enhancement)

Description

[Nicholas Nethercote [inactive]]

A tiny clean-up.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: Remove unused PrefTypeFlags values

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: (part 1) - Remove unused PrefTypeFlags values

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: (part 2) - Redo clang-format on libpref/

Comment 4

[Nicholas Nethercote [inactive]]

Attachment: (part 3) - Remove nsPrefBranch::RemoveObserverFromMap declaration

Comment 5

[Mike Hommey [:glandium]]

Comment on attachment 8920924 [details] [diff] [review]
(part 1) - Remove unused PrefTypeFlags values

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

::: modules/libpref/Preferences.cpp
@@ -244,5 @@
>      PREF_FLAG_LOCKED = 4,
>      PREF_FLAG_USERSET = 8,
> -    PREF_FLAG_CONFIG = 16,
> -    PREF_FLAG_REMOTE = 32,
> -    PREF_FLAG_LILOCAL = 64,

FWIW, they were effectively removed in bug 99611.

Comment 6

[Mike Hommey [:glandium]]

Comment on attachment 8920933 [details] [diff] [review]
(part 3) - Remove nsPrefBranch::RemoveObserverFromMap declaration

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

Static analysis for this kind of stuff could be interesting (declared methods that we never define).

Comment 7

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/69b08044cafb8a2cf2e2426382084bbd9279137f
Bug 1410765 (part 1) - Remove unused PrefTypeFlags values. r=glandium.

https://hg.mozilla.org/integration/mozilla-inbound/rev/37eb7a2fb6ac6c897f8bccbf0e103a3e2b40159c
Bug 1410765 (part 2) - Redo clang-format on libpref/. r=glandium.

https://hg.mozilla.org/integration/mozilla-inbound/rev/71a01e882f44c3143b9b2dd777f988f58e7af862
Bug 1410765 (part 3) - Remove nsPrefBranch::RemoveObserverFromMap declaration. r=glandium.

Comment 8

[Nicholas Nethercote [inactive]]

> Static analysis for this kind of stuff could be interesting (declared
> methods that we never define).

Sylvestre, do you know if a clang static analysis could be written to detect methods that are declared but never defined? (Like the one removed by part 3 above.)

Comment 9

[Nicholas Nethercote [inactive]]

(See comment 8)

Comment 10

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

https://hg.mozilla.org/mozilla-central/rev/69b08044cafb
https://hg.mozilla.org/mozilla-central/rev/37eb7a2fb6ac
https://hg.mozilla.org/mozilla-central/rev/71a01e882f44

Comment 11

[Andi [:andi]]

(In reply to Nicholas Nethercote [:njn] from comment #8)
> > Static analysis for this kind of stuff could be interesting (declared
> > methods that we never define).
> 
> Sylvestre, do you know if a clang static analysis could be written to detect
> methods that are declared but never defined? (Like the one removed by part 3
> above.)

This would have several issues:
1. Possibility of external addressing of functions, that is resolved only during linkage. So in a compilation unit we would only define functions that would be externally called by other compilation units.
2. The impossibility of clang to match decl and call simultaneously, in an ast tree, this would have to be done only by transvering manually the tree by matching for each declaration all of the funcCalls and then triage them by signature.
3. There is the possibility to also call functions by pointer to functions or pointers to member function that again will complicate the resolution and would generate a lot of false positives.

Having all of this issues I don't see the real gaining of having this kind of checker, only the fact that we might reduce TXT size.

Comment 12

[Nicholas Nethercote [inactive]]

Fair enough!

Comment 13

[Sylvestre Ledru [:Sylvestre]]

I asked Andi to answered about that :) Merci Andi!