echristo added a comment. In D100776#2699603 <https://reviews.llvm.org/D100776#2699603>, @thakis wrote:
> In D100776#2699565 <https://reviews.llvm.org/D100776#2699565>, @rnk wrote: > >> Of the three people who commented on D17183 >> <https://reviews.llvm.org/D17183>, you and I are on the only ones in favor >> of this approach. I think @echristo and @jyknight both preferred approach 2. >> I'd like to get at least an agreement to disagree from one of them before >> approving this. > > That's 50% in people, and 15/24 voting by changes that landed in clang/ since > start of year ;) Don't know why 2/4 is "only". I mean that's fair ;) > I'm also not dismissing approach 2: I implemented it as well, and it's > ready-to-go as soon as someone wants to pursue the direction mentioned there. > If someone wants to do that, I don't have a problem with it, but until then > this here is simpler and more self-consistent since it preserves the original > design of TargetInfo. how about some historical context. I'm reasonably certain I was one of the motivators behind this change. The prior state often lead to either very subtle bugs that were hard to diagnose without an asserts build or a fairly inscrutable assert that the front and back ends disagreed on what context a string should have. The idea was that you could then ask the "backend" how to construct up a layout for what was wanted. As is mentioned there are tradeoffs around this though: a) it does make it harder to have clang generate code without a backend or llvm itself around, b) it does have a dependency when none existed. So, if this is really causing some consternation then we can pull back and reinstate what we had, but it was a direction around solving a set of hard to find bugs. Thoughts? -eric CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100776/new/ https://reviews.llvm.org/D100776 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
