<div class="gmail_quote">
<blockquote style="BORDER-LEFT: #ccc 1px solid; MARGIN: 0px 0px 0px 0.8ex; PADDING-LEFT: 1ex" class="gmail_quote"><br>Generally, I suggest that you post patches to the list, because there are others who might also provide reviews. Some comments follow.</blockquote>
<div> </div>
<div>Sorry Doug, I copied both you and the list, which is redundant, so I'll just use the list from now on.</div>
<div> </div>
<blockquote style="BORDER-LEFT: #ccc 1px solid; MARGIN: 0px 0px 0px 0.8ex; PADDING-LEFT: 1ex" class="gmail_quote"><br>Index: tools/clang/include/clang/Basic/Builtins.h<br><br>===================================================================<br>
--- tools/clang/include/clang/Basic/Builtins.h (revision 73735)<br>+++ tools/clang/include/clang/Basic/Builtins.h (working copy)<br>@@ -31,6 +31,7 @@<br><br> enum ID {<br> NotBuiltin = 0, // This is not a builtin function.<br>
#define BUILTIN(ID, TYPE, ATTRS) BI##ID,<br>+#undef alloca<br> #include "clang/Basic/Builtins.def"<br> FirstTSBuiltin<br> };<br><br>I thought this build issue was fixed. Do you still need this?</blockquote>
<div> </div>
<div>Actually, I didn't see my prior change in the update, but for some reason, with my original change, I still was getting the error, even after a full rebuild. Perhaps it was a weirdness with Visual Studio. But moving the undefine up into the header fixed it, again. Do you think I should redefine it (#define abort _abort) for Windows after the Buildtins.def include?</div>
<div> </div>
<blockquote style="BORDER-LEFT: #ccc 1px solid; MARGIN: 0px 0px 0px 0.8ex; PADDING-LEFT: 1ex" class="gmail_quote"><span></span><br><br>Index: tools/clang/lib/AST/DeclBase.cpp<br>===================================================================<br>
--- tools/clang/lib/AST/DeclBase.cpp (revision 73735)<br>+++ tools/clang/lib/AST/DeclBase.cpp (working copy)<br>@@ -164,6 +164,7 @@<br><br> case ParmVar:<br> case OriginalParmVar:<br> case NonTypeTemplateParm:<br>
+ case Using:<br> case ObjCMethod:<br> case ObjCContainer:<br> case ObjCCategory:<br><br>This puts all using declarations into the ordinary namespace. However, what happens if the using declaration refers to something not in the ordinary namespace, e.g.,<br>
<br> namespace N {<br> struct X;<br> }<br><br> using N::X;<br> void X(int); // okay<br><br> struct X *x; // okay<br><br>I guess my expectation was that UsingDecl would bitwise-OR the identifier namespace values of all of the things that it points to.</blockquote>
<div> </div>
<div>Sorry, I blindly put that in not understanding it while searching for places where decl types were used. I'm afraid I still don't understand it, so I'll have to study it some more as part of working on the semantics, unless you have an easy fix. I've left what I had in for now, to keep the test file working.</div>
<div> </div>
<blockquote style="BORDER-LEFT: #ccc 1px solid; MARGIN: 0px 0px 0px 0.8ex; PADDING-LEFT: 1ex" class="gmail_quote"><br>+ // Parse namespace-name.<br>+ if (SS.isInvalid() || Tok.isNot(tok::identifier)) {<br>+ Diag(Tok, diag::err_expected_namespace_or_type_name);<br>
+ // If there was invalid namespace name, skip to end of decl, and eat ';'.<br>+ SkipUntil(tok::semi);<br>+ return DeclPtrTy();<br>+ }<br><br>"expected namespace or type name" seems like the wrong diagnostic. I think we should separate the two cases:<br>
- If the scope specifier is invalid, we've already emitted an error so we can just return.<br><br> - If the token following the scope specifier is not an identifier, we can say something like "expected an identifier in using directive". It would also be really great if we detected the case of a template-id (e.g., "using std::vector<int>;") specifically (C++ [namespace.udecl]p5), because it's such an easy error to make. This is a case where we can provide a very specific diagnostic.<br>
</blockquote>
<div>I've taken a stab at fixing this in the new patch.</div>
<blockquote style="BORDER-LEFT: #ccc 1px solid; MARGIN: 0px 0px 0px 0.8ex; PADDING-LEFT: 1ex" class="gmail_quote"><span></span><br>+ // Lookup target name.<br>+ LookupResult R = LookupParsedName(S, &SS, TargetName,<br>
+ LookupOrdinaryName, false);<br>+<br>+ if (NamedDecl *NS = R) {<br>+ if (IsTypeName && !isa<TypeDecl>(NS)) {<br>+ Diag(IdentLoc, diag::err_using_typename_non_type);<br>
+ return DeclPtrTy();<br>+ }<br><br>Good, good. Once we've emitted the error, I don't think we have to return, because we can recover from this particular error easily.</blockquote>
<div>So just delete the return statement? I've added that to the new patch.</div>
<blockquote style="BORDER-LEFT: #ccc 1px solid; MARGIN: 0px 0px 0px 0.8ex; PADDING-LEFT: 1ex" class="gmail_quote"><span></span><br>Index: tools/clang/lib/Sema/SemaLookup.cpp<br>===================================================================<br>
--- tools/clang/lib/Sema/SemaLookup.cpp (revision 73735)<br>+++ tools/clang/lib/Sema/SemaLookup.cpp (working copy)<br>@@ -1151,8 +1151,13 @@<br><br> Name, NameKind, RedeclarationOnly);<br> }<br>
<br>- return LookupName(S, Name, NameKind, RedeclarationOnly,<br>- AllowBuiltinCreation, Loc);<br>+ LookupResult result(LookupName(S, Name, NameKind, RedeclarationOnly,<br>+ AllowBuiltinCreation, Loc));<br>
+ UsingDecl* usingDecl;<br>+ if (!RedeclarationOnly && (bool)result &&<br>+ ((usingDecl = dyn_cast<UsingDecl>(result.getAsDecl())) != NULL))<br>+ return(LookupResult::CreateLookupResult(Context, usingDecl->getTargetDecl()));<br>
+ return(result);<br> }<br><br>This is only one of many places where we would have to perform the UsingDecl -> target declaration mapping. I think it would be much easier to move this actual mapping into CreateLookupResult (or some helper that we use instead of CreateLookupResult). That way, there's only one point where we have to cope with using declarations.<br>
</blockquote>
<div>Okay. I'll work on this after this patch.</div>
<blockquote style="BORDER-LEFT: #ccc 1px solid; MARGIN: 0px 0px 0px 0.8ex; PADDING-LEFT: 1ex" class="gmail_quote"><span></span><br>We're definitely getting closer. My comments about the parsing side of things were pretty trivial; if you want to make those little changes first, I can commit that part of your patch to the tree so you can focus on semantics (which are significantly tougher).</blockquote>
<blockquote style="BORDER-LEFT: #ccc 1px solid; MARGIN: 0px 0px 0px 0.8ex; PADDING-LEFT: 1ex" class="gmail_quote"><span></span><br> - Doug</blockquote>
<div> </div>
<div>Thanks, Doug. I've enclosed a new patch, usingdecl3.patch. I've also added your examples to the test file, the last part of which is for later work. I left in a couple of the old changes to SemaDecl to keep the test file working, but you can ignore them if you want. Otherwise I'll take them out when I have the substitution working.</div>
<div> </div>
<div>I hope my working on Clang is working out for you guys, and not taking more hand-holding than it is worth. I fear I might be in a bit over my head, but I'd definitely like to and help out, if I can.</div></div>
<div></div><br>-- <br>John Thompson<br><a href="mailto:John.Thompson.JTSoftware@gmail.com">John.Thompson.JTSoftware@gmail.com</a><br><br>