[PATCH] D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used

David Blaikie via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 30 12:32:59 PDT 2019


dblaikie added a comment.

In D38061#1484568 <https://reviews.llvm.org/D38061#1484568>, @wenlei wrote:

> In D38061#1484486 <https://reviews.llvm.org/D38061#1484486>, @dblaikie wrote:
>
> > Seems like the right thing would be for the DWARF code that wants a rendered type name to pass its own printing policy, rather than changing some relatively global one.
> >
> > (though also I have my doubts about the whole approach - macro expansion can change the line number as well as the column number, so only suppressing column number would be insufficient - and this also reduces the usability for users (because the file/line/column of an anonymous type is a useful debugging aid). Seems to me like this should be opt-in if it's supported at all - though ideally build systems would use -frewrite-includes rather than full preprocessing, and then macros and lines/columns would be preserved, I think)
>
>
> Line number is ok because preprocessor also inserts #linemarker, and later on these markers are used to recover the original line number before macro expansion. Column is problematic, as there's nothing like a column marker (if possible at all) so there's no way to see through the column change from macro expansion. We tried -frewrite-includes too, but it has its own issues - without macro expansion, it bloats the file size that needs to send to distcc remote machine significantly.


Do you have some data on that growth/size comparison & its overall performance impact on the build? I'd be curious.

But, yeah, fair point about the line markers - I'd thought a multiline macro would break that, but seems not (it gets stamped out by the preprocessor all on one line anyway).

Still - figure this should be opt-in and done at the debug info level, not by changing a global printing policy. (& I'd still suggest using -frewrite-includes because Clang changes its diagnostic behavior based on the presence of macros to reduce diagnostic false positives, so you'll be degrading the usability in other ways by compiling fully preprocessed code)


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D38061/new/

https://reviews.llvm.org/D38061





More information about the cfe-commits mailing list