[PATCH] [ms-cxxabi] Mangle function pointer arguments correctly
Richard Smith
richard at metafoo.co.uk
Mon Jun 3 12:58:37 PDT 2013
================
Comment at: lib/AST/MicrosoftMangle.cpp:1217-1225
@@ -1210,11 +1216,11 @@
if (D) {
- // If we got a decl, use the type-as-written to make sure arrays
- // get mangled right. Note that we can't rely on the TSI
- // existing if (for example) the parameter was synthesized.
+ // If we got a decl, use it to get the source range. This used to be
+ // important for getting the type-as-written. We've since changed to
+ // traversing the type-as-written, so this shouldn't be necessary anymore.
for (FunctionDecl::param_const_iterator Parm = D->param_begin(),
ParmEnd = D->param_end(); Parm != ParmEnd; ++Parm) {
TypeSourceInfo *TSI = (*Parm)->getTypeSourceInfo();
QualType Type = TSI ? TSI->getType() : (*Parm)->getType();
mangleArgumentType(Type, (*Parm)->getSourceRange());
}
} else {
----------------
Reid Kleckner wrote:
> Richard Smith wrote:
> > Can you just use the parameter types from the FunctionProtoType here?
> Yes, but I either need to do a parallel iteration or lose the source range. What's preferable?
It seems fine to just pass in the source range for the function type; these source ranges are just used for emitting "we can't mangle this yet" diagnostics, so accuracy doesn't seem like a massive priority. We need this to work for the case of functions with no corresponding FunctionDecl anyway, and that case would be improved by passing mangleFunctionType a SourceRange instead of a FunctionDecl.
Incidentally, the MS mangler's use of SourceRange seems like overkill -- all it uses is the begin location from the range, so it may as well use a SourceLocation.
================
Comment at: lib/AST/MicrosoftMangle.cpp:263-264
@@ -262,2 +262,4 @@
// We don't even know how to mangle their types anyway :).
- const FunctionProtoType *FT = FD->getType()->castAs<FunctionProtoType>();
+ TypeSourceInfo *TSI = FD->getTypeSourceInfo();
+ QualType T = TSI ? TSI->getType() : FD->getType();
+ const FunctionProtoType *FT = T->castAs<FunctionProtoType>();
----------------
Reid Kleckner wrote:
> Peter Collingbourne wrote:
> > Richard Smith wrote:
> > > Which declaration is 'FD' here? If I have this in a header:
> > >
> > > void f(int p[123]);
> > >
> > > ... and this in my source file:
> > >
> > > #include "header"
> > > void f(int *const p) {}
> > >
> > > ... then do we use the mangling for the header's declaration or for the source file's declaration? I would guess (or rather, hope) that we use the mangling for the first declaration of the function.
> > ITYM
> >
> > void f(int *p) {}
> >
> > i.e. a redeclaration of f with a different mangling (the const qualified version would have the same mangling).
> Congratulations, you just identified a mangling bug that results in real link errors.
>
> If the forward decl for f is present, the parameter is mangled as const. If the forward is missing, it is non-const, resulting in link errors. Awesome.
>
> I'm adding FD = FD->getFirstDeclaration(), which I hope is the right fix.
SGTM
http://llvm-reviews.chandlerc.com/D844
More information about the cfe-commits
mailing list