This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by shivanshu3 on Nov 9 2020, 11:52 PM.

Details

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);
            ^

Diff Detail

Event Timeline

shivanshu3 created this revision.Nov 9 2020, 11:52 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 9 2020, 11:52 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
shivanshu3 requested review of this revision.Nov 9 2020, 11:52 PM

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

shivanshu3 edited the summary of this revision. (Show Details)

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

shivanshu3 updated this revision to Diff 308827.Dec 1 2020, 5:50 PM

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

rsmith added inline comments.Dec 15 2020, 2:37 PM
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?

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

shivanshu3 marked an inline comment as done.Dec 15 2020, 3:25 PM
shivanshu3 added inline comments.
clang/lib/Parse/ParseExpr.cpp
2281–2295

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

rsmith accepted this revision.Dec 16 2020, 11:49 AM
This revision is now accepted and ready to land.Dec 16 2020, 11:49 AM
This revision was landed with ongoing or failed builds.Dec 16 2020, 12:14 PM
This revision was automatically updated to reflect the committed changes.