MSVC /Za considered harmful

Hi clang wizards,

Apparently, when clang is built with MSVC, it uses the /Za compiler option (see [0]). I say "apparently" because we've received numerous reports (see [1]), including from people who are building clang (see [2] and [3]), that VC11 RC's <system_error> doesn't compile under /Za for boring reasons. We've fixed <system_error> for VC11 RTM, but I wanted to mention this because:

/Za is buggy. Please don't use it.

In particular, it breaks perfectly conformant code like vector<unique_ptr<T>> (reason: [4]). As a result, we stopped testing the STL under /Za back during VC10's development. That's how thinkos like the <system_error> thing are able to sneak in. We're willing to fix such thinkos as they're brought to our attention (missing this-> is another example, of several), but that doesn't magically fix /Za. (I've asked the compiler team to deprecate /Za and eventually remove it - but while they agree that it shouldn't be used, we have tests that depend on it, and getting rid of it would be a fair amount of work.)

I understand why you'd want to use /Za - because VC has various extensions that you don't want to use in portable code (most notoriously, the Evil Extension of permitting modifiable lvalue references to bind to rvalues). Requesting extra conformance is a good idea in theory. It's just that /Za is flaky, and it's rarely used which means that even less code exercises it. Instead of "request extra conformance", think of it as "enable obscure compiler bugs". Instead of /Za, the best thing to do is to compile at /W4 (which warns about most of the extensions), and also regularly build with another compiler (which I suspect will not be a problem).

Stephan T. Lavavej
Visual C++ Libraries Developer

0. http://msdn.microsoft.com/en-us/library/0k0w269d.aspx
1. http://connect.microsoft.com/VisualStudio/feedback/details/745614/system-error-error-c2382-when-compiling-with-za-disable-language-extensions
2. http://connect.microsoft.com/VisualStudio/feedback/details/745643/error-c2382-on-std-generic-category-std-iostream-category-std-system-category
3. http://connect.microsoft.com/VisualStudio/feedback/details/745967/compiler-error-c2382-with-standard-header-file-system-error-in-visual-c-2012-rc
4. /Za performs an "elided copy constructor" accessibility check that's required by the Standard, but which VC ordinarily doesn't bother to do. However, /Za performs it too aggressively, including during move construction when no copy constructors are being called, even theoretically.

(I've asked the compiler team to deprecate /Za and eventually remove it - but while they agree that it shouldn't be used, we have tests that depend on it, and getting rid of it would be a fair amount of work.)

Instead of asking them to get rid of it, why don't you ask them to fix it? Having a compiler option that disables nonstandard extensions is generally desirable.

Regards,
Nate

Instead of asking them to get rid of it, why don't you ask them to fix it?
Having a compiler option that disables nonstandard extensions is generally
desirable.

Hear, hear. Saying "Fix the warnings at /W4 to get compliance" is all very well,
but keeping them from compiling in the first place works a lot better in a large
programming team with people working on many platforms.

(I would encourage folks to at least keep STL on the to-line, or mail him directly as he may not read cfe-dev as closely as others…)

Instead of asking them to get rid of it, why don’t you ask them to fix it?
Having a compiler option that disables nonstandard extensions is generally
desirable.

Hear, hear. Saying “Fix the warnings at /W4 to get compliance” is all very well,
but keeping them from compiling in the first place works a lot better in a large
programming team with people working on many platforms.

The common problem is that standard library headers shipping with the compile should have access to the extensions. They’re “implementation details” that can be relied upon within the standard library, or any other part of the toolchain.

FWIW, Clang, GCC, and others handle this with a two-pronged approach:

  1. Make a special warning mode that warns (perhaps promoting warnings to errors) on all extensions.
  2. Suppress warnings (even if they might be promoted to errors) when they come from system headers.

This allows the compiler to warn (or error) on extensions in the user code, without turning off entire features, some of which may be in use.

Note that this does require the extensions to be correctly attributed to the system header even when templates macros and other devices are used. Sometimes this is problematic.

Anyways, this is all a bit off-topic for Clang development. The right approach is for Clang to not set flags when being built by Visual Studio that are flaky or problematic. People experimenting or developing on Clang shouldn’t have a degraded experience when first hacking on the code. Hopefully one of our Windows developers can send a patch my way for the CMake builds? I’m happy to review it, but I don’t have windows to really test it out on…

Hi clang wizards,

Apparently, when clang is built with MSVC, it uses the /Za compiler option (see [0]). I say "apparently" because we've received numerous reports (see [1]), including from people who are building clang (see [2] and [3]), that VC11 RC's <system_error> doesn't compile under /Za for boring reasons. We've fixed <system_error> for VC11 RTM, but I wanted to mention this because:

/Za is buggy. Please don't use it.

In particular, it breaks perfectly conformant code like vector<unique_ptr<T>> (reason: [4]). As a result, we stopped testing the STL under /Za back during VC10's development. That's how thinkos like the <system_error> thing are able to sneak in. We're willing to fix such thinkos as they're brought to our attention (missing this-> is another example, of several), but that doesn't magically fix /Za. (I've asked the compiler team to deprecate /Za and eventually remove it - but while they agree that it shouldn't be used, we have tests that depend on it, and getting rid of it would be a fair amount of work.)

I understand why you'd want to use /Za - because VC has various extensions that you don't want to use in portable code (most notoriously, the Evil Extension of permitting modifiable lvalue references to bind to rvalues). Requesting extra conformance is a good idea in theory. It's just that /Za is flaky, and it's rarely used which means that even less code exercises it. Instead of "request extra conformance", think of it as "enable obscure compiler bugs". Instead of /Za, the best thing to do is to compile at /W4 (which warns about most of the extensions), and also regularly build with another compiler (which I suspect will not be a problem).

Hi Stephan,

The /Za flag was added to the build of clang not to encourage writing portable code but because an extension was breaking perfectly valid c++ code for "pre-C++11 move emulation". (See the lengthy comment in include/clang/Sema/Ownership.h [1]). This was with msvc8 though and it's possible that the code isn't used in a way that breaks without /Za anymore or the compiler was fixed in the meantime.

I find the decision to deprecate /Za worrisome because it is a useful feature, but that's your choice and we should remove uses from clang if possible. We don't want to break users who build clang with msvc with confusing error messages if it's easily avoidable. Coverage of building clang with msvc11 will probably decline sharply when it hits the market as setting up a build will require buying a license, build breakages will stay unnoticed for a longer time.

A few words to /W4: We experimented with having an automated builder using /W4 in the msvc8-days but it was very noisy and slowed down the build by 2x or so, which was unacceptable for a bot. This may have changed now but it still requires someone to clean up all the false positives before it will be useful.

- Ben

[1] http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Ownership.h?revision=135576&view=markup

If the warnings that will need cleaning are false positives, I’m very opposed to this cleanup effort. We get no benefit from it really, and it has a high long-term maintenance cost as the majority of developers will continually check in new code regressing these warnings, as the warnings don’t represent actual problems with the code.

I just removed a couple of lines from CMakeLists that mentioned /Za and projects are now generated without this flag. I did a rebuild using Visual Studio 2010 and everything went fine except for CFG.cpp which says:

clang\lib\Analysis\CFG.cpp(3203): error C2059: syntax error : ‘=’
clang\lib\Analysis\CFG.cpp(3203): error C2143: syntax error : missing ‘;’ before ‘{’
clang\lib\Analysis\CFG.cpp(3204): error C2059: syntax error : ‘__cdecl’

Renaming cdecl variable to decl fixed this, funny.

P.S. I know very little about CMake, somebody who knows this stuff should review the patch

remove-za.txt (3.08 KB)

I zapped /Za in r158063, cdecl is a reserved keyword in MSVC. (calling
convention).

Make sure to update the comment mentioning /Za on line 142 of include/clang/Sema/Ownership.h

–Sean Silva

[Nathan Ridge]

Instead of asking them to get rid of it, why don't you ask them to fix it?

I did. When I'm elected World Controller in 2012, I'll actually get everything I ask for. Or else.

[Chandler Carruth]

(I would encourage folks to at least keep STL on the to-line, or mail him directly as he may not read cfe-dev as closely as others...)

Thanks. I actually subscribed, then disabled mail delivery, since I wasn't planning to monitor cfe-dev for the MSVC bat-signal like I do for Boost. (I was planning to check the web archives for replies, and only later realized that I'd break threading if I didn't have any mails to reply to.)

The common problem is that standard library headers shipping with the compile *should* have access to the extensions.

Yeah. In VC's STL, we definitely attempt to remain portable except when we absolutely must use extensions (e.g. DLL stuff, type traits compiler hooks), but the CRT, ATL/MFC, windows.h, and other libraries definitely vary in their use of extensions.

The right approach is for Clang to not set flags when being built by Visual Studio that are flaky or problematic.

A few others that are hopefully not relevant:

* /J is incredibly evil.
* /RTCc breaks strictly conforming code by design (e.g. casts that truncate). I am barely willing to fix /RTCc problems in the STL when the fixes are trivial, but we don't run our tests with this.
* /Wp64 is notoriously buggy (it has actually been command-line deprecated for three major versions: VC9-11). It suffers from false positives and false negatives, especially in templated code. The correct thing to do is to compile with a 64-bit-targeting compiler in order to catch 64-bit problems.

[Benjamin Kramer]

The /Za flag was added to the build of clang not to encourage writing portable code but because an extension
was breaking perfectly valid c++ code for "pre-C++11 move emulation".

I wasn't aware of that (but it doesn't surprise me - in fact I bet it's the Evil Extension).

However, I consider breaking real move semantics to be more severe than breaking simulated move semantics.

I find the decision to deprecate /Za worrisome because it is a useful feature, but that's your choice

It's actually the compiler team's choice (they have to worry about things like dev/test cost and breaking users; those aren't my costs and I love breaking users) - my preferences are certainly #1 remove extensions outright, #2 fix /Za, #3 remove /Za, #4 command-line deprecate /Za (as a prelude to #3), #5 notify people when we see them using /Za.

Coverage of building clang with msvc11 will probably decline sharply when it hits the market as setting up a build will require buying a license

I don't actually know anything about this subject (and I won't until I can get VC11 RTM in a VM), but if you care about command-line builds only (as I do), I believe that Magic 8 Ball says Outlook Good.

A few words to /W4: We experimented with having an automated builder using /W4 in the msvc8-days but it was very noisy and slowed down the build by 2x or so

Compared to /W3, or compared to no (!!!) warnings at all? That is news to me.

STL

[Nathan Ridge]

Instead of asking them to get rid of it, why don’t you ask them to fix it?

I did. When I’m elected World Controller in 2012, I’ll actually get everything I ask for. Or else.

[Chandler Carruth]

(I would encourage folks to at least keep STL on the to-line, or mail him directly as he may not read cfe-dev as closely as others…)

Thanks. I actually subscribed, then disabled mail delivery, since I wasn’t planning to monitor cfe-dev for the MSVC bat-signal like I do for Boost. (I was planning to check the web archives for replies, and only later realized that I’d break threading if I didn’t have any mails to reply to.)

The common problem is that standard library headers shipping with the compile should have access to the extensions.

Yeah. In VC’s STL, we definitely attempt to remain portable except when we absolutely must use extensions (e.g. DLL stuff, type traits compiler hooks), but the CRT, ATL/MFC, windows.h, and other libraries definitely vary in their use of extensions.

The right approach is for Clang to not set flags when being built by Visual Studio that are flaky or problematic.

A few others that are hopefully not relevant:

  • /J is incredibly evil.
  • /RTCc breaks strictly conforming code by design (e.g. casts that truncate). I am barely willing to fix /RTCc problems in the STL when the fixes are trivial, but we don’t run our tests with this.
  • /Wp64 is notoriously buggy (it has actually been command-line deprecated for three major versions: VC9-11). It suffers from false positives and false negatives, especially in templated code. The correct thing to do is to compile with a 64-bit-targeting compiler in order to catch 64-bit problems.

[Benjamin Kramer]

The /Za flag was added to the build of clang not to encourage writing portable code but because an extension
was breaking perfectly valid c++ code for “pre-C++11 move emulation”.

I wasn’t aware of that (but it doesn’t surprise me - in fact I bet it’s the Evil Extension).

However, I consider breaking real move semantics to be more severe than breaking simulated move semantics.

I find the decision to deprecate /Za worrisome because it is a useful feature, but that’s your choice

It’s actually the compiler team’s choice (they have to worry about things like dev/test cost and breaking users; those aren’t my costs and I love breaking users) - my preferences are certainly #1 remove extensions outright, #2 fix /Za, #3 remove /Za, #4 command-line deprecate /Za (as a prelude to #3), #5 notify people when we see them using /Za.

Coverage of building clang with msvc11 will probably decline sharply when it hits the market as setting up a build will require buying a license

Is this more of a legal or technical issue? Right now, you can get a C++ toolchain from the “free” tools by installing Visual Studio 2012 Express and then the Windows 8 SDK. Both of these will presumably be free for RTM. I just built LLVM with this combination.

The Win 8 SDK even states "You can use the Windows SDK, along with your chosen development environment, to write Windows Metro style apps […]; desktop applications that use the native (Win32/COM) programming model; …"

Folks, I’m going to suggest that we not hold this discussion on this mailing list.

The tech press has covered the issue thoroughly (if not necessarily well), but I suspect that none of the Microsoft engineers you might run into on this list will be able to clarify things, any more than I can comment about Google products, or Apple folks can comment about Apple products.

Let’s get back to compiler warnings and building Clang/LLVM. Those are the interesting bits. =]

We use /W3 by default with quite a few warnings disabled. I just
tested out /W4 with 2012 RC, and while the compile time seems fine, it
generates 157k lines (21MiB) of warnings on LLVM (only x86 target)
alone. Very few of them are useful because they fire in too many
contexts. We can disable them, but then they don't fire at all, even
when they are correct.

As for the /W4 specific extension warnings, I have been unable to find
exactly which numbers they are. We could specifically add these to our
build.

And on a related note, are bug reports about non-standard and
non-extension errors in the Visual C++ Libraries useful? We often
encounter cases with, for example, missing typenames and incorrect
name lookup when compiling with Clang. We have added support for
emulating MSVC's behavior, but it would be nice to be able to not have
to do this, as it makes writing portable code harder.

- Michael Spencer

[Nathan Ridge]

Instead of asking them to get rid of it, why don't you ask them to fix it?

I did. When I'm elected World Controller in 2012, I'll actually get everything I ask for. Or else.

[Chandler Carruth]

(I would encourage folks to at least keep STL on the to-line, or mail him directly as he may not read cfe-dev as closely as others...)

Thanks. I actually subscribed, then disabled mail delivery, since I wasn't planning to monitor cfe-dev for the MSVC bat-signal like I do for Boost. (I was planning to check the web archives for replies, and only later realized that I'd break threading if I didn't have any mails to reply to.)

The common problem is that standard library headers shipping with the compile *should* have access to the extensions.

Yeah. In VC's STL, we definitely attempt to remain portable except when we absolutely must use extensions (e.g. DLL stuff, type traits compiler hooks), but the CRT, ATL/MFC, windows.h, and other libraries definitely vary in their use of extensions.

The right approach is for Clang to not set flags when being built by Visual Studio that are flaky or problematic.

A few others that are hopefully not relevant:

* /J is incredibly evil.
* /RTCc breaks strictly conforming code by design (e.g. casts that truncate). I am barely willing to fix /RTCc problems in the STL when the fixes are trivial, but we don't run our tests with this.
* /Wp64 is notoriously buggy (it has actually been command-line deprecated for three major versions: VC9-11). It suffers from false positives and false negatives, especially in templated code. The correct thing to do is to compile with a 64-bit-targeting compiler in order to catch 64-bit problems.

[Benjamin Kramer]

The /Za flag was added to the build of clang not to encourage writing portable code but because an extension
was breaking perfectly valid c++ code for "pre-C++11 move emulation".

I wasn't aware of that (but it doesn't surprise me - in fact I bet it's the Evil Extension).

However, I consider breaking real move semantics to be more severe than breaking simulated move semantics.

I agree, it looks like it's not an issue anymore as /Za was removed from the clang build and everything seems to be fine :slight_smile:

I find the decision to deprecate /Za worrisome because it is a useful feature, but that's your choice

It's actually the compiler team's choice (they have to worry about things like dev/test cost and breaking users; those aren't my costs and I love breaking users) - my preferences are certainly #1 remove extensions outright, #2 fix /Za, #3 remove /Za, #4 command-line deprecate /Za (as a prelude to #3), #5 notify people when we see them using /Za.

Coverage of building clang with msvc11 will probably decline sharply when it hits the market as setting up a build will require buying a license

I don't actually know anything about this subject (and I won't until I can get VC11 RTM in a VM), but if you care about command-line builds only (as I do), I believe that Magic 8 Ball says Outlook Good.

A few words to /W4: We experimented with having an automated builder using /W4 in the msvc8-days but it was very noisy and slowed down the build by 2x or so

Compared to /W3, or compared to no (!!!) warnings at all? That is news to me.

Compared to a clean /W3 build. But this was years ago and it is likely that there were other factors leading to the slowdown. It produced an enormous wall of text that had to be printed to stderr, logged and sent over the network (it was a buildbot instance). That alone could have caused the slowdown we observed.

- Ben

[Michael Spencer]

As for the /W4 specific extension warnings, I have been unable to find
exactly which numbers they are. We could specifically add these to our
build.

Here's the one for the Evil Extension:

C:\Temp>type meow.cpp
struct X { };

void meow(X&) { }

int main() {
    meow(X());
}

C:\Temp>cl /EHsc /nologo /W4 /MTd meow.cpp
meow.cpp
meow.cpp(6) : warning C4239: nonstandard extension used : 'argument' : conversion from 'X' to 'X &'
        A non-const reference may only be bound to an lvalue

I believe there are others, but I can't remember them at the moment.

And on a related note, are bug reports about non-standard and
non-extension errors in the Visual C++ Libraries useful? We often
encounter cases with, for example, missing typenames and incorrect
name lookup when compiling with Clang. We have added support for
emulating MSVC's behavior, but it would be nice to be able to not have
to do this, as it makes writing portable code harder.

For the STL (and anything that the STL drags in) - yes, I consider such things to be bugs, and I'll gladly accept such reports so I can fix them. (Missing typename/template disambiguation is obvious, so it just needs to be pointed out. Ditto for missing this-> to reach into dependent base classes. For more complicated things like two-stage name lookup, I would appreciate a more detailed explanation of what a conformant compiler will do with the code.)

As you may have heard, VC10+'s Intellisense is powered by EDG. What you probably haven't heard is that there's an undocumented command-line option, /BE , which invokes the EDG front-end (for compilation only, it isn't wired up to codegen). We build all of our STL tests with (and without, obviously) this switch, to ensure that EDG and therefore Intellisense can parse us. This catches various STL bugs (and EDG bugs, more rarely). By default, VC runs the EDG FE in its bug-compatible "I can't believe it's not C1XX mode". However, there's an "even more undocumented" option, /BE /dE--parse_templates , which tells EDG to be aggressive about compiling templates when they're first seen (like all other compilers). This catches lots of missing typename/template disambiguation, plus stuff like straight-up typos, which C1XX doesn't validate when a template is first seen.

In VC11, I've cleaned out most of the problems revealed by /BE /dE--parse_templates in the STL (and also the PPL headers that <future>/etc. drag in now). There was one in <scoped_allocator> that I wasn't able to trivially fix - I'm going to let Dinkumware do that surgery. In the long run, we'll build our STL tests with /BE /dE--parse_templates to continuously run this strict validation (we don't yet because something in our headers is triggering an ICE under these options - it has been reported).

However I'm sure that clang will find additional issues, which I'd love to fix.

For libraries that the STL doesn't drag in, I can't speak for their maintainers, but you can report them through MS Connect.

STL

[Benjamin Kramer]

The /Za flag was added to the build of clang not to encourage writing portable code but because an extension
was breaking perfectly valid c++ code for "pre-C++11 move emulation".

[STL]

I wasn't aware of that (but it doesn't surprise me - in fact I bet it's the Evil Extension).
However, I consider breaking real move semantics to be more severe than breaking simulated move semantics.

[Benjamin Kramer]

I agree, it looks like it's not an issue anymore as /Za was removed from the clang build and everything seems to be fine :slight_smile:

Yay!

[Benjamin Kramer]

A few words to /W4: We experimented with having an automated builder using /W4 in the msvc8-days but it was very noisy and slowed down the build by 2x or so

[STL]

Compared to /W3, or compared to no (!!!) warnings at all? That is news to me.

[Benjamin Kramer]

Compared to a clean /W3 build. But this was years ago and it is likely that there were other factors leading to the slowdown.
It produced an enormous wall of text that had to be printed to stderr, logged and sent over the network (it was a buildbot instance).
That alone could have caused the slowdown we observed.

Yep, that certainly sounds like the culprit. For clean builds, I would expect /W3 and /W4 to be equally fast - according to my understanding, the compiler doesn't go out of its way to track more things at higher warning levels, it simply notices things and then determines whether the current warning level says it should warn. (In contrast, /analyze invokes a different and much more expensive FE.)

STL