<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:x="urn:schemas-microsoft-com:office:excel" xmlns:p="urn:schemas-microsoft-com:office:powerpoint" xmlns:a="urn:schemas-microsoft-com:office:access" xmlns:dt="uuid:C2F41010-65B3-11d1-A29F-00AA00C14882" xmlns:s="uuid:BDC6E3F0-6DA3-11d1-A2A3-00AA00C14882" xmlns:rs="urn:schemas-microsoft-com:rowset" xmlns:z="#RowsetSchema" xmlns:b="urn:schemas-microsoft-com:office:publisher" xmlns:ss="urn:schemas-microsoft-com:office:spreadsheet" xmlns:c="urn:schemas-microsoft-com:office:component:spreadsheet" xmlns:odc="urn:schemas-microsoft-com:office:odc" xmlns:oa="urn:schemas-microsoft-com:office:activation" xmlns:html="http://www.w3.org/TR/REC-html40" xmlns:q="http://schemas.xmlsoap.org/soap/envelope/" xmlns:rtc="http://microsoft.com/officenet/conferencing" xmlns:D="DAV:" xmlns:Repl="http://schemas.microsoft.com/repl/" xmlns:mt="http://schemas.microsoft.com/sharepoint/soap/meetings/" xmlns:x2="http://schemas.microsoft.com/office/excel/2003/xml" xmlns:ppda="http://www.passport.com/NameSpace.xsd" xmlns:ois="http://schemas.microsoft.com/sharepoint/soap/ois/" xmlns:dir="http://schemas.microsoft.com/sharepoint/soap/directory/" xmlns:ds="http://www.w3.org/2000/09/xmldsig#" xmlns:dsp="http://schemas.microsoft.com/sharepoint/dsp" xmlns:udc="http://schemas.microsoft.com/data/udc" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:sub="http://schemas.microsoft.com/sharepoint/soap/2002/1/alerts/" xmlns:ec="http://www.w3.org/2001/04/xmlenc#" xmlns:sp="http://schemas.microsoft.com/sharepoint/" xmlns:sps="http://schemas.microsoft.com/sharepoint/soap/" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:udcs="http://schemas.microsoft.com/data/udc/soap" xmlns:udcxf="http://schemas.microsoft.com/data/udc/xmlfile" xmlns:udcp2p="http://schemas.microsoft.com/data/udc/parttopart" xmlns:wf="http://schemas.microsoft.com/sharepoint/soap/workflow/" xmlns:dsss="http://schemas.microsoft.com/office/2006/digsig-setup" xmlns:dssi="http://schemas.microsoft.com/office/2006/digsig" xmlns:mdssi="http://schemas.openxmlformats.org/package/2006/digital-signature" xmlns:mver="http://schemas.openxmlformats.org/markup-compatibility/2006" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns:mrels="http://schemas.openxmlformats.org/package/2006/relationships" xmlns:spwp="http://microsoft.com/sharepoint/webpartpages" xmlns:ex12t="http://schemas.microsoft.com/exchange/services/2006/types" xmlns:ex12m="http://schemas.microsoft.com/exchange/services/2006/messages" xmlns:pptsl="http://schemas.microsoft.com/sharepoint/soap/SlideLibrary/" xmlns:spsl="http://microsoft.com/webservices/SharePointPortalServer/PublishedLinksService" xmlns:Z="urn:schemas-microsoft-com:" xmlns:st="" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv=Content-Type content="text/html; charset=utf-8">
<meta name=Generator content="Microsoft Word 12 (filtered medium)">
<style>
<!--
/* Font Definitions */
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Tahoma;
panose-1:2 11 6 4 3 5 4 4 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0cm;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman","serif";}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
span.apple-tab-span
{mso-style-name:apple-tab-span;}
span.EmailStyle18
{mso-style-type:personal-reply;
font-family:"Calibri","sans-serif";
color:#1F497D;}
.MsoChpDefault
{mso-style-type:export-only;}
@page Section1
{size:612.0pt 792.0pt;
margin:72.0pt 72.0pt 72.0pt 72.0pt;}
div.Section1
{page:Section1;}
-->
</style>
<!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang=EN-GB link=blue vlink=purple>
<div class=Section1>
<p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'>Hi Chandler,<o:p></o:p></span></p>
<p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'><o:p> </o:p></span></p>
<p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'>Thanks for the thorough review. My comments inline.<o:p></o:p></span></p>
<p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'><o:p> </o:p></span></p>
<p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'>Cheers,<o:p></o:p></span></p>
<p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'><o:p> </o:p></span></p>
<p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'>James<o:p></o:p></span></p>
<p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'><o:p> </o:p></span></p>
<div style='border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0cm 0cm 0cm'>
<p class=MsoNormal><b><span lang=EN-US style='font-size:10.0pt;font-family:
"Tahoma","sans-serif"'>From:</span></b><span lang=EN-US style='font-size:10.0pt;
font-family:"Tahoma","sans-serif"'> Chandler Carruth
[mailto:chandlerc@google.com] <br>
<b>Sent:</b> 26 January 2012 10:52<br>
<b>To:</b> James Molloy<br>
<b>Cc:</b> cfe-commits@cs.uiuc.edu; juli@clockworksquid.com;
ken.dyck@onsemi.com<br>
<b>Subject:</b> Re: [cfe-commits] [PATCH] Fix tag decls/enum constants in
function prototypes<o:p></o:p></span></p>
</div>
<p class=MsoNormal><o:p> </o:p></p>
<div>
<p class=MsoNormal>A couple of high-level comments:<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal><o:p> </o:p></p>
</div>
<div>
<p class=MsoNormal>- Clang doesn't use date-based test cases, and I don't want
to start. The date is completely irrelevant to the test case, they should be
named based on the semantics they're checking.<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal><span style='color:#1F497D'><o:p> </o:p></span></p>
<p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'>No problem. This is a convention in LLVM’s test dirs and I hadn’t
clocked that Clang didn’t have the same convention. WILLFIX.<o:p></o:p></span></p>
<p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'><o:p> </o:p></span></p>
</div>
<div>
<p class=MsoNormal>- You currently check both code generation and semantic
analysis in the same test, they need to be split into two tests, one for Sema
and one for CodeGen. You might be able to eliminate the CodeGen test completely
if you can find a way to observe the value of the enumerator entirely within
the frontend. This is easy with C++ by using a static assert, not sure off hand
about C.<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal><span style='color:#1F497D'><o:p> </o:p></span></p>
<p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'>Sure – I will go about doing this.<o:p></o:p></span></p>
<p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'><o:p> </o:p></span></p>
</div>
<div>
<p class=MsoNormal>- You shouldn't ever add a warning to the warning-flags.c
test. The goal of that test is to ensure we never add a new warning without a
warning flag associated with it. You should provide a flag to control the new
warning you're introducing.<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal><span style='color:#1F497D'><o:p> </o:p></span></p>
<p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'>Separation of concerns is good. Yes, I’m adding a new warning
that has no flag, but it’s essentially a clone of another warning with one verb
different that currently has no flag. Do you suggest I add the flag to cover
both the old and new warnings in the same patch? I can do it as a followup and
I’d prefer that if possible.<o:p></o:p></span></p>
<p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'><o:p> </o:p></span></p>
</div>
<div>
<p class=MsoNormal>- You need to add visiting logic for all of these decls to
the various visitors I suspect. We have several, I'd have to go looking to
enumerate all of them. Off the top of my head I would check:<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal> - RecursiveASTVisitor descends into these decls.<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal> - The various printing and dumping visitors catch
them<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal> - The CFG builder handles them (not sure how much it
really has to care)<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal> - Maybe update libclang? Not likely important for the
first pass.<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal><span style='color:#1F497D'><o:p> </o:p></span></p>
<p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'>Thanks for catching this – I didn’t think it would be required.
I’ll ensure everything acts as it should, although I’m not sure how much effect
this will have. The decls already existed previously, just were in the wrong
scope.<o:p></o:p></span></p>
</div>
<div>
<p class=MsoNormal><o:p> </o:p></p>
</div>
<div>
<p class=MsoNormal>Specific comments on the patch:<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal><o:p> </o:p></p>
</div>
<div>
<div>
<p class=MsoNormal>Index: include/clang/Sema/Sema.h<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>===================================================================<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>--- include/clang/Sema/Sema.h (revision 148299)<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+++ include/clang/Sema/Sema.h (working copy)<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>@@ -886,6 +886,16 @@<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal> // Symbol table / Decl tracking callbacks:
SemaDecl.cpp.<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal> //<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal> <o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+private:<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+ /// List of decls defined in a function prototype.
This contains EnumConstants<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+ /// that incorrectly end up in translation unit
scope because there is no<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+ /// function to pin them on. ActOnFunctionDeclarator
reads this list and patches<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+ /// them into the FunctionDecl.<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+ std::vector<NamedDecl*> DeclsInPrototypeScope;<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+ bool InFunctionDeclarator;<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+public:<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal><o:p> </o:p></p>
</div>
<div>
<p class=MsoNormal>Why make these private? That's not common in Sema because of
the large number of file-local static helper functions which need to reach into
the innards of Sema. These in particular seem like reasonable things to query
from anywhere that has the Sema object.<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal><span style='color:#1F497D'><o:p> </o:p></span></p>
<p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'>I’m sure I had a reason but it evades me.<o:p></o:p></span></p>
<p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'><o:p> </o:p></span></p>
</div>
<div>
<p class=MsoNormal>Secondly, why place these here? Sema is a just a grab bag of
crap, but you could probably profitably stash these nearer to the function that
is most likely to use them, or with some other Sema state-management fields.<o:p></o:p></p>
<p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'><o:p> </o:p></span></p>
<p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'>The reason is simple – that place is the start of the functions
that are used in SemaDecl.cpp (see the comment in the first line of context in
the diff). I placed the variables that are used in SemaDecl.cpp right above the
functions that use them.<o:p></o:p></span></p>
</div>
<div>
<p class=MsoNormal><o:p> </o:p></p>
</div>
<div>
<div>
<p class=MsoNormal>Index: include/clang/AST/Decl.h<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>===================================================================<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>--- include/clang/AST/Decl.h<span class=apple-tab-span> </span>(revision
148299)<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+++ include/clang/AST/Decl.h<span class=apple-tab-span> </span>(working
copy)<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>@@ -1388,6 +1388,13 @@<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal> /// no formals.<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal> ParmVarDecl **ParamInfo;<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal> <o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+ /// DeclsInPrototypeScope - new[]'d array of
pointers to NamedDecls for<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+ /// decls defined in the function prototype that are
not parameters. E.g.<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+ /// 'AA' in 'void f(enum {AA} x) {}'. This is null
if a prototype or if there<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+ /// are no such decls.<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+ NamedDecl **DeclsInPrototypeScope;<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+ unsigned NumDeclsInPrototypeScope;<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+<o:p></o:p></p>
</div>
</div>
<div>
<p class=MsoNormal><o:p> </o:p></p>
</div>
<div>
<p class=MsoNormal>I assume that this is a raw new[]'d array to allow us to
allocate it using the ASTContext BumpPtrAllocator? we really need to thread
allocators into the LLVM ADTs. :: sigh ::<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal><o:p> </o:p></p>
</div>
<div>
<p class=MsoNormal>If this is the case, I would suggest making the member a
ArrayRef<NamedDecl *> or MutableArrayRef<NamedDecl *>.<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal><span style='color:#1F497D'><o:p> </o:p></span></p>
<p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'>It’s raw new’d for the reason you state. I didn’t use arrayref
because the existing code for ParmVarDecl (see diff context) also doesn’t use
ArrayRef. I copied its interface completely, assuming (for better or worse)
that it was designed that way for a reason.<o:p></o:p></span></p>
<p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'><o:p> </o:p></span></p>
</div>
<div>
<div>
<p class=MsoNormal>@@ -1753,6 +1761,20 @@<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal> setParams(getASTContext(),
NewParamInfo);<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal> }<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal> <o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+ unsigned getNumDeclsInPrototypeScope() const {
return NumDeclsInPrototypeScope; }<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+ const NamedDecl *getDeclInPrototypeScope(unsigned i)
const {<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+ assert(i < NumDeclsInPrototypeScope);<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+ return DeclsInPrototypeScope[i];<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+ }<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+ NamedDecl *getDeclInPrototypeScope(unsigned i) {<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+ assert(i < NumDeclsInPrototypeScope);<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+ return DeclsInPrototypeScope[i];<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+ }<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal><o:p> </o:p></p>
</div>
<div>
<p class=MsoNormal>Why not just provide access via an ArrayRef? That would seem
a cleaner and simpler interface.<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal><span style='color:#1F497D'><o:p> </o:p></span></p>
<p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'>See above, copying existing coding style. While ArrayRef is
better, I didn’t want to mix coding styles in one class unless required. I can
go through and change if you want?<o:p></o:p></span></p>
<p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'><o:p> </o:p></span></p>
</div>
<div>
<p class=MsoNormal>+ void
setDeclsInPrototypeScope(llvm::ArrayRef<NamedDecl *> NewDecls) {<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+ setDeclsInPrototypeScope(getASTContext(),
NewDecls);<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+ }<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+<o:p></o:p></p>
</div>
</div>
<div>
<p class=MsoNormal><o:p> </o:p></p>
</div>
<div>
<p class=MsoNormal>Do we really need this wrapper?<o:p></o:p></p>
<p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'><o:p> </o:p></span></p>
<p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'>Again, above.<o:p></o:p></span></p>
</div>
<div>
<p class=MsoNormal><o:p> </o:p></p>
</div>
<div>
<div>
<p class=MsoNormal>Index: lib/Sema/SemaDecl.cpp<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>===================================================================<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>--- lib/Sema/SemaDecl.cpp<span class=apple-tab-span> </span>(revision
148299)<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+++ lib/Sema/SemaDecl.cpp<span class=apple-tab-span> </span>(working
copy)<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>@@ -1218,6 +1218,11 @@<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal> }<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal> }<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal> <o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+void Sema::ActOnStartFunctionDeclarator() {<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+ DeclsInPrototypeScope.clear();<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal><o:p> </o:p></p>
</div>
<div>
<p class=MsoNormal>I would clear this once we move them to the new function
decl below.<o:p></o:p></p>
</div>
</div>
<div>
<p class=MsoNormal><span style='color:#1F497D'><o:p> </o:p></span></p>
<p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'>Sure.<o:p></o:p></span></p>
<p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'><o:p> </o:p></span></p>
</div>
<div>
<div>
<p class=MsoNormal>@@ -7075,6 +7090,14 @@<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal> }<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal> }<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal> <o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+ // If we had any enum constants defined in the
function prototype,<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+ // introduce them to the function scope.<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+ for (unsigned i = 0, N =
FD->getNumDeclsInPrototypeScope(); i < N; ++i) {<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+ NamedDecl *D =
FD->getDeclInPrototypeScope(i);<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal><o:p> </o:p></p>
</div>
<div>
<p class=MsoNormal>The convention for for loops is more 'i' and 'e', but if you
make this an array ref it becomes super easy just to get begin and end pointers
and treat it like an iterator.<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal><o:p> </o:p></p>
</div>
<div>
<p class=MsoNormal>+ if (FnBodyScope)<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal><o:p> </o:p></p>
</div>
<div>
<p class=MsoNormal>Isn't this loop-invariant? Seems like it should guard the
whole loop.<o:p></o:p></p>
<p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'><o:p> </o:p></span></p>
<p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'>Quite possibly. WILLFIX<o:p></o:p></span></p>
</div>
<div>
<p class=MsoNormal><o:p> </o:p></p>
</div>
</div>
<div>
<div>
<p class=MsoNormal>@@ -8028,7 +8051,21 @@<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>
!isa<CXXRecordDecl>(Def) ||<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>
cast<CXXRecordDecl>(Def)->getTemplateSpecializationKind() <o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>
== TSK_ExplicitSpecialization) {<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>-
Diag(NameLoc, diag::err_redefinition) << Name;<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+ //
A redeclaration in function prototype scope in C isn't<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+ //
visible elsewhere, so merely issue a warning.<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+
bool FoundPrototypeScope = false;<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+
Scope *S2 = S;<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+
while (S2) {<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+
if (S2->isFunctionPrototypeScope()) {<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+
FoundPrototypeScope = true;<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+
break;<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+
}<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+
S2 = S2->getParent();<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+ }<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal><o:p> </o:p></p>
</div>
<div>
<p class=MsoNormal>This loop should be hoisted into a helper function, likely
on Scope.<o:p></o:p></p>
<p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'><o:p> </o:p></span></p>
<p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'>Will do.<o:p></o:p></span></p>
</div>
<div>
<p class=MsoNormal><o:p> </o:p></p>
</div>
<div>
<p class=MsoNormal>+ if
(FoundPrototypeScope && !getLangOptions().CPlusPlus)<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal><o:p> </o:p></p>
</div>
<div>
<p class=MsoNormal>And then make sure you check CPlusPlus first so we don't do
the recursive walk up the scopes when we don't need to.<o:p></o:p></p>
<p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'><o:p> </o:p></span></p>
<p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'>Will do.<o:p></o:p></span></p>
</div>
<div>
<p class=MsoNormal><o:p> </o:p></p>
</div>
<div>
<p class=MsoNormal>+
Diag(NameLoc, diag::warn_redefinition_in_param_list) << Name;<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+
else<o:p></o:p></p>
</div>
<div>
<p class=MsoNormal>+
Diag(NameLoc, diag::err_redefinition) << Name;<o:p></o:p></p>
</div>
</div>
<div>
<p class=MsoNormal><o:p> </o:p></p>
</div>
</div>
</div>
</body>
</html>