<table border="1" cellspacing="0" cellpadding="8">
    <tr>
        <th>Issue</th>
        <td>
            <a href=https://github.com/llvm/llvm-project/issues/141165>141165</a>
        </td>
    </tr>

    <tr>
        <th>Summary</th>
        <td>
            Decide on how to handle differences in configurations between module dependencies
        </td>
    </tr>

    <tr>
      <th>Labels</th>
      <td>
            new issue
      </td>
    </tr>

    <tr>
      <th>Assignees</th>
      <td>
      </td>
    </tr>

    <tr>
      <th>Reporter</th>
      <td>
          matts1
      </td>
    </tr>
</table>

<pre>
    This is a follow-up to #131569. Please read that PR for context

@Bigcheese @ChuanqiXu9 @cor3ntin @mizvekov @atetubou FYI

The current problem is that there are several options such as RTTI and exceptions that affect the codegen phase, and not the generation of the AST. However, they unfortunately are visible through macros, and thus can indirectly affect the generation of the AST.

I have created a demo at https://github.com/matts1/demo/tree/module-config-mismatch which you're welcome to experiment and play around with. This is my summary of my conclusions from the demo:

### Likely changes based on macros
* Most of the time it will be perfectly safe, as header files are unaffected ([example](https://github.com/llvm/llvm-project/blob/d0acddbdd615f1503e88903570ee7484bd217615/libcxx/include/regex#L993-L1000))
* Sometimes dependent macros will be created that can be observed (eg. `LIB_HAS_EXCEPTIONS`)
* Sometimes methods may only appear ([example](https://github.com/llvm/llvm-project/blob/d0acddbdd615f1503e88903570ee7484bd217615/libcxx/include/any#L272-L280)) when a macro is / isn't defined.
* It is possible to have a scenario where the signature changes based on whether a macro is defined, though in practice I haven't seen any instance of this
  * Eg. `void foo()` for exceptions vs `Error foo()` for no-exceptions

## Without modules
If `user.cc` is built without exceptions, it naturally includes `lib.h` without exceptions, and thus it retrieves the version of the header file for no exceptions. Thus, linking against the version of `lib.o` built with exceptions may not work if the header files differ in a way that affects the API, or if you attempt to read macros that were based on `lib`'s `__cpp_exceptions` flag.

Thus, unless `lib.h` is agnostic to whether a given macro is defined, it does not make sense to be able to build `lib` with a macro defined, and the dependent library without.

## With modules
With modules, if a user of a library defines a macro, it is not visible to said library. Thus, it becomes possible to compile a library with different parameters to its users (on a purely technical level). However, whether we *should* do it is another question. So let's go through the demo.

Note that we need to decide on the semantics both with and without the `-W[no-]module-file-config-mismatch` flags.

### `-Wmodule-file-config-mismatch`
Current behaviour:
```
clang++ -cc1 -x c++ -emit-module -o out/modules/user_except_lib_noexcept.pcm -fmodules modules.modulemap -fmodule-name=user -fexceptions -fcxx-exceptions -fmodule-file=out/modules/lib_noexcept.pcm
error: exception handling was disabled in PCH file but is currently enabled
error: module file out/modules/lib_noexcept.pcm cannot be loaded due to a configuration mismatch with the current compilation [-Wmodule-file-config-mismatch]
2 errors generated.
```

I believe that the current behavior is the correct one here. While, as I said, it's technically possible to have one library depend on another with different configuration, it is a rare situation that should be used with an abundance of caution.

### `-Wno-module-file-config-mismatch`
#### Is it reasonable to depend on a mismatch?
I think my library demonstrates a reasonable use case for not just why we should allow depending across changed langopts, but also why things being observable through macros is the behaviour we want.
* The library changes header file contents based on whether exceptions are enabled for said library
* The library exposes this change through the macro `LIB_HAS_EXCEPTIONS`
* The user can read `LIB_HAS_EXCEPTIONS` to determine whether the dependent library was built with exceptions.

#### Current behaviour:
In summary, when there is a config mismatch, it will create a new module for the current configuration and use that. It will not truly allow a config mismatch.

```
clang++ -cc1 -x c++ -emit-module -o out/modules/user_except_lib_noexcept.pcm -fmodules modules.modulemap -fmodule-name=user -fexceptions -fcxx-exceptions -fmodule-file=out/modules/lib_noexcept.pcm -Wno-module-file-config-mismatch
```

The above command succeeds, but not in a way I believe to be correct.
1. It observes that the user module is attempting to depend on the `lib` module
2. It finds all instances of the `lib` module, and gets one with exceptions disabled.
3. It finds that there is no  configuration matching, and that we have `-Wno-module-config-mismatch`
4. It attempts to read the header file in a manner similar to no modules (ie. with the same header configuration)
5. It has now successfully compiled, but what it did was to silently compile against a version of `lib` which has exceptions

#### Proposed behaviour:
As such, I propose the following change:
* (unchanged) `LANGOPT`s cannot differ between a library and its user
* (unchanged) `BENIGN_LANGOPT`s don't affect the construction of the AST, and can thus differ safely
* (unchanged) `COMPATIBLE_LANGOPT`s affect the construction of the AST in a compatible way, and can thus differ safely
* (new) `MOSTLY_COMPATIBLE_LANGOPT`s (not satisfied with the name - suggestions welcome) do not directly affect the AST in incompatible ways, but are observable through the preprocessor, and thus may be different in arbitrary ways. When the flag "treat  mostly compatible as compatible" is set, these are treated as compatible. While this is not a guarantee of safety, this should generally work under any of the following conditions:
  * The library API does not change based on the macro (this should be true most of the time)
  * The library API does change based on the macro, but exposes a macro of its own to check whether it's enabled, and the user checks that macro when using parts of the library that change instead of the one for the option directly (eg. check `LIB_HAS_EXCEPTIONS` instead of `__cpp_exceptions`)

The reason I suggest a new flag rather than using the existing `-Wno-module-file-config-mismatch` is because of backwards compatibility - there may be some people who rely upon the existing behaviour. If this isn't something we're concerned about, I'm happy to just use that existing flag.
</pre>
<img width="1" height="1" alt="" src="http://email.email.llvm.org/o/eJzkWV1v47bS_jXKzcCGLMeJfZELJ9m0BtLdoBug7VVAkSOLjUSq_LDj99e_GJKSZSfZ7bk7wAEC7Domh_P5zDMTZq3cKsSbbHGbLe4vmHe1Njctc87OLkotDjfPtbQgLTCodNPo_cR34DRkxXw2ny2uVlN4apBZBINMgKuZg6ffodIGuFYO31yWr-nnMr-VW14jWoTsMr-rPVP_yD_9ij5xbebKSUX_b-X_7fBV7-j_zKHzpfbw8NcmynmuEbg3BpWDzuiywZa0C--6Gg0CMwgWd2hYA7pzUisL1vMamIXfn583wJQAfOOYvgtXWVUhDxKAa4FbVNDVzGJW3IXzSscvt6jQMLoIugq_WX9_nsKvek8v0mlX4wG8qrRxXjGHzSFotJNWlg2Cq4322xpaxo22vXhXewucKZBKSIPc0a2jSh-_Gh2ygZrtELhB5lAAA4GtBuagdq6z2XydFQ9Z8bCVrvbllOs2Kx5ifLPigc5mxYMziPRrLXyDE65VJbeTVtqWOV7Dvpa8hoP2WXFtEPbYcN0iJQG-dWhkS7EgK7qGkbHaKwF76eop9MnTHsD6tmXmQAa0B8oN3ngbAlAZ3QajgjbzdUqYYh5_4FG-khd5zdQWLZTMogCtehfS0TX8pq3rneNkiyAd7GXTQInQoamiTy2rYkgt1MgEGqhkgzZEyKvocRSQFctscYtvrO0azBb3WbH8gTebZtf_M-mM_hu5y4qHstEleThnXIhSiKvZopot8jkul6t8vrjOEa8vl5elKGbXV7MFCZAlf3vLigdJvhEUEYNbfMuK-eNqNZ88zvI8z4oV_USjv-sWyVgLAjtUggIRvTLY3udFyHLKsBJBlxbNLtqJ2ylkV_nj5vbl1_X3ly9_3n15et58-_o9u8o_eqhFV2thoWUH0IrStOuQmf8elzF1IIcV18XksVgmf8G-RgUsOocSMiseQFqVFdcOBFZSoZgmWzeODnTapoLVscAYWI6KGalJmMGQaYSezHmD77NzXyPh0fjR9FBEiYACUkFnGHeSI8RCjipZJHXVAaSyjimOMbUlZTsAafklxm2npYBKa3J_scqu8oC8I3TbWTr2xRht3p9TenI8Oi47-EOShg4iJNB3m4oEeYtmyjkJkBZKLxsXSp3OjkQVd1R_wTWsaciMEJ6gSyPLaU0CPr43wKF0YNAZiTu0wdk7NHYEgaMCTsaMJBH0-CCvkepVqi2wLSNnnktKCmlS6GjO2IOU6QT_e21eQb5724KQVYWGYslgzw7jhhIVXz9tSBNt6PpBe2DOYds5yq3QNVPNhot7yq0hi6J6oRavg_deXnjXvYxcRqFs2Hbat8dotVcN2lN3Uw_fKm2d5PTwMT-3cofqwyyVDoRGG8xv2Sv1VWVDSZQILFUHeU0cFY3-67N-JCwGFkdQ1cjSUEtIiTB9l4Gj9Dv5SKpVwICykYLIBlHxPdu_n4yQ0YShB2uwTIr-0jFXpIMSqbmd1j_XbUdZxk40TnEPRIQZ1qJDY-m4dDZoRiiz1JQVnTfUwRzyWknOGmhwh01WrE6YQx-RPVKF21r7RlCpC51sYEqHA_94tBT8KXzX0KALubHVA7foe2ly6FftsM8tUEjNgALDpUBKsYBj2DLlJLdQalenEKYuTiVKZ7KrfPJHtrhVepIt7hNXoAo4Jwx9StrpeScPIn58M8vXd4nelVizndTeJE5wlaeffM0bprZZcZsVtzDhfAaTN-D9Z2ylm8RXYKJBezdwG5sVDxSaVEAvjSxflI4fph1vYVKlg32uTeO_LeuG7yaKtZjN70P2TaoRVEwq_vY2OfnFyNpsfn-uy_n7Wb5Ggupsvj5CENRMiYYwbM8IbSxVniC8ebr7NeJf6UOCJGLcHABVODSWlzwSzv9MD2IKVDElQqOZQAHCh1JgEEPmEx09kkRKGTci57Fo4qlscfvjuC_us3xdQNDV9nQ3teRR2APdLbGhnjBQ_uHFlC8mzgNE5A1RadCKANvgFP6oKQyR_20CBsSqDwU0VGdzeN_9ScYRYwjAqHT6gjyDgxMXHSGIgQmziXQ-uiVYEOucPO0J81PpASu9En3n58yHev-wnpSe_LSkhit0a5N6K7Na9SA-smmIaTZ_CA53tVSvRNqPDmi1so5CFKw6SvIWgdMwGBuyg7-9dbCvDwQ9yVJGU2R6MDRm6n02ESgBVNm6cwGPKa1ZY3WQQGpsLZRIlyKFZe8nqj74A3jQy3umXE_vaIDsDelJ25hKhKlVuQ-Y3KiuKY6pxIKp43bywTv41mkbWIzsDT0B69grP6XhR4EBcojFB9rw2YUYUIemlQoH9T_pvcx-zHvOk40y5xNk3qh-ukttTKVRPCR9TMhjVsV6CPNJHE6AgcL9gE_anOHIGG-oKVGSUelMiaoHOWE6N56mkZBc7x7tjfkfaCHwU0R4h6mUWazUO0r-tiUXW885ohiKkBw88NsRBAcimHCWfDwLMUnz5XErE_M2-ZOSIvJfKuQT7Ek0IxHJeJ4aQ5BaSSUsBXgYimw_Cry7k-jmFp0N0H3O6fsmSjrPR9JHa6TAGeG83ZEDJeVNz2cjqwo94gyNPwTiy_BYst8OA8D5PCPjsKoUGrCylQ0zdFbpPqeIWkqcHtuuZe0g4qz_0BC_CO_WjIzax_BaW3nqdYndij7We7KJmL8UAR2ILcsm0oqBCadhir0fpcIIEDZG9NrH42WCkyejCRfFOZ6s48KOFNpAFw8FI-MGktImgmgihsWa3OFVaiE07xM0rr_-8u3pObvKbU9n0qRWotuHAXtAQYplT9s_l3j75evml68vY8FCx4H9ZH9IvdHzs3VdnzKE3mG-TcpYVmFz-PzRu2-_Pa2fN7ePX04e_vmDMYkoYswFLrNnh3-rhMJ9ev63b9-fH_96-UQLOqodWOakrWRPX0gBAjiYgPXbbRxWbL84JMFCQ4zH-11n0lyqU82PfMDgR92frnYGO6Mps7U5WSXQCF_iiKGRa0wpXWqBB0vcMLatMLpAVhSOmhNAq22f-UkbZkefsqIgqLA0hYXlr40baNfvY8eHEwONPCANpQy2nhmmHAaqR5FwhyiL5EbOFBkxVWvYQnhFZc7UoQ_3qDC0EjIWXCiOuC0ak5H10-Y41ScyMnCdERsplmMNSjLJY3DHeM8a4eUHr3z6Qh_Qnhz1KwNdhVLUexVm7xr568BhElXvp5vRViEyIzqcUDwKC2TEW_JMx4wbOkavZlyLRhUJ0QiM0xHqGz0ZiX9HOOZrWpxG5T6lYSOBH29u0oI1duBIo2kuiTWTaFFIR8MShWO9NaQVvkkbmui_mwPCxg45I_6kKygZf90zI44JKhvpDjBJDTAVjdUtQoe6ozqsqV81B_BdiuSgwgDhU9hUfYKnZaZuMZB32GP8EwLXiqNRVB5l4DN3sMmK6xZq1nUHCnuYG3qid3wlbLouxM1crOYrdoE3s-vL69VitbhaXtQ3OV7i1WwprmbIZ6trkc8KtlzNyqv5gnE2X17ImyIvFvmimOf5bFUsp6tKLHC1ENfL-Wq2LHl2mWPLZDNtml071WZ7Ia31eDO7nM2uFhcNK7Gx4Y9lRUHBCd9mRZEt7i_MTVhnl35rs8u8kdbZoxgnXYM398PSpdb7OFsq0RyBiSiNVKf92w7tKvGnnsNzifbCm-bmP96zB62JOyazdjfF_wcAAP__Ww-cTw">