[PATCH] D72982: [Clang] Un-break scan-build after integrated-cc1 change

Hans Wennborg via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 21 10:20:01 PST 2020


hans added a comment.

In D72982#1831648 <https://reviews.llvm.org/D72982#1831648>, @xazax.hun wrote:

> In D72982#1831595 <https://reviews.llvm.org/D72982#1831595>, @hans wrote:
>
> > Wait, do we really want the "(in-process)" marker to be written to a separate line? I'm not sure that we do.
>
>
> Since the `-###` command had this property of emitting copy pastable `-cc1` invocations I would be surprised if `scan-build` would be the only tool/script to rely on this. Whatever fix we come up with I think it would be great to maintain this property.


The way I see it, the purpose of -### was only ever for debugging and testing. If tools are parsing it, they need to deal with changes to the format. The cc1 interface is also not stable, so such tools should probably be used to adapting.

I think in this case, it would be better to fix scan-build to handle the prefix. Also, doesn't it need to do that anyway if it's printed on a separate line?

In D72982#1831692 <https://reviews.llvm.org/D72982#1831692>, @aganea wrote:

> In D72982#1831595 <https://reviews.llvm.org/D72982#1831595>, @hans wrote:
>
> > Wait, do we really want the "(in-process)" marker to be written to a separate line? I'm not sure that we do.
>
>
> Do you see value in keeping "(in-process)" at all? I added it in the first place for testing.


Yes, I think it's good for testing and debugging, which is the purpose of -###.

> 
> 
>> The change description doesn't explain how scan-build was broken or how this fixes it.
> 
> Fixed in the summary above. Should I revert the patch and recommit with a proper description/proper fix?

I think we should revert the patch and fix scan-build instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72982





More information about the cfe-commits mailing list