1648010 - Remove NS_*LITERAL_*STRING macros

Status: RESOLVED FIXED

(Core :: XPCOM, task)

Description

[Simon Giesecke [:sg] [he/him]]

The NS_*LITERAL_*STRING macros are no longer really necessary, and can be replaced by regular constructor calls to the corresponding nsLiteralString/nsLiteralCString classes.

In particular, the NS_NAMED_LITERAL_STRING and NS_NAMED_LITERAL_CSTRING macros obscure the declaration of constants.

Comment 1

[Simon Giesecke [:sg] [he/him]]

Attachment: Bug 1648010 - Remove NS_NAMED_LITERAL_CSTRING and NS_NAMED_LITERAL_STRING macros. r=#xpcom-reviewers

Comment 2

[Simon Giesecke [:sg] [he/him]]

Attachment: Bug 1648010 - Replace uses of NS_LITERAL_STRING/NS_LITERAL_CSTRING macros by _ns literals. r=#xpcom-reviewers

Depends on D80631

Comment 3

[Simon Giesecke [:sg] [he/him]]

Attachment: Bug 1648010 - Fix uses of NS_LITERAL_STRING with C string literals. r=#xpcom-reviewers

Depends on D80860

Comment 4

[Simon Giesecke [:sg] [he/him]]

Attachment: Bug 1648010 - Remove NS_LITERAL_STRING and NS_LITERAL_CSTRING macros. r=#xpcom-reviewers

Depends on D80861

Comment 5

[Simon Giesecke [:sg] [he/him]]

Attachment: Bug 1648010 - Add user-defined string literals. r=#xpcom-reviewers

Comment 6

[Simon Giesecke [:sg] [he/him]]

Instead of calling the constructor directly, user-defined literals can be used for a significantly more concise syntax.

Comment 7

[Simon Giesecke [:sg] [he/him]]

Attachment: Bug 1648010 - Update coding style with respect to string literals. r=#xpcom-reviewers

Depends on D80861

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d1416483de0d
Add user-defined string literals. r=froydnj

Comment 9

[Alexandru Michis [:malexandru]]

Backed out changeset d1416483de0d for causing bustages in nsTLiteralString.h

Backout link: https://hg.mozilla.org/integration/autoland/rev/ebb333a4a297259adbdc72cb40ebd488aee28818

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&selectedTaskRun=O66zbE1fQ6ajwT8GxGIYzQ.0&resultStatus=testfailed%2Cbusted%2Cexception%2Crunnable&revision=d1416483de0dd9b6070336e9a364bd6551b941b6

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=307865952&repo=autoland&lineNumber=5664

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9063f494163b
Add user-defined string literals. r=froydnj

Comment 11

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/9063f494163b

Comment 12

[Simon Giesecke [:sg] [he/him]]

Fixed and re-landed already.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3f8100fb7431
Replace uses of NS_LITERAL_STRING/NS_LITERAL_CSTRING macros by _ns literals. r=geckoview-reviewers,jgilbert,agi,hsivonen,froydnj
https://hg.mozilla.org/integration/autoland/rev/3d7ffaefcd5a
Fix uses of NS_LITERAL_STRING with C string literals. r=geckoview-reviewers,agi,froydnj
https://hg.mozilla.org/integration/autoland/rev/e52e5ee6f9d6
Remove NS_NAMED_LITERAL_CSTRING and NS_NAMED_LITERAL_STRING macros. r=froydnj

Comment 14

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/3f8100fb7431
https://hg.mozilla.org/mozilla-central/rev/3d7ffaefcd5a
https://hg.mozilla.org/mozilla-central/rev/e52e5ee6f9d6

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5f8a5717324d
Remove NS_LITERAL_STRING and NS_LITERAL_CSTRING macros. r=froydnj

Comment 16

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/5f8a5717324d

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d402a579fb2d
Update coding style with respect to string literals. r=froydnj

Comment 18

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/d402a579fb2d

Comment 19

[Simon Giesecke [:sg] [he/him]]

Actually, the main patches already landed in Firefox 80, just the documentation update took much longer. It should probably better have been moved to a separate bug. Sorry for that!

Comment 20

[Ryan VanderMeulen [:RyanVM]]

This burns us pretty often on ESR78 uplifts. While I'm not suggesting we should try to land a tree-wide rewrite there, could we consider uplifting at least enough to support the _ns strings so builds don't break?

Comment 21

[Simon Giesecke [:sg] [he/him]]

Comment on attachment 9159347 [details]
Bug 1648010 - Add user-defined string literals. r=#xpcom-reviewers

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Many patches that are considered for uplifting depend on the user-defined literals that are added by this patch.
• User impact if declined: Delay in uplifting patches to esr
• Fix Landed on Version:
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This only adds new functions which do not affect existing code.
• String or UUID changes made by this patch:

Comment 22

[Simon Giesecke [:sg] [he/him]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #20)

This burns us pretty often on ESR78 uplifts.

:(

While I'm not suggesting we should try to land a tree-wide rewrite there, could we consider uplifting at least enough to support the _ns strings so builds don't break?

Sure, that makes sense, and I checked it grafts cleanly to esr78, so I made an uplift request. Sorry I didn't think of that myself!

Comment 23

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9159347 [details]
Bug 1648010 - Add user-defined string literals. r=#xpcom-reviewers

Thanks, this will make our lives much easier over the next year or so :). Approved for 78.4esr.

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr78/rev/c9bca8151896

Comment 25

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9159347 [details]
Bug 1648010 - Add user-defined string literals. r=#xpcom-reviewers

Removing the approval flag and ESR78 status since this isn't really "fixed" there.

Comment 26

[Simon Giesecke [:sg] [he/him]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #25)

Comment on attachment 9159347 [details]
Bug 1648010 - Add user-defined string literals. r=#xpcom-reviewers

Removing the approval flag and ESR78 status since this isn't really "fixed" there.

Yes, and we probably specifically do NOT want to remove the NS_LITERAL_(C)STRING macros (D80862) in esr78 as that would again be an obstacle to uplifting patches, which is what the title of this bug is telling.

Comment 27

[:Gijs (he/him)]

Should we consider adding this to the vcs "ignored csets for annotate/blame" lists? These patches touched a lot of code without materially changing semantics in consumers, AIUI...

Comment 28

[Simon Giesecke [:sg] [he/him]]

Yes, makes sense to me (without knowing the general policy on this). I just don't know where/how to add that. Can you provide a patch, or point me to directions how to do that?

Comment 29

[:Gijs (he/him)]

(In reply to Simon Giesecke [:sg] [he/him] from comment #28)

Yes, makes sense to me (without knowing the general policy on this). I just don't know where/how to add that. Can you provide a patch, or point me to directions how to do that?

I do not know how either; hg log on the git and hg ignored revs files in the root of the tree point at bug 1633969, which is blocking bug 1637889, which has a comment suggesting Sylvestre knows. Sylvestre?

Comment 30

[Magnus Melin [:mkmelin]]

.hg-annotate-ignore-revs and .git-blame-ignore-revs I suppose

Comment 31

[Sylvestre Ledru [:Sylvestre]]

Yeah and it should also ignore patches with

# ignore-this-changeset

in the commit message

Comment 32

[Marco Castelluccio [:marco]]

Could you add the hashes of these commits to .hg-annotate-ignore-revs and .git-blame-ignore-revs?

Comment 33

[Simon Giesecke [:sg] [he/him]]

(In reply to Marco Castelluccio [:marco] from comment #32)

Could you add the hashes of these commits to .hg-annotate-ignore-revs and .git-blame-ignore-revs?

Sure, I hope I got that right. Sorry I lost track of the earlier request to do this.

Comment 34

[Simon Giesecke [:sg] [he/him]]

Attachment: Bug 1648010 - Add revisions for the replacement of literal string macros by string literals to the ignore sets. r=marco

Comment 35

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/79dea42aa15d
Add revisions for the replacement of literal string macros by string literals to the ignore sets. r=marco,glandium DONTBUILD

Comment 36

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/79dea42aa15d