r174980 - Accept over-qualified constructor in MSVC emulation mode

[Richard Smith]

On Tue, Feb 12, 2013 at 11:31 AM, Richard Smith <richard at metafoo.co.uk>wrote:

> On Tue, Feb 12, 2013 at 9:27 AM, Dmitri Gribenko <gribozavr at gmail.com>wrote:
>
>> Author: gribozavr
>> Date: Tue Feb 12 11:27:41 2013
>> New Revision: 174980
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=174980&view=rev
>> Log:
>> Accept over-qualified constructor in MSVC emulation mode
>>
>> MSVC accepts this:
>>
>> class A {
>>   A::A();
>> };
>>
>> Clang accepts regular member functions with extra qualification as an MS
>> extension, but not constructors.  This changes the parser to defer
>> rejecting
>> qualified constructors so that the same Sema logic can apply to
>> constructors as
>> regular member functions.  This also improves the error message when MS
>> extensions are disabled (in my opinion). Before it was:
>>
>> /Users/jason/Desktop/test.cpp:2:8: error: expected member name or ';'
>> after declaration specifiers
>>   A::A();
>>   ~~~~ ^
>> 1 error generated.
>>
>> After:
>>
>> /Users/jason/Desktop/test.cpp:2:6: error: extra qualification on member
>> 'A'
>>   A::A();
>>   ~~~^
>> 1 error generated.
>>
>> Patch by Jason Haslam.
>>
>> Modified:
>>     cfe/trunk/lib/Parse/ParseDecl.cpp
>>     cfe/trunk/test/CXX/special/class.ctor/p1.cpp
>>     cfe/trunk/test/SemaCXX/MicrosoftExtensions.cpp
>>
>> Modified: cfe/trunk/lib/Parse/ParseDecl.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=174980&r1=174979&r2=174980&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Parse/ParseDecl.cpp (original)
>> +++ cfe/trunk/lib/Parse/ParseDecl.cpp Tue Feb 12 11:27:41 2013
>> @@ -2274,8 +2274,7 @@ void Parser::ParseDeclarationSpecifiers(
>>          // name, then the code is ill-formed; this interpretation is
>>          // reinforced by the NAD status of core issue 635.
>>          TemplateIdAnnotation *TemplateId =
>> takeTemplateIdAnnotation(Next);
>> -        if ((DSContext == DSC_top_level ||
>> -             (DSContext == DSC_class && DS.isFriendSpecified())) &&
>> +        if ((DSContext == DSC_top_level || DSContext == DSC_class) &&
>>              TemplateId->Name &&
>>              Actions.isCurrentClassName(*TemplateId->Name, getCurScope(),
>> &SS)) {
>>            if (isConstructorDeclarator()) {
>> @@ -2325,8 +2324,7 @@ void Parser::ParseDeclarationSpecifiers(
>>
>>        // If we're in a context where the identifier could be a class
>> name,
>>        // check whether this is a constructor declaration.
>> -      if ((DSContext == DSC_top_level ||
>> -           (DSContext == DSC_class && DS.isFriendSpecified())) &&
>> +      if ((DSContext == DSC_top_level || DSContext == DSC_class) &&
>>            Actions.isCurrentClassName(*Next.getIdentifierInfo(),
>> getCurScope(),
>>                                       &SS)) {
>>          if (isConstructorDeclarator())
>> @@ -4445,8 +4443,7 @@ void Parser::ParseDirectDeclarator(Decla
>>        else if (D.getCXXScopeSpec().isSet())
>>          AllowConstructorName =
>>            (D.getContext() == Declarator::FileContext ||
>> -           (D.getContext() == Declarator::MemberContext &&
>> -            D.getDeclSpec().isFriendSpecified()));
>> +           D.getContext() == Declarator::MemberContext);
>>        else
>>          AllowConstructorName = (D.getContext() ==
>> Declarator::MemberContext);
>>
>>
>> Modified: cfe/trunk/test/CXX/special/class.ctor/p1.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/special/class.ctor/p1.cpp?rev=174980&r1=174979&r2=174980&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/CXX/special/class.ctor/p1.cpp (original)
>> +++ cfe/trunk/test/CXX/special/class.ctor/p1.cpp Tue Feb 12 11:27:41 2013
>> @@ -1,5 +1,5 @@
>>  // RUN: %clang_cc1 -fsyntax-only -verify %s
>> -// expected-no-diagnostics
>> +
>>  struct X0 {
>>    struct type { };
>>
>> @@ -41,3 +41,7 @@ template<typename T> X1<T>::X1() { }
>>  template<typename T> (X1<T>::X1)(double) { }
>>  template<typename T> X1<T> X1<T>::f1(int) { return 0; }
>>  template<typename T> X1<T> (X1<T>::f1)(type) { return 0; }
>> +
>> +class A {
>> +  A::A(); // expected-error{{extra qualification on member 'A'}}
>> +};
>>
>> Modified: cfe/trunk/test/SemaCXX/MicrosoftExtensions.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/MicrosoftExtensions.cpp?rev=174980&r1=174979&r2=174980&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/SemaCXX/MicrosoftExtensions.cpp (original)
>> +++ cfe/trunk/test/SemaCXX/MicrosoftExtensions.cpp Tue Feb 12 11:27:41
>> 2013
>> @@ -204,3 +204,7 @@ struct PR11150 {
>>  void f() { int __except = 0; }
>>
>>  void ::f(); // expected-warning{{extra qualification on member 'f'}}
>> +
>> +class C {
>> +  C::C(); // expected-warning{{extra qualification on member 'C'}}
>> +};
>>
>
> We used to accept this code but now reject it:
>
> typedef int T;
> struct X { X::X(T()); } x;
> void f() { X().T(); }
>
> I'm not entirely sure whether the old or new behavior is correct (the new
> behavior matches g++ and EDG, though).
>

OK, I think under the resolutions of core issue 1435 and particularly 1310,
the new behavior is correct. Please add a test for that!
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130212/ed0a1c2d/attachment.html>