1679745 - TelemetryHistogramEnums.h:4240:3: error: expected identifier before numeric constant

Status: RESOLVED FIXED

(Core :: Audio/Video: cubeb, defect)

Description

[Petr Sumbera]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:83.0) Gecko/20100101 Firefox/83.0

Steps to reproduce:

While building latest Firefox sources on Solaris I got following error:

 2:12.24 /builds/psumbera/FIREFOX/obj-x86_64-pc-solaris2.11/dist/include/mozilla/TelemetryHistogramEnums.h:4240:3: error: expected identifier before numeric constant
 2:12.24  4240 |   sun
 2:12.24       |   ^~~
 2:12.24 /builds/psumbera/FIREFOX/obj-x86_64-pc-solaris2.11/dist/include/mozilla/TelemetryHistogramEnums.h:4240:3: error: expected ‘}’ before numeric constant
 2:12.24 In file included from /builds/psumbera/FIREFOX/obj-x86_64-pc-solaris2.11/dist/include/mozilla/Telemetry.h:11,
 2:12.24                  from /builds/psumbera/FIREFOX/accessible/base/Statistics.h:10,
 2:12.24                  from /builds/psumbera/FIREFOX/accessible/base/nsAccessibilityService.cpp:38,
 2:12.24                  from Unified_cpp_accessible_base1.cpp:65:
 2:12.24 /builds/psumbera/FIREFOX/obj-x86_64-pc-solaris2.11/dist/include/mozilla/TelemetryHistogramEnums.h:4226:50: note: to match this ‘{’
 2:12.24  4226 | enum class LABELS_MEDIA_AUDIO_BACKEND : uint32_t {
 2:12.25       |                                                  ^
 2:12.25 /builds/psumbera/FIREFOX/obj-x86_64-pc-solaris2.11/dist/include/mozilla/TelemetryHistogramEnums.h:4240:3: error: expected unqualified-id before numeric constant
 2:12.25  4240 |   sun
 2:12.25       |   ^~~
 2:12.25 /builds/psumbera/FIREFOX/obj-x86_64-pc-solaris2.11/dist/include/mozilla/TelemetryHistogramEnums.h:4765:42: error: ‘LABELS_ABOUT_CONFIG_FEATURES_USAGE’ was not declared in this scope; did you mean ‘mozilla::Telemetry::LABELS_ABOUT_CONFIG_FEATURES_USAGE’?
 2:12.25  4765 | template<> struct IsCategoricalLabelEnum<LABELS_ABOUT_CONFIG_FEATURES_USAGE> : std::true_type {};
 2:12.25       |                                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 2:12.25       |                                          mozilla::Telemetry::LABELS_ABOUT_CONFIG_FEATURES_USAGE		

I was able to bisect it to following commit:

changeset: 557207:7448f9dcbc12
user: Chun-Min Chang <[e-mail]>
date: Tue Nov 10 23:23:35 2020 +0100
description:
Bug 1671714 - Add telemetry probes tracking audio backends r=padenot

    Differential Revision: https://phabricator.services.mozilla.com/D95625

Comment 1

[Petr Sumbera]

'sun' is actually predefined compiler macro:

gcc -dM -E - < /dev/null | grep sun
#define __sun 1
#define sun 1
#define __sun__ 1

Comment 2

[Petr Sumbera]

Not sure how it's related. When I reaname sun to xsun I get following error:

 2:21.12 /builds/psumbera/FIREFOX/accessible/base/NotificationController.cpp:597:62: error: ‘OTHER’ was not declared in this scope
 2:21.12   597 |   AUTO_PROFILER_LABEL("NotificationController::WillRefresh", OTHER);
 2:21.12       |                                                              ^~~~~

Comment 3

[Paul Adenot (:padenot)]

This is generated from a json file, I don't know what is best here.

Comment 4

[Petr Sumbera]

Above issue is probably not realated as I was able sucesfully compile changeset: 557207:7448f9dcbc12 with bellow change:

diff -r 030df0988097 dom/media/CubebUtils.cpp
--- a/dom/media/CubebUtils.cpp  Thu Nov 05 12:35:09 2020 +0100
+++ b/dom/media/CubebUtils.cpp  Mon Nov 30 18:48:47 2020 +0100
@@ -142,7 +142,7 @@
         {"pulse", LABELS_MEDIA_AUDIO_BACKEND::pulse},
         {"pulse-rust", LABELS_MEDIA_AUDIO_BACKEND::pulse_rust},
         {"sndio", LABELS_MEDIA_AUDIO_BACKEND::sndio},
-        {"sun", LABELS_MEDIA_AUDIO_BACKEND::sun},
+        {"xsun", LABELS_MEDIA_AUDIO_BACKEND::xsun},
 };

 // Prefered samplerate, in Hz (characteristic of the hardware, mixer, platform,
diff -r 030df0988097 toolkit/components/telemetry/Histograms.json
--- a/toolkit/components/telemetry/Histograms.json      Thu Nov 05 12:35:09 2020 +0100
+++ b/toolkit/components/telemetry/Histograms.json      Mon Nov 30 18:48:47 2020 +0100
@@ -9988,7 +9988,7 @@
       "pulse",
       "pulse_rust",
       "sndio",
-      "sun"
+      "xsun"
     ],
     "description": "The operating system audio backend",
     "bug_numbers": [1671714]
diff -r 030df0988097 toolkit/components/telemetry/geckoview/streaming/metrics.yaml
--- a/toolkit/components/telemetry/geckoview/streaming/metrics.yaml     Thu Nov 05 12:35:09 2020 +0100
+++ b/toolkit/components/telemetry/geckoview/streaming/metrics.yaml     Mon Nov 30 18:48:47 2020 +0100
@@ -1684,7 +1684,7 @@
       - pulse
       - pulse_rust
       - sndio
-      - sun
+      - xsun
     gecko_datapoint: MEDIA_AUDIO_BACKEND
     description: >
       The operating system audio backend

I wonder what 'sun' should mean? And what should be proper fix/name.

Comment 5

[Paul Adenot (:padenot)]

"sun" is for the "sun" audio backend, for illuminos/solaris/netbsd backend, named like that by the original author of the backend, https://github.com/kinetiknz/cubeb/commit/9f39687e7fa8ad98604334cda512d27b6bf93092, unclear what is best here.

chunmin, do you think simply renaming this would work? Maybe a simple special case will be enough here, sorry I forgot to NI yesterday.

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1671714

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

The product::component has been changed since the backlog priority was decided, so we're resetting it.
For more information, please visit auto_nag documentation.

Comment 9

[Chris H-C :chutten]

Assigning product/component based on regressing bug.

Comment 10

[Petr Sumbera]

Attachment: Bug 1679745 - Avoid name collision with compiler macro sun on Solaris

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d319246c3f01
Avoid name collision with compiler macro sun on Solaris r=chunmin

Comment 12

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

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

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Is this something we should uplift to Beta for 85?

Comment 14

[Petr Sumbera]

No, there is no need for uplift. I'm mainly concerned with ESR versions.