This revision was automatically updated to reflect the committed changes.
Closed by commit rC332639: Fix a mangling failure on clang-cl C++17 (authored
by rnk, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D46929?vs=147244=147359#toc
Repository:
rC Clang
thakis added a comment.
Please do, tzik doesn't have commit access yet.
Repository:
rC Clang
https://reviews.llvm.org/D46929
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
lgtm
Shall I go ahead and commit this for you?
Comment at: lib/AST/VTableBuilder.cpp:2507
const MethodInfo = I.second;
+ assert(MD == MD->getCanonicalDecl());
+
tzik added a comment.
In https://reviews.llvm.org/D46929#1101780, @rnk wrote:
> I searched around, and I noticed that `VTableContext::getMethodVTableIndex`
> has the same implied contract that the caller must always provide a canonical
> declaration or things will break. It looks like it has
tzik updated this revision to Diff 147244.
Repository:
rC Clang
https://reviews.llvm.org/D46929
Files:
lib/AST/VTableBuilder.cpp
lib/CodeGen/CGCXX.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/CodeGen/MicrosoftCXXABI.cpp
test/CodeGenCXX/PR37481.cpp
Index: test/CodeGenCXX/PR37481.cpp
rnk added a comment.
I searched around, and I noticed that `VTableContext::getMethodVTableIndex` has
the same implied contract that the caller must always provide a canonical
declaration or things will break. It looks like it has three callers, and they
all do this. We should probably sink the
tzik created this revision.
tzik added reviewers: majnemer, rnk.
Herald added a subscriber: cfe-commits.
MethodVFTableLocations in MigrosoftVTableContext contains canonicalized
decl. But, it's sometimes asked to lookup for non-canonicalized decl,
and that causes assertion failure, and compilation
