[PATCH] D154130: [lit] Avoid os.path.realpath on Windows due to MAX_PATH limitations

Tristan Labelle via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jul 12 10:59:40 PDT 2023


MrTrillian added a subscriber: benlangmuir.
MrTrillian added a comment.

In D154130#4486898 <https://reviews.llvm.org/D154130#4486898>, @tahonermann wrote:

>> 95% of the %>t are around clang modulemap files, because that code resolves real paths in C++ by design, so I can't avoid it.
>
> Can you link to evidence that this behavior is by design? It isn't obvious to me why modulemap files would demand different behavior; especially since that would exacerbate `MAX_PATH` problems.

I agree with you. The original rationale seems to be here:
https://github.com/llvm/llvm-project/blob/926f3759ec62a8f170e76a60316cc0bdd9dd2ec9/clang/lib/Lex/HeaderSearch.cpp#L257

  cpp
      // To avoid false-negatives, we form as canonical a path as we can, and map
      // to lower-case in case we're on a case-insensitive file system.

And this change has more context: https://reviews.llvm.org/D134923 by @benlangmuir

> I'm not fond of the `safe_abs_path` name; "safe" doesn't communicate anything and the implementation is no more safe than `os.path.abspath` or `os.path.realpath`. Suggested alternatives:
>
> - `short_abs_path`
> - `shortest_abs_path`
> - `abs_path_no_subst_drive`
> - `abs_path_preserve_drive`

Thanks for the name suggestions, those are better indeed! I will update with the last one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154130



More information about the cfe-commits mailing list