New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
The linkage changes in r359260 break MIDL code #41162
Comments
assigned to @rnk |
Yes, this was unintended. (The code is invalid, but we support it under -fms-compatibility. Someone should probably file a bug on MIDL to get it to stop generating ill-formed code.) |
Should be fixed in r360637. |
I had to revert that in r360657 because it broke Chromium, see http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20190513/271324.htm |
Mostly unrelated, but: dmajor, do you actually need the _c.c files? If you pass |
I gave it a try, and it turns out that we do need them. Interesting tip though; thanks. |
I chatted with Richard about this, and I think we probably want to try to implement this compatibility hack a bit more intentionally, i.e. actually take some action when we encounter these things. Someone must have added this compatibility hack earlier on in the project history, and simply downgraded the error into a warning, which doesn't actually implement MSVC's behavior. The fix is to finish the job. |
rnk, just checking, is this still on your radar? |
I'd say it's fallen off of the end of my priority list, unfortunately. I think the agreed upon behavior is that we:
All of those change are only when compiling C code, as in MIDL generated code. MSVC leaves const globals with extern declarations as external when compiling C++, which is clang's new behavior, so we should just warn and drop the static storage class specifier like we do today. |
The link in comment 3 doesn't work, the URL for the Chromium reproducer should be: http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20190513/271324.html (htm -> html) |
Nominating this as a 9.0.0 blocker since it's a regression from the previous release. |
It doesn't sound like there's any traction here. I don't think we can block the 9 release on this, but hopefully it can get fixed for 9.0.1. |
mentioned in issue llvm/llvm-bugzilla-archive#42011 |
mentioned in issue #42705 |
Extended Description
The following code builds successfully with r359259 but not r359260:
$ cat a.cpp
extern const int x;
static const int x = 3;
const int* foo() { return &x; }
$ cat b.cpp
extern const int x;
static const int x = 7;
const int* bar() { return &x; }
$ cat c.cpp
const int* foo();
const int* bar();
int main(int argc, char**) { return argc ? *foo() : *bar(); }
$ clang-cl -c a.cpp b.cpp c.cpp
$ lld-link -nodefaultlib -entry:main a.obj b.obj c.obj
lld-link: error: duplicate symbol: int const x in a.obj and in b.obj
This pattern can be seen in code generated by MIDL, e.g. https://searchfox.org/mozilla-central/search?q=text%3AHandlerData__MIDL_ProcFormatString
Since there are no templates involved here, I'm guessing this change in behavior was unintended?
The text was updated successfully, but these errors were encountered: