rnk marked 2 inline comments as done.
rnk added a comment.
In D62975#1533019 <https://reviews.llvm.org/D62975#1533019>, @inglorion wrote:
> Can you clarify "which will usually result in a linker error"? E.g. an
> example of link.exe rejecting the object file or the wrong function being
> called.
Their linker gives an error, but then it tries to be helpful:
$ cat b.c
struct Foo {int x; };
int __fastcall bar(struct Foo o) { return o.x + 42; }
extern int (__fastcall *fp)(struct Foo);
int main() {
struct Foo o = { 13 };
return (*fp)(o);
}
$ cat t.c
struct Foo;
int __fastcall bar(struct Foo o);
int (__fastcall *fp)(struct Foo) = &bar;
$cl -c t.c b.c && link t.obj b.obj -out:t.exe
Microsoft (R) C/C++ Optimizing Compiler Version 19.20.27508.1 for x86
Copyright (C) Microsoft Corporation. All rights reserved.
t.c
b.c
Generating Code...
Microsoft (R) Incremental Linker Version 14.20.27508.1
Copyright (C) Microsoft Corporation. All rights reserved.
t.obj : error LNK2001: unresolved external symbol @bar@0
Hint on symbols that are defined and could potentially match:
@bar@4
t.exe : fatal error LNK1120: 1 unresolved externals
> The reason I ask is that if we can be sure at compile time that the resulting
> code will not work or do the wrong thing, I think giving an error is
> appropriate. But if that isn't the case, we would be rejecting code that
> cl.exe accepts and we might want to make it a Warning instead.
I guess the only reason I made it a hard error is that we'd have to teach
codegen to tolerate incomplete types if we make it a warning that can be
bypassed. But that might be worth doing anyway. We'd just do what they do,
mangle to `@0`.
================
Comment at: clang/lib/Sema/SemaExpr.cpp:14801
+ const llvm::Triple &TT = S.Context.getTargetInfo().getTriple();
+ if (!TT.isOSWindows() || (TT.getArch() != llvm::Triple::x86 &&
+ TT.getArch() != llvm::Triple::x86_64))
----------------
inglorion wrote:
> Do we need those checks or would it be enough to just check the calling
> convention?
>
> Also, I think s/Do nothing/return false/
I think you can use the calling conventions on non-Windows, but you don't get
the mangling, so I think this should return false. Non-x86 architectures
probably ignore the __fastcall qualifier with a warning, so we don't need the
arch check.
================
Comment at: clang/lib/Sema/SemaExpr.cpp:14813
+ default:
+ break;
+ }
----------------
inglorion wrote:
> You can just return false here.
I did it this way in an attempt to avoid worrying about the possibility of
compilers that warn about falling off the end of the function. Such compilers
used to exist, I don't know if we still support them, but I didn't want to find
out, so I arranged it this way.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62975/new/
https://reviews.llvm.org/D62975
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
