[clang] Fix crash upon stray coloncolon token in C2x mode

[SuibianP]

Summary

The parser assumes that the lexer never emits coloncolon token for C code, but this assumption no longer holds in C2x attribute namespaces. As a result, stray coloncolon tokens out of attributes cause assertion failures and hangs in release build, which this patch tries to handle.

Crash input minimal example: T n::v

[inclyc]

clang/lib/Parse/ParseDecl.cpp

5340

Maybe we can make a new error diagnostic definition and fire that here?

clang/test/Parser/c2x-attributes.c

146

Could we improve this diagnostic message?

expected ';' after top level declarator}

[tbaeder]

clang/include/clang/Parse/Parser.h

863

[aaron.ballman]

Good catch that this was crashing! LGTM aside from the nit pointed out by @tbaeder. Can you also add a release note for the fix? Do you need someone to commit on your behalf? If so, what name and email address would you like used for patch attribution?

clang/lib/Parse/ParseDecl.cpp

5340

I don't think we should -- this function is used to query whether something is a declaration specifier or not; it'd be surprising to issue a diagnostic from that kind of interface.

clang/test/Parser/c2x-attributes.c

146

That might be possible, but it should happen as a separate patch. That said, I'm not certain how much improvement there is to be had there, especially in C mode. It really does look like the user is trying to declare a variable named n and got the wrong kind of colon.

[SuibianP]

Remove superfluous parentheses

[SuibianP]

Add release note entry

[SuibianP]

@aaron.ballman Thanks for the guidance! I have rectified the parentheses and appended to the release notes. Please feel free to point out any additional issues with the Differential.

If all is good, I would like to have the patch attributed to Jialun Hu <[e-mail]>.

[inclyc]

In D133248#3865010, @SuibianP wrote:

@aaron.ballman Thanks for the guidance! I have rectified the parentheses and appended to the release notes. Please feel free to point out any additional issues with the Differential.

If all is good, I would like to have the patch attributed to Jialun Hu <[e-mail]>.

Thanks @SuibianP ! I've committed this for you.

clang/include/clang/Parse/Parser.h

public:
   // If NeedType is true, then TryAnnotateTypeOrScopeToken will try harder to
   // find a type name by attempting typo correction.
   bool TryAnnotateTypeOrScopeToken();
   bool TryAnnotateTypeOrScopeTokenAfterScopeSpec(CXXScopeSpec &SS,
                                                  bool IsNewScope);
   bool TryAnnotateCXXScopeToken(bool EnteringContext = false);
 
   bool MightBeCXXScopeToken() {
-    return Tok.is(tok::identifier) || Tok.is(tok::coloncolon) ||
+    return (getLangOpts().CPlusPlus) &&
+           (Tok.is(tok::identifier) || Tok.is(tok::coloncolon) ||
             (Tok.is(tok::annot_template_id) &&
              NextToken().is(tok::coloncolon)) ||
-           Tok.is(tok::kw_decltype) || Tok.is(tok::kw___super);
+            Tok.is(tok::kw_decltype) || Tok.is(tok::kw___super));
   }
   bool TryAnnotateOptionalCXXScopeToken(bool EnteringContext = false) {
     return MightBeCXXScopeToken() && TryAnnotateCXXScopeToken(EnteringContext);
   }
 
 private:
   enum AnnotatedNameKind {
     /// Annotation has failed and emitted an error.

clang/lib/Parse/ParseDecl.cpp

case tok::kw_typename: // typename T::type
     // the start of a declaration.
     if (DisambiguatingWithExpression &&
         isStartOfObjCClassMessageMissingOpenBracket())
       return false;
 
     return isDeclarationSpecifier();
 
   case tok::coloncolon:   // ::foo::bar
+    if (!getLangOpts().CPlusPlus)
+      return false;
     if (NextToken().is(tok::kw_new) ||    // ::new
         NextToken().is(tok::kw_delete))   // ::delete
       return false;
 
     // Annotate typenames and C++ scope specifiers.  If we get one, just
     // recurse to handle whatever we get.
     if (TryAnnotateTypeOrScopeToken())
       return true;

clang/lib/Parse/Parser.cpp

if (ParsedType Ty = Actions.getTypeName(
 
       // In case the tokens were cached, have Preprocessor replace
       // them with the annotation token.
       PP.AnnotateCachedTokens(Tok);
       return false;
     }
 
     if (!getLangOpts().CPlusPlus) {
-      // If we're in C, we can't have :: tokens at all (the lexer won't return
-      // them).  If the identifier is not a type, then it can't be scope either,
-      // just early exit.
+      // If we're in C, the only place we can have :: tokens is C2x
+      // attribute which is parsed elsewhere. If the identifier is not a type,
+      // then it can't be scope either, just early exit.
       return false;
     }
 
     // If this is a template-id, annotate with a template-id or type token.
     // FIXME: This appears to be dead code. We already have formed template-id
     // tokens when parsing the scope specifier; this can never form a new one.
     if (NextToken().is(tok::less)) {
       TemplateTy Template;

clang/test/Parser/c2x-attributes.c

 
 // Do not allow 'using' to introduce vendor attribute namespaces.
 [[using vendor: attr1, attr2]] void f14(void); // expected-error {{expected ','}} \
                                                // expected-warning {{unknown attribute 'using' ignored}}
 
 struct [[]] S4 *s; // expected-error {{an attribute list cannot appear here}}
 struct S5 {};
 int c = sizeof(struct [[]] S5); // expected-error {{an attribute list cannot appear here}}
+
+// Ensure that '::' outside of attributes does not crash and is not treated as scope
+double n::v; // expected-error {{expected ';' after top level declarator}}