[PATCH] D18953: [ms][dll] #26935 Defining a dllimport function should cause it to be exported

2017-10-17 Thread Nico Weber via Phabricator via cfe-commits
thakis added inline comments. Comment at: cfe/trunk/lib/Sema/SemaDecl.cpp:5650 + NewImportAttr->getRange(), S.Context, + NewImportAttr->getSpellingListIndex())); +} else { NewImportAttr can be nullptr here, at least for invalid code. This ch

Re: [PATCH] D18953: [ms][dll] #26935 Defining a dllimport function should cause it to be exported

2016-05-25 Thread Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL270686: [ms][dll] #26935 Defining a dllimport function should cause it to be exported (authored by dzobnin). Changed prior to commit: http://reviews.llvm.org/D18953?vs=58233&id=58401#toc Repository:

Re: [PATCH] D18953: [ms][dll] #26935 Defining a dllimport function should cause it to be exported

2016-05-24 Thread Reid Kleckner via cfe-commits
rnk added a comment. lgtm http://reviews.llvm.org/D18953 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18953: [ms][dll] #26935 Defining a dllimport function should cause it to be exported

2016-05-24 Thread Andrew V. Tischenko via cfe-commits
avt77 updated this revision to Diff 58233. avt77 added a comment. I built the project from scratch and found an issue in Clang build system. There was a problem with undefined DiagGroup in DiagnosticSemaKinds.td: the error message was generated but all executables were created that's why I was

Re: [PATCH] D18953: [ms][dll] #26935 Defining a dllimport function should cause it to be exported

2016-05-23 Thread Reid Kleckner via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm http://reviews.llvm.org/D18953 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18953: [ms][dll] #26935 Defining a dllimport function should cause it to be exported

2016-05-23 Thread Andrew V. Tischenko via cfe-commits
avt77 added a comment. OK, as I see all issues were resolved, right? Could I commit the patch? http://reviews.llvm.org/D18953 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18953: [ms][dll] #26935 Defining a dllimport function should cause it to be exported

2016-05-13 Thread Reid Kleckner via cfe-commits
rnk added inline comments. Comment at: test/SemaCXX/dllimport.cpp:179 @@ -140,1 +178,3 @@ +template +int ExternVarTmplDeclInit = 1; avt77 wrote: > majnemer wrote: > > avt77 wrote: > > > rnk wrote: > > > > Can you check with MSVC 2015 update 2 actually does with

Re: [PATCH] D18953: [ms][dll] #26935 Defining a dllimport function should cause it to be exported

2016-04-28 Thread Andrew V. Tischenko via cfe-commits
avt77 added inline comments. Comment at: test/SemaCXX/dllimport.cpp:179 @@ -140,1 +178,3 @@ +template +int ExternVarTmplDeclInit = 1; majnemer wrote: > avt77 wrote: > > rnk wrote: > > > Can you check with MSVC 2015 update 2 actually does with definitions of >

Re: [PATCH] D18953: [ms][dll] #26935 Defining a dllimport function should cause it to be exported

2016-04-26 Thread David Majnemer via cfe-commits
majnemer added a subscriber: majnemer. Comment at: test/SemaCXX/dllimport.cpp:179 @@ -140,1 +178,3 @@ +template +int ExternVarTmplDeclInit = 1; avt77 wrote: > rnk wrote: > > Can you check with MSVC 2015 update 2 actually does with definitions of > > dllimport

Re: [PATCH] D18953: [ms][dll] #26935 Defining a dllimport function should cause it to be exported

2016-04-26 Thread Andrew V. Tischenko via cfe-commits
avt77 added a comment. It seems the latest MSVC changed the support of several features covering in this patch: more investigations needed. Comment at: test/SemaCXX/dllimport.cpp:179 @@ -140,1 +178,3 @@ +template +int ExternVarTmplDeclInit = 1; rnk wrote: >

Re: [PATCH] D18953: [ms][dll] #26935 Defining a dllimport function should cause it to be exported

2016-04-26 Thread Andrew V. Tischenko via cfe-commits
avt77 added a comment. In fact you introduced the plan of actions to fix the issue. Tnx. http://reviews.llvm.org/D18953 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18953: [ms][dll] #26935 Defining a dllimport function should cause it to be exported

2016-04-20 Thread Richard Smith via cfe-commits
rsmith added a comment. In http://reviews.llvm.org/D18953#397279, @rnk wrote: > Richard, do you think we should be handling this by rewriting the AST-level > attribute in Sema or by changing our interpretation of things in CodeGen? > We're already creating a bunch of implicit attributes to impl

Re: [PATCH] D18953: [ms][dll] #26935 Defining a dllimport function should cause it to be exported

2016-04-20 Thread Reid Kleckner via cfe-commits
rnk added a comment. I think this generally seems right, but we should make sure our behavior is more consistent in the case of a template definition. Comment at: lib/Sema/SemaDecl.cpp:5570-5571 @@ -5565,4 +5569,4 @@ // exceptions being inline function definitions, local ext

Re: [PATCH] D18953: [ms][dll] #26935 Defining a dllimport function should cause it to be exported

2016-04-15 Thread Andrew V. Tischenko via cfe-commits
avt77 changed the visibility of this Differential Revision from "All Users" to "Public (No Login Required)". avt77 updated this revision to Diff 53870. avt77 added a comment. I fixed all issues discovered by Richard and Reid. As result the tests were updated as well. Please, review again. http

Re: [PATCH] D18953: [ms][dll] #26935 Defining a dllimport function should cause it to be exported

2016-04-12 Thread Andrew V. Tischenko via cfe-commits
avt77 marked an inline comment as done. Comment at: lib/Sema/SemaDecl.cpp:5570 @@ -5565,3 +5569,3 @@ // and qualified friend declarations. - // NB: MSVC converts such a declaration to dllexport. + // NB: MSVC converts such a declaration to dllexport that's why we do it also.

Re: [PATCH] D18953: [ms][dll] #26935 Defining a dllimport function should cause it to be exported

2016-04-11 Thread Richard Smith via cfe-commits
rsmith added inline comments. Comment at: lib/Sema/SemaDecl.cpp:5570 @@ -5565,3 +5569,3 @@ // and qualified friend declarations. - // NB: MSVC converts such a declaration to dllexport. + // NB: MSVC converts such a declaration to dllexport that's why we do it also. bool I

Re: [PATCH] D18953: [ms][dll] #26935 Defining a dllimport function should cause it to be exported

2016-04-11 Thread Reid Kleckner via cfe-commits
rnk added a comment. Richard, do you think we should be handling this by rewriting the AST-level attribute in Sema or by changing our interpretation of things in CodeGen? We're already creating a bunch of implicit attributes to implement class-level import/export. Comment at:

[PATCH] D18953: [ms][dll] #26935 Defining a dllimport function should cause it to be exported

2016-04-11 Thread Andrew V. Tischenko via cfe-commits
avt77 created this revision. avt77 added a reviewer: rnk. avt77 added a subscriber: cfe-commits. avt77 changed the visibility of this Differential Revision from "Public (No Login Required)" to "All Users". If we have some function with dllimport attribute and then we have the function definition