Print source location in the error message when parens are missing around sizeof typename and the expression is inside macro expansion

[shivanshu3]

Summary

Given the following code:

void Foo(int);

#define Bar(x) Foo(x)

void Baz()
{
	Bar(sizeof int);
}

The error message which is printed today is this:

error: expected parentheses around type name in sizeof expression

There is no source location printed whatsoever, so fixing a compile break like this becomes extremely hard in a large codebase.

My change improves the error message. But it doesn't output a FixItHint because I wasn't able to figure out how to get the locations for left and right parens. So any tips would be appreciated.

<source>:7:6: error: expected parentheses around type name in sizeof expression
        Bar(sizeof int);
            ^

[shivanshu3]

Note that this regression seems to be there ever since Clang 3.4.1

[shivanshu3]

Use getFileLoc instead of getExpansionLoc to get the location of error.

[shivanshu3]

Clang format the test file. Forgot to do it before.

[shivanshu3]

Ping!

[rsmith]

clang/lib/Parse/ParseExpr.cpp

2281–2295

I don't think we should be assuming that getLocForEndOfToken will fail if and only if the token is a macro -- that seems brittle. It would seem better to check whether it actually failed and respond to that directly. Would something like this suggestion work?

[shivanshu3]

Don't assume that getLocForEndOfToken will fail if and only if the token is a macro

[shivanshu3]

clang/lib/Parse/ParseExpr.cpp

2281–2295

Yup, I like that better, thank you for the suggestion!

clang/lib/Parse/ParseExpr.cpp

if (Tok.isNot(tok::l_paren)) {
     if (OpTok.isOneOf(tok::kw_sizeof, tok::kw___alignof, tok::kw_alignof,
                       tok::kw__Alignof)) {
       if (isTypeIdUnambiguously()) {
         DeclSpec DS(AttrFactory);
         ParseSpecifierQualifierList(DS);
         Declarator DeclaratorInfo(DS, DeclaratorContext::TypeNameContext);
         ParseDeclarator(DeclaratorInfo);
 
-        SourceLocation LParenLoc = PP.getLocForEndOfToken(OpTok.getLocation());
+        SourceLocation OpTokLoc = OpTok.getLocation();
+        if (OpTokLoc.isMacroID()) {
+          SourceLocation OpTokExpansionLoc =
+              PP.getSourceManager().getFileLoc(OpTokLoc);
+          Diag(OpTokExpansionLoc,
+               diag::err_expected_parentheses_around_typename)
+              << OpTok.getName();
+        } else {
+          SourceLocation LParenLoc = PP.getLocForEndOfToken(OpTokLoc);
           SourceLocation RParenLoc = PP.getLocForEndOfToken(PrevTokLocation);
           Diag(LParenLoc, diag::err_expected_parentheses_around_typename)
-          << OpTok.getName()
-          << FixItHint::CreateInsertion(LParenLoc, "(")
+              << OpTok.getName() << FixItHint::CreateInsertion(LParenLoc, "(")
               << FixItHint::CreateInsertion(RParenLoc, ")");
+        }
         isCastExpr = true;
         return ExprEmpty();
       }
     }
 
     isCastExpr = false;
     if (OpTok.is(tok::kw_typeof) && !getLangOpts().CPlusPlus) {
       Diag(Tok, diag::err_expected_after) << OpTok.getIdentifierInfo()
                                           << tok::l_paren;

clang/test/Parser/sizeof-missing-parens.c

+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+void Foo(int);
+
+#define Bar(x) Foo(x)
+
+void Baz()
+{
+  Foo(sizeof int); // expected-error {{expected parentheses around type name in sizeof expression}}
+  Bar(sizeof int); // expected-error {{expected parentheses around type name in sizeof expression}}
+}