I think we need to relax the test cases. MSVC usually prints the calling convention, and it's often useful information. Maybe we can make the diagnostic text smaller by using the __thiscall, __cdecl, __stdcall, etc keywords when printing types with LangOpts.MicrosoftExt, but it will require moving the attribute from after the identifier to before, which is exciting. On Tue, Dec 10, 2013 at 4:20 PM, Hans Wennborg <hans at chromium.org> wrote:> On Tue, Dec 10, 2013 at 3:57 PM, Reid Kleckner <rnk at google.com> wrote: > > On Tue, Dec 10, 2013 at 11:58 AM, Hans Wennborg <hans at chromium.org> > wrote: > >> > >> It would be nice if we could make the TypePrinter not print the > >> calling convention if it's the default one for the ABI, but > >> TypePrinter doesn't have a lot of context.. no Sema, no ASTContext :/ > > > > > > Does this patch fix any failures for you? It doesn't fix that test case, > > unfortunately. > > Unfortunately, no, I still see the same failures. These are the > TypePrinter related failures I see: > > Clang :: CXX/class.access/p6.cpp > Clang :: CXX/expr/expr.const/p3-0x.cpp > Clang :: CXX/expr/expr.mptr.oper/p5.cpp > Clang :: CXX/expr/expr.mptr.oper/p6-0x.cpp > Clang :: CXX/expr/expr.unary/expr.unary.op/p4.cpp > Clang :: CXX/temp/temp.arg/temp.arg.nontype/p5.cpp > Clang :: SemaCXX/addr-of-overloaded-function.cpp > Clang :: SemaCXX/const-cast.cpp > Clang :: SemaCXX/cstyle-cast.cpp > Clang :: SemaCXX/functional-cast.cpp > Clang :: SemaCXX/reinterpret-cast.cpp > Clang :: SemaCXX/static-cast.cpp > Printing thiscall attribute on member function pointer type. > > Clang :: CXX/dcl.dcl/dcl.spec/dcl.constexpr/p1.cpp > Clang :: SemaTemplate/explicit-instantiation.cpp > Clang :: SemaTemplate/instantiate-method.cpp > Clang :: SemaTemplate/temp_arg_nontype.cpp > Printing thiscall attribute for function type after explicit > specialization function match failure. > > - Hans >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20131210/950d1b04/attachment.html>
On Tue, Dec 10, 2013 at 4:39 PM, Reid Kleckner <rnk at google.com> wrote:> I think we need to relax the test cases. MSVC usually prints the calling > convention, and it's often useful information.I was going to argue that MSVC doesn't print them, but then I tried, and you're right - it does :) I'll start relaxing the tests then.> Maybe we can make the diagnostic text smaller by using the __thiscall, > __cdecl, __stdcall, etc keywords when printing types with > LangOpts.MicrosoftExt, but it will require moving the attribute from after > the identifier to before, which is exciting.This seems like a nice thing to do, but I'll get the tests going with what we've got for now. - Hans> On Tue, Dec 10, 2013 at 4:20 PM, Hans Wennborg <hans at chromium.org> wrote: >> >> On Tue, Dec 10, 2013 at 3:57 PM, Reid Kleckner <rnk at google.com> wrote: >> > On Tue, Dec 10, 2013 at 11:58 AM, Hans Wennborg <hans at chromium.org> >> > wrote: >> >> >> >> It would be nice if we could make the TypePrinter not print the >> >> calling convention if it's the default one for the ABI, but >> >> TypePrinter doesn't have a lot of context.. no Sema, no ASTContext :/ >> > >> > >> > Does this patch fix any failures for you? It doesn't fix that test >> > case, >> > unfortunately. >> >> Unfortunately, no, I still see the same failures. These are the >> TypePrinter related failures I see: >> >> Clang :: CXX/class.access/p6.cpp >> Clang :: CXX/expr/expr.const/p3-0x.cpp >> Clang :: CXX/expr/expr.mptr.oper/p5.cpp >> Clang :: CXX/expr/expr.mptr.oper/p6-0x.cpp >> Clang :: CXX/expr/expr.unary/expr.unary.op/p4.cpp >> Clang :: CXX/temp/temp.arg/temp.arg.nontype/p5.cpp >> Clang :: SemaCXX/addr-of-overloaded-function.cpp >> Clang :: SemaCXX/const-cast.cpp >> Clang :: SemaCXX/cstyle-cast.cpp >> Clang :: SemaCXX/functional-cast.cpp >> Clang :: SemaCXX/reinterpret-cast.cpp >> Clang :: SemaCXX/static-cast.cpp >> Printing thiscall attribute on member function pointer type. >> >> Clang :: CXX/dcl.dcl/dcl.spec/dcl.constexpr/p1.cpp >> Clang :: SemaTemplate/explicit-instantiation.cpp >> Clang :: SemaTemplate/instantiate-method.cpp >> Clang :: SemaTemplate/temp_arg_nontype.cpp >> Printing thiscall attribute for function type after explicit >> specialization function match failure. >> >> - Hans > >
On Tue, Dec 10, 2013 at 5:08 PM, Hans Wennborg <hans at chromium.org> wrote:> On Tue, Dec 10, 2013 at 4:39 PM, Reid Kleckner <rnk at google.com> wrote: >> I think we need to relax the test cases. MSVC usually prints the calling >> convention, and it's often useful information. > > I was going to argue that MSVC doesn't print them, but then I tried, > and you're right - it does :) > > I'll start relaxing the tests then.Attached is a patch that fixes all the test failures I saw due to printing out the thiscall attribute on types. Do we want to land this now, or wait until the thiscall switch is actually flipped for MinGW? - Hans>> On Tue, Dec 10, 2013 at 4:20 PM, Hans Wennborg <hans at chromium.org> wrote: >>> >>> On Tue, Dec 10, 2013 at 3:57 PM, Reid Kleckner <rnk at google.com> wrote: >>> > On Tue, Dec 10, 2013 at 11:58 AM, Hans Wennborg <hans at chromium.org> >>> > wrote: >>> >> >>> >> It would be nice if we could make the TypePrinter not print the >>> >> calling convention if it's the default one for the ABI, but >>> >> TypePrinter doesn't have a lot of context.. no Sema, no ASTContext :/ >>> > >>> > >>> > Does this patch fix any failures for you? It doesn't fix that test >>> > case, >>> > unfortunately. >>> >>> Unfortunately, no, I still see the same failures. These are the >>> TypePrinter related failures I see: >>> >>> Clang :: CXX/class.access/p6.cpp >>> Clang :: CXX/expr/expr.const/p3-0x.cpp >>> Clang :: CXX/expr/expr.mptr.oper/p5.cpp >>> Clang :: CXX/expr/expr.mptr.oper/p6-0x.cpp >>> Clang :: CXX/expr/expr.unary/expr.unary.op/p4.cpp >>> Clang :: CXX/temp/temp.arg/temp.arg.nontype/p5.cpp >>> Clang :: SemaCXX/addr-of-overloaded-function.cpp >>> Clang :: SemaCXX/const-cast.cpp >>> Clang :: SemaCXX/cstyle-cast.cpp >>> Clang :: SemaCXX/functional-cast.cpp >>> Clang :: SemaCXX/reinterpret-cast.cpp >>> Clang :: SemaCXX/static-cast.cpp >>> Printing thiscall attribute on member function pointer type. >>> >>> Clang :: CXX/dcl.dcl/dcl.spec/dcl.constexpr/p1.cpp >>> Clang :: SemaTemplate/explicit-instantiation.cpp >>> Clang :: SemaTemplate/instantiate-method.cpp >>> Clang :: SemaTemplate/temp_arg_nontype.cpp >>> Printing thiscall attribute for function type after explicit >>> specialization function match failure. >>> >>> - Hans >> >>-------------- next part -------------- A non-text attachment was scrubbed... Name: relax-tests-for-thiscall-in-types.patch Type: application/octet-stream Size: 24194 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20131211/e65448f6/attachment.obj>