[PATCH] D133622: [clang][test] Disallow using the default module cache path in lit tests

Ben Langmuir via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 14 14:26:19 PDT 2022


benlangmuir added a comment.

In D133622#3788218 <https://reviews.llvm.org/D133622#3788218>, @bruno wrote:

>> I'm not sure how to deal with missing `env -u`.
>>
>> - We could do `env CLANG_MODULE_CACHE_PATH=` and change the compiler's interpretation of empty string for this variable. I'm not sure if the current behaviour (there will be no module cache in the cc1 at all) is intentional or useful.  Hesitant to change this behaviour.
>
> How about using `self.with_environment('CLANG_MODULE_CACHE_PATH', '')` so we don't need to worry about using `env -u` to unset? My understand is that (1) `getDefaultModuleCachePath` is the only place using `CLANG_MODULE_CACHE_PATH`, and (2) `std::getenv` return nullptr if it's empty, which will fallback to system provided path instead.

Where are you thinking we would call `self.with_environment` in this case? We explicitly do not want the system-provided path in most tests.  I think we would need to set it to `None`, since

> (2) `std::getenv` return nullptr if it's empty, which will fallback to system provided path instead.

getenv returns empty string, not nullptr.

> Not sure it helps much but according to `option-X.test`, `system-aix` support `unset`.

Heh, I'm worried we'll just hit an issue with a different platform (Windows?).  If we can't find a better fix I guess I can at least attempt it and see what breaks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133622



More information about the llvm-commits mailing list