1585472 - Syntax Error in Windows SDK leading to undefined behavior during build

Status: RESOLVED WORKSFORME

(Firefox Build System :: Toolchains, defect)

Description

[Marc Streckfuß [:MeFisto94]]

Aplogizes if this is the wrong category and/or my findings are imprecise, usually I am not into that deep C++ behavior, but since the build broke for me I had to figure my way around.

What is the problem: When using headers like <Windows.Media.h> in a header/within the unified sources, there are symbol clashes between windows types and mozilla types like VirtualKey or FontStretch.

Where is the problem:
See this for example:

typedef enum FontStretch : int FontStretch;

I was told that one has to choose between typedeffing OR the enum a : type syntax.

Either way gcc is telling me:

test2.cpp:3:26: error: expected ‘;’ or ‘{’ before ‘c’
     typedef enum c : int c;
                          ^
test2.cpp:3:26: error: expected class-key before ‘c’

It currently works, where is the problem? The problem comes from clang half-supporting that syntax, as you can see in the following two snippets: When the typedef enum comes before the class, it works as intended, if it is the other way around, it won't work.

works:

namespace a {
  namespace b {
    typedef enum c : int c;
  }
}
using namespace a::b;

namespace d {
  namespace e {
    class c {
      public:
        void test();
    };
  }
}
using namespace d::e;

fails:

namespace d {
  namespace e {
    class c {
      public:
        void test();
    };
  }
}
using namespace d::e;

namespace a {
  namespace b {
    typedef enum c : int c;
  }
}
using namespace a::b;

You can try it out with clang here, test2.cpp is working, main.cpp is not.

What does that mean again? The Win 10 SDK makes heavy use of these typedeffed enums, which means that every win 10 API that we want to use has to be included BEFORE any mozilla class is defined. This has gone under the radar because we've excluded Win 10 API Files from the unified build and they never had any WinAPI definitions in the header, thus it never conflicted with mozilla symbols.

The workaround is simple: adding CXXFLAGS += ["/FIWindows.Media.h"] to moz.build, which is what I'll contribute to this bug, however we might want to report this to both clang and microsoft to raise awareness of this issue (MSVC might compile this just fine).

Comment 1

[Marc Streckfuß [:MeFisto94]]

Attachment: Bug 1585472 - Fix potential build issues related to a corner-case in clang when using the Win API and fix files to work with Unified Sources again

Comment 2

[Marc Streckfuß [:MeFisto94]]

Can you assign someone from the build-system to maybe take care of reporting this upstream?
I was able to work around this symptom but that doesn't make the problem non-existant.

Comment 3

[Nathan Froyd [:froydnj]]

(In reply to Marc Streckfuß [:MeFisto94] from comment #2)

Can you assign someone from the build-system to maybe take care of reporting this upstream?
I was able to work around this symptom but that doesn't make the problem non-existant.

Can't you file the bug with clang upstream?

Comment 4

[Marc Streckfuß [:MeFisto94]]

So, I've reported that upstream at https://bugs.llvm.org/show_bug.cgi?id=44941, but so far it doesn't seem to yield any resonance.
The good thing is: We can probably work around any bug of this sort.

Comment 5

[Marc Streckfuß [:MeFisto94]]

This is fixed in https://github.com/llvm/llvm-project/commit/c90e198107431f64b73686bdce31c293e3380ac7
Judging by the last file diff, I suspect the change to be scheduled for clang 11.
Can you guys somehow ensure this issue is closed once clang 11 is updated? Or ensure that someone looks after it when updating clang?

Comment 6

[Nathan Froyd [:froydnj]]

Since we are now on clang 11, I think we can say this is done.