[cfe-commits] r146480 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaDecl.cpp test/CXX/dcl.decl/dcl.meaning/p1-0x.cpp test/CXX/expr/expr.prim/expr.prim.general/p8-0x.cpp

[David Blaikie]

On Wed, Dec 14, 2011 at 7:58 AM, Douglas Gregor <dgregor at apple.com> wrote:
>
> On Dec 13, 2011, at 12:03 AM, David Blaikie wrote:
>
>> Author: dblaikie
>> Date: Tue Dec 13 02:03:36 2011
>> New Revision: 146480
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=146480&view=rev
>> Log:
>> Disallow decltype in qualified declarator-ids.
>>
>> Added:
>>    cfe/trunk/test/CXX/dcl.decl/dcl.meaning/p1-0x.cpp
>> Modified:
>>    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>>    cfe/trunk/lib/Sema/SemaDecl.cpp
>>    cfe/trunk/test/CXX/expr/expr.prim/expr.prim.general/p8-0x.cpp
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=146480&r1=146479&r2=146480&view=diff
>> ==============================================================================
>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Dec 13 02:03:36 2011
>> @@ -1144,6 +1144,8 @@
>> def warn_cxx98_compat_decltype : Warning<
>>   "'decltype' type specifier is incompatible with C++98">,
>>   InGroup<CXX98Compat>, DefaultIgnore;
>> +def err_decltype_in_declarator : Error<
>> +    "'decltype' cannot be used to name a declaration">;
>>
>> // C++11 auto
>> def warn_cxx98_compat_auto_type_specifier : Warning<
>>
>> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=146480&r1=146479&r2=146480&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue Dec 13 02:03:36 2011
>> @@ -3203,6 +3203,16 @@
>>          (S->getFlags() & Scope::TemplateParamScope) != 0)
>>     S = S->getParent();
>>
>> +  if (NestedNameSpecifierLoc SpecLoc =
>> +        D.getCXXScopeSpec().getWithLocInContext(Context)) {
>> +    while (SpecLoc.getPrefix())
>> +      SpecLoc = SpecLoc.getPrefix();
>> +    if (dyn_cast_or_null<DecltypeType>(
>> +          SpecLoc.getNestedNameSpecifier()->getAsType()))
>> +      Diag(SpecLoc.getBeginLoc(), diag::err_decltype_in_declarator)
>> +        << SpecLoc.getTypeLoc().getSourceRange();
>> +  }
>> +
>
> The code here looks correct (thanks!), but please:
>
>  (1) Move it down to where we do the other checks on the nested-name-specifier (with the comment that starts with "C++ 7.3.1.2p2")

Ah, sure thing. Moving this down puts it under a "Ss.isSet()"
condition, so my test case changes (we don't flag this error if the
CXXScopeSpec is already invalid). Should this condition be changed
above (line 3298) be changed from "if (!D.getCXXScopeSpec().isSet())"
to "if (D.getCXXScopeSpec().isEmpty())" ? or would that cause more
confusion/error noise than would be helpful. I suppose the other
errors in this block only work if name lookup succeeds, and doing name
lookup on an invalid scope spec isn't going to be useful (in which
case "if (D.getCXXScopeSpec().isValid())" might be the right thing).
Alternatively, my new test could be moved down outside the
if(SS.isSet())/else block but it'd still be close to the related
checks.

[side note: CXXScopeSpec.isSet is deprecated, but it has a simpler
implementation that "isValid", yet /seems/ to provide the same
guarantee - isSet checks the ScopeRep != 0, isValid tests for a
non-empty range and ScopeRep != 0. Is it possible to have a non-null
scoperep and an invalid/empty source range? If not then isSet and
isValid should use the same (simpler) implementation]

>  (2) Add a reference to the C++11 spec, quoting the paragraph that introduces this restriction.

I quoted the relevant sentence from that paragraph (I assume you
didn't want the whole paragraph, it's somewhat long-winded).

Interestingly there's a note in that paragraph (about what
nested-name-specifiers are allowed in declarator-ids) that says "If
the qualifier is the global :: scope resolution operator, the
declarator-id refers to a name declared in the global namespace
scope." Yet clang disallows this construct. I can't see any reason for
it to be a useful/valid construct - but is that just a spec
bug/oversight, or am I (& clang) missing something?

Changes committed as r146576.

- David