This doesn’t work.
Some tests depend on -fno-delayed-template-parsing to exist at the
driver level.
Good catch! I’ve revised the patch so that
-fno-delayed-template-parsing will be honored in the driver.
Having option -fno-delayed-template-parsing without
-fdelayed-template-parsing doesn’t sound right to me.
How about we keep it.
Are you suggesting we keep the flag, but still make it implicitly on
when ms-compatibility is specified? Or just keep things as they are
today? I am fine with keeping the flag, but I still think we should
implicitly turn it on for ms-compatibility.
Shouldn’t those tests that are currently using
-fno-delayed-template-parsing be modified to use -fms-compatibility or
pass -fno-delayed-template-parsing via -Xclang? Or are those test case
enough to justify the existence of -fno-delayed-template-parsing as a
driver-visible flag?
I usually figure that if there’s a test for it, there’s a reason for
it, so I think they do justify the ability to turn off delayed
template parsing explicitly.
My best guess, glancing at the tests, is that the switch was used to
make the tests portable - probably just because the switch existed and
was the smallest change necessary to get the tests to pass across
platforms. It doesn’t look like there would be a problem with just
-fno-ms-compatibility being used entirely for those tests but I’m
open to suggestions.
In fact they were pretty much all added in r138942 by Francois:
“Enable -fdelayed-template-parsing by default on Win32.
I had to force -fno-delayed-template-parsing on some Index tests
because delayed template parsing will change the output of some
tests.”
So presumably if we make -fms-compatibility default on in Win32 (which
if we haven’t already, we probably should if it’s going to subsume
-fdelayed-template-parsing) then it make sense to just change these
all to -fno-ms-compatibility.
Hold your horses! Win32 != borked MSVC headers! There’s also GCC for
windows, and as I don’t know what implications the delayed template parsing
has on the libstdc++/libc++ headers, it’s not a good idea to make this a
default for Win32.
Perhaps you misunderstood - delayed template parsing already is the
default in windows & has been for a couple of months.
Wow, OK. Didn’t realize that. I assumed ms-compatibility == implement “features” needed to parse MSVC headers.
If I may ask, what could delayed template parsing do on other platforms? I noticed a post about a 5% speed increase when the option is used
Hold your horses! Win32 != borked MSVC headers! There's also GCC for
windows, and as I don't know what implications the delayed template parsing
has on the libstdc++/libc++ headers, it's not a good idea to make this a
default for Win32.
We're talking about -fms-compatibility, which does = borked MSVC
headers, which generally require delayed template parsing. That's why
the discussion is happening about making -fms-compatibility
automatically adding -fdelayed-template-parsing.
I think this is very a theoretical discussion, in practice both
-fdelayed-template-parsing and -fms-compatibility options are passed
by the driver to the -cc1 clang internal spawning if the triple is
Win32. For GCC on Windows there is a different MinGW32 triple.
After a discussion on IRC, it sounds like the current patch would be
acceptable as-is. But I agree with Francois that this is more
theoretical than I was originally thinking. It sounds like this patch
would be helpful for people using a clang build that was not made with
Visual Studio (MinGW, et al), but want to target the Windows platform
SDK headers (instead of the MinGW headers, for instance). I don't
think this is a huge audience.
If there are continued reservations, I'm happy to let the patch drop.
If people think there's use from the patch, it sounds like it may be
good to go.
Please let this be a separate option from the ms-mode option. See comment
below.
>
>
>>> I don't see why the changes to lib/Frontend/CompilerInvocation.cpp and
>>> include/clang/Driver/CC1Options.td are necessary. The changes to
>>> lib/Driver/Tools.cpp look fine.
>>
>> I'm not certain I understand why the two are separate from one
>> another. Can you give me a quite education on the topic?
>>
>>> How about we remove --fdelayed-template-parsing altogether and check
>>> for fms-compatibility to do late template parsing?
>>>
>>> Anybody else needs delayed template parsing without microsoft compatibility?
We do! And we don't want them to be tied to ms-mode, because we want to
be able to set ms-mode (to get ms language extensions) but allow 2-phase
lookup by default. For legacy code however, we want to allow users to
be able to disable 2-phase lookup.
Just to be clear, -fdelayed-template-parsing doesn't disable 2-phase
lookup. It just delay phase 1 lookup until the end of the translation
unit with the effect of making more symbols available then. In
practice that's enough to get very good compatibility with MSVC
template code.
If it's good enough for MSVC compatibility, then it's probably good
enough for Borland's MSVC compatibility mode as well :). Borland's C++
compiler actually implements a bizzarre combination of delayed and
2-phase lookup (long story). But in general, it tries to be compatibile
with MSVC, so I'm hoping that being able to set this option independent
of MSVC will be good enough.