Sorry for the delay. LGTM.

Nice catch. This LGTM.

In D30882#1075461, @dexonsmith wrote:

In D30882#1075407, @ddunbar wrote:

In D30882#1074822, @dexonsmith wrote:

I don't think this is quite right. I know at least make-based incremental builds wouldn't deal well with this.

This is actually not a novel problem w.r.t. this patch. The exact same situation comes up with Makefile-included .d files and when one of the referenced headers is removed.

This is typically solved somewhere in the build system, for example Make has -include, and Ninja and llbuild have explicit support for this situation.

Of course, yes, I was wrong. Thanks for the correction.

I agree we might want to tread cautiously on how we change the .d output, but in this case I think it might be safe.

If we decide this isn't safe, then we may want to consider a new flag which tracks *all* anti-dependencies (file's for which Clang checked existence but did not exist), and include that here. The major concern with that approach is it is a much larger list, and while it would support being substantially more correct, it is also well beyond what people currently expect out of the build system + compiler generated deps approaches.

Even though this is likely safe, it seems really noisy. Consider the case where someone has __has_include(<missing>) -- we'll get an entry for every -I, -F, -isystem, and -iframework. If we're going to add that noise, I prefer a separate flag that covers all anti-dependencies.

This all looks reasonable. Its a nice localized change to this pass. LGTM.

Good catch! LGTM.

This is a great improvement over what we had. LGTM.

Sorry for forgetting to review this.

I scrolled through fairly quickly, but everything looked fine. Looks like basically renaming and fixing formatting along the way.

In D30723#695798, @jordan_rose wrote:

Thanks, Pete! Will land soon, then.

I think we could support SpecificBumpPtrAllocator but it would require walking the objects by 'sizeof(T) + RedZoneSize'. RedZoneSize itself would also have to be set to make sure we continue to have appropriate alignment. Not something you need to do now, but could be interesting in future.

FWIW, this doesn't work as is because SpecificBumpPtrAllocator lets you allocate *arrays* of objects as well as single objects, and the arrays obviously can't have red zones between elements.

Good point. Never thought of that. Wouldn't be surprised if there's a whole bunch of users who never use arrays and could benefit, but still something to think about more.

Very cool. Justin and I looked at ASan on BumpPtrAllocator a while ago and I think this would have been useful then too. I think the reason you didn't find issues is that he fixed many of them already.

Whats the behavior here in terms of implicit conversions and overloading?

Thanks for the quick review Fred. Those comments make complete sense so have committed with those changes.

Thanks for the quick review! r271020.

Yeah, that doesn’t seem to be particularly useful. The small number of tests you had to update shows just how few people probably even knew it existed.

LGTM. Thanks for fixing this so quickly.

Thanks for the quick review! r263774