This is an archive of the discontinued LLVM Phabricator instance.

Improvements on Diagnostic in Macro Expansions
ClosedPublic

Authored by zhengkai on Aug 5 2015, 2:01 PM.

Details

Summary

Add two helper functions to check if the current macro expansion contains enough information for the error/warning message that we don't need further expansion.

Modify two test files which generates too verbose information in macros.

Add two test files. reduced-diags-macros is to check the different solutions when different error messages are generated. reduced-diags-macros-backtrace is to check the correctness of macro back trace limits in our new methods.

Diff Detail

Repository
rL LLVM

Event Timeline

zhengkai updated this revision to Diff 31391.Aug 5 2015, 2:01 PM
zhengkai retitled this revision from to Improvements on Diagnostic in Macro Expansions.
zhengkai updated this object.
zhengkai added a reviewer: rtrieu.
zhengkai added a subscriber: cfe-commits.
rtrieu edited edge metadata.Aug 5 2015, 2:54 PM

Some questions about your tests.

test/Misc/reduced-diags-macros-backtrace.cpp
27 ↗(On Diff #31391)

Why are macro notes printed after this one? Clang found 'p' so it does not need to give any more information.

36 ↗(On Diff #31391)

Same question, why are notes still printed after this one?

53–78 ↗(On Diff #31391)

Did you mean "SKIP-NEXT"?

test/Misc/reduced-diags-macros.cpp
13–14 ↗(On Diff #31391)

If this had one more macro expansion, would it be printed or not?

27–29 ↗(On Diff #31391)

How is this different than the test for 'b' above? Specifically, why does the error for 'b' print no macros notes while the error for 'x' prints two notes?

zhengkai updated this revision to Diff 31425.Aug 5 2015, 5:58 PM
zhengkai edited edge metadata.

Bug fixed

zhengkai marked 3 inline comments as done.Aug 5 2015, 6:01 PM
rtrieu added a comment.Aug 6 2015, 4:12 PM

Which bug did you fix?

Also, are you planning to add more test cases? Something that impacts all macro printing should be well-tested.

The helper function checkRangesForMacroArgExpansion is to check if the current Loc and Ranges are expansions of a macro's arguments.
If so, the IgnoredEnd will record this position and thus omitting all later expanded macros (Later expanded macros are showed on top of the stack though).

Two test cases are added.
The reduced-diags-macros-backtrace.cpp is to show that my method works correctly with the option -fmacro-backtrace-limit
The reduced-diags-macros.cpp is to show that my method reduced verbose information printed indeed.

There are still a lot of other cases in which my method still works as the old version did.
And I think there are several bugs in the existing functions like "emitDiagnostic" and "mapDiagnosticRanges".
This is issued as a bug, the link is https://llvm.org/bugs/show_bug.cgi?id=24423.
Future work will be done to improve the current result.

test/Misc/reduced-diags-macros.cpp
14–15 ↗(On Diff #31425)

Yes.
This works like the old version, should be improved later

There are still a lot of other cases in which my method still works as the old version did.
And I think there are several bugs in the existing functions like "emitDiagnostic" and "mapDiagnosticRanges".
This is issued as a bug, the link is https://llvm.org/bugs/show_bug.cgi?id=24423.
Future work will be done to improve the current result.

The bug you linked to is a tracking bug for all macro improvements. File bugs on specific examples you encounter and link it to that bug.

test/Misc/reduced-diags-macros.cpp
14–15 ↗(On Diff #31425)

File this issue as another bug.

rtrieu accepted this revision.Aug 12 2015, 11:20 AM
rtrieu edited edge metadata.

This is a first step towards making macro diagnostics better.

This revision is now accepted and ready to land.Aug 12 2015, 11:20 AM
This revision was automatically updated to reflect the committed changes.