Nested macros producing different locations due to whitespace changes

[Quuxplusone]

Bugzilla Link	PR26247
Status	NEW
Importance	P normal
Reported by	rtrieu@google.com
Reported on	2016-01-21 18:37:37 -0800
Last modified on	2018-10-25 20:12:01 -0700
Version	trunk
Hardware	PC Linux
CC	llvm-bugs@lists.llvm.org, richard-llvm@metafoo.co.uk
Fixed by commit(s)
Attachments	macro-location.cc (276 bytes, text/x-c++src)
Blocks
Blocked by
See also

Created attachment 15686
Test Case

I was attempting to track down an issue with a warning and nesting macros,
specifically the NULL macro.  That was when I discovered the warning behavior
was dependent on the whitespace inside one of the macro definitions.  In the
attached test case, a warning is generated for "assert(test1());" while no
warning is emitted for "assert(test2());"  However, the definition of test1 and
test2 only differ by two spaces.

The warning logic is in SemaChecking.cpp:7045 in the DiagnoseNullConversion
function.  The difference is that one will have the MacroName of "NULL" while
the one does not.  The warning depends on finding the NULL macro name to
perform the analysis properly.

From the lexing side, one location difference was found in TokenLexer.cpp:796
inside updateConsecutiveMacroArgTokens.  The extra whitespace pushes one of the
macros over the relative offset limit of 50, splitting the macro tokens among
different SLoc's.  Remove the condition of "RelOffs > 50" appears to fix the
SourceLocation issue, but I'm not sure what else with macro handling that
condition affects.

[Quuxplusone]

Attached macro-location.cc (276 bytes, text/x-c++src): Test Case

[Quuxplusone]

Removing "RelOffs > 50" creates additional problems, so that's not the answer.

One promising approach is to use the getImmediateMacroName from DiagnosticRender.cpp instead of the one from Lexer.

I am also looking at the where the locations are coming from during the creation of the Expr's.