[PATCH] D134784: [IR] Don't allow DLL storage-class and local linkage

[ben via Phabricator via llvm-commits]

bd1976llvm marked 5 inline comments as done.
bd1976llvm added a comment.

In D134784#3822333 <https://reviews.llvm.org/D134784#3822333>, @MaskRay wrote:

> In D134784#3822303 <https://reviews.llvm.org/D134784#3822303>, @bd1976llvm wrote:
>
>> In D134784#3821336 <https://reviews.llvm.org/D134784#3821336>, @MaskRay wrote:
>>
>>> For Clang, it seems that MSVC allows dllspec on a local linkage variable, so that's why we have to add it to CodeGen instead of rejecting it in Sema.
>>
>> Can you give an example? MSVC certainly rejects simple cases e.g. https://godbolt.org/z/bG3M7oj5W. I am worried that I have missed something...
>
> `__declspec(dllexport) const int foo = 10;` foo with internal linkage is accepted.

Thanks. I have reviewed the discussion here: https://reviews.llvm.org/D45978 and I experimented with the MSVC toolchain.

In D134784#3824309 <https://reviews.llvm.org/D134784#3824309>, @rnk wrote:

> In D134784#3822333 <https://reviews.llvm.org/D134784#3822333>, @MaskRay wrote:
>
>> `__declspec(dllexport) const int foo = 10;` foo with internal linkage is accepted.
>
> I believe in this case MSVC will make the symbol external, so it's not actually internal. The dll attributes tend to imply `extern` in many cases.

Thanks. Interestingly, MinGW seems to reject such constructs which is also the behaviour that we implement for PlayStation.

In conclusion, as MaskRay alluded to upthread, this anomaly means that this restriction can't be applied during Sema. However, it does make sense for the IR.


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

https://reviews.llvm.org/D134784