<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jul 4, 2013 at 10:25 AM, Nico Rieck <span dir="ltr"><<a href="mailto:nico.rieck@gmail.com" target="_blank" class="cremed">nico.rieck@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">This patch series implements full support for dllexport/dllimport attributes</blockquote></div><br>It's cool that you're working on this, but this patch is too big to review, and can't even be sent to the right people to review. For example, you have a significant change to Clang here, but sent it to the LLVM commits list for review.</div>
<div class="gmail_extra"><br></div><div class="gmail_extra">I would suggest:</div><div class="gmail_extra"><br></div><div class="gmail_extra">1) Take all of the parsing, semantic analysis, and AST changes to Clang to support these attributes and propose that as a single patch. No code generation changes at all. Include most of the tests, etc. Get that reviewed and committed.</div>
<div class="gmail_extra"><br></div><div class="gmail_extra">2) Propose whatever LLVM support you need to code gen #1 correctly as an LLVM-only patch. You should have comprehensive testing purely at the LLVM layer.</div><div class="gmail_extra">
<br></div><div class="gmail_extra">3) Propose a patch that wires up the AST representation you added in #1 to the LLVM IR representation in #2 to the Clang commits list.</div><div class="gmail_extra"><br></div><div class="gmail_extra">
<br></div><div class="gmail_extra">This will make all three patches much easier to review, and will get the right people involved at each step.</div><div class="gmail_extra"><br></div><div class="gmail_extra">Thanks,</div>
<div class="gmail_extra">-Chandler</div></div>