[libcxx-commits] [PATCH] D139270: [libc++] Rename __tuple to __tuple_dir to avoid file collision

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a comment.

In D139270#4005162 <https://reviews.llvm.org/D139270#4005162>, @mgorny wrote:

>> We also have the same problem for __string unless I am mistaken, @mgorny
>
> Actually, `__string` was granularized in LLVM 15 already, so I don't think that's necessary. However, I can still do it if you preferred.

Oh, really? Ok, then leave it out.

In D139270#4005151 <https://reviews.llvm.org/D139270#4005151>, @mgorny wrote:

> It's not perfect but it's good enough. I suppose you'll also accept future one-release renames if similar problems occur?

Yes, in fact the expectation would be that we rename them to `__foo_dir` immediately upon creating them if we remember about this (which I hope we will, and also the risk for this happening again is not suuuuper high since we don't have that many headers in that category).

> Also, I'd really prefer if you didn't call it "deficiency in the package manager" because I find that insulting to the lot of effort many of us put in the package managers. It's not a "deficiency", it's a conscious decision not to add a lot of complexity combined with fragility to handle a corner case affecting at most a few (libc++ is the only one I'm aware of) out of 20000+ packages.

Don't get me wrong, I think everybody here has tremendous respect for folks who handle distributions. From our perspective though, it seems kind of surprising that making a seemingly valid and simple change to our code organization would trigger an issue much further down the distribution pipeline due to a directory/file mismatch. But like I said, I don't think it matters much -- the workaround is simple and temporary, so we'll try to remember to do it on a best effort basis and everyone should be happy.



================
Comment at: libcxx/utils/generate_iwyu_mapping.py:34
     for i in detail_directories:
-        result.append(f'{generate(f"@<{i}/.*>", i[2:])},')
+        result.append(f'{generate(f"@<{i}/.*>", i[2:].rstrip("2"))},')
 
----------------
tahonermann wrote:
> This is a change I would object to on the basis that it is fragile.
I actually don't quite understand why this change is needed, can you explain? Sorry if that's simple.


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

https://reviews.llvm.org/D139270