Implement MSVC's language extension to form pointers to members of virtual bases

[rnk]

Bugzilla Link	15713
Resolution	FIXED
Resolved on	Dec 02, 2019 13:50
Version	unspecified
OS	Windows NT
Blocks	#14079
CC	@XVilka,@DougGregor,@nico,@timurrrr

Extended Description

Clang currently rejects this program, as the standard says it is ill-formed in [conv.mem], I think:

struct A { int a; };
struct B : virtual A { int b; };
int B::*memptr_b = &B::a; // error

memptr_data_virtual.cpp:8:20: error: conversion from pointer to member of class 'A' to pointer to member of class 'B' via virtual base 'A' is not allowed
int B::*memptr_b = &B::a; // error

MSVC accepts this program, and has a whole bunch of machinery baked into the ABI to support it. Data member pointers can be formed with up to 3 (!) i32s.

There's basically no way to implement this within the Itanium ABI since data memptrs there are always just one offset, so this would only be in -fms-compatibility (or -fms-extensions, I forget which is which).

Mostly I'm just filing this to have a discussion of record with a decision to implement or not. We should document this as part of http://clang.llvm.org/compatibility.html, probably.

[timurrrr]

I think it's perfectly fine to not support it until we observe a real code that actually uses it and has some very strong reasons to use this bad pattern.

[rnk]

Right, I'm mostly just filing the PR to document the state.

FWIW I think John said it best in the code review. This isn't a bad pattern, it's just a feature that MSVC implemented which was pared down during standardization.

[nico]

r240383, r240384 removed a few FIXMEs pointing to this bug. (The bug is still valid though.)

Since we haven't needed this yet, it's probably ok to WontFix it if it's only implementable on Windows.

[XVilka]

Is this bug still relevant? Because it is mentioned at the MSVC compatibility page and was updated 4 years ago...

https://clang.llvm.org/docs/MSVCCompatibility.html

[rnk]

Yeah, we implemented this a long time ago.

[rnk]

mentioned in issue llvm/llvm-bugzilla-archive#15875

[majnemer]

mentioned in issue llvm/llvm-bugzilla-archive#23452