Sorry for duplicate email. Hit the wrong button...<br><br><div class="gmail_quote">On Fri, Jun 3, 2011 at 11:36 AM, Sean Hunt <span dir="ltr"><<a href="mailto:scshunt@csclub.uwaterloo.ca">scshunt@csclub.uwaterloo.ca</a>></span> wrote:<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
+ static unsigned getHashValue (const clang::SpecialMemberID SMI) {<br>
+ // Vary higher bits of the pointer for hashing. Attempt to match the<br>
+ // bit-field representation to reduce masking if the optimizer is awake.<br>
+ // Note that the LLVM optimizer sleeps through this one.<br>
+ return (uintptr_t)SMI.D ^<br>
+ ((<a href="http://SMI.SM" target="_blank">SMI.SM</a> << 24) + (SMI.ConstArg << 27) + (SMI.VolatileArg << 28) +<br>
+ (SMI.RValueThis << 29) + (SMI.ConstThis << 30) +<br>
+ (SMI.VolatileThis << 31));<br></blockquote><div><br></div><div>I worry about the person who adds a new bitfield to the type, but fails to update this function. Is there a way to both avoid specifying each of these fields, and their offsets? My current theory would be to memcpy from the offset of the first bitfield, and then shift into high bits based on the offset of a dummy final field.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
+ }<br>
+ static bool isEqual(const clang::SpecialMemberID LHS,<br>
+ const clang::SpecialMemberID RHS) {<br>
+ return LHS.D == RHS.D && <a href="http://LHS.SM" target="_blank">LHS.SM</a> == <a href="http://RHS.SM" target="_blank">RHS.SM</a> && LHS.ConstArg == RHS.ConstArg &&<br>
+ LHS.VolatileArg == RHS.VolatileArg &&<br>
+ LHS.RValueThis == RHS.RValueThis && LHS.ConstThis == RHS.ConstThis &&<br>
+ LHS.VolatileThis == RHS.VolatileThis;<br></blockquote><div><br></div><div>memcmp? Actually, why can't we just use LHS == RHS and let the compiler do the work for us? Again listing these down here seems really unfortunate for a future maintainer.</div>
<div><br></div><div>If we have to manually enumerate the members, can we do it in an operator==() in the struct so that its more clear it has to be updated when the struct is updated?</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
+ }<br>
+};<br>
+} // namespace llvm<br>
+<br>
+namespace clang {<br>
/// Sema - This implements semantic analysis and AST building for C.<br>
class Sema {<br>
Sema(const Sema&); // DO NOT IMPLEMENT<br>
@@ -600,6 +646,47 @@<br>
/// A stack of expression evaluation contexts.<br>
llvm::SmallVector<ExpressionEvaluationContextRecord, 8> ExprEvalContexts;<br>
<br>
+ /// SpecialMemberOverloadResult - The overloading result for a special member<br>
+ /// function.<br>
+ ///<br>
+ /// This is basically a wrapper around PointerIntPair. The lowest bit of the<br>
+ /// integer is used to determine whether we have a parameter qualification<br>
+ /// match, the second-lowest is whether we had success in resolving the<br>
+ /// overload to a unique non-deleted function.<br>
+ ///<br>
+ /// The ConstParamMatch bit represents whether, when looking up a copy<br>
+ /// constructor or assignment operator, we found a potential copy<br>
+ /// constructor/assignment operator whose first parameter is const-qualified.<br></blockquote><div><br></div><div>You've documented one of these bits twice. The first paragraph mentions the lowest and second lowest bit. The second paragraph talks about "The ConstParamMatch bit" which isn't defined, but seems below to correspond to the second lowest bit.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
+ /// This is used for determining parameter types of other objects and is<br>
+ /// utterly meaningless on other types of special members.<br></blockquote><div><br></div><div>This comment doesn't make sense for me....</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
+ class SpecialMemberOverloadResult {<br>
+ llvm::PointerIntPair<CXXMethodDecl*, 2> Pair;<br>
+ public:<br>
+ SpecialMemberOverloadResult(CXXMethodDecl *MD, bool Success,<br>
+ bool ConstParamMatch)<br>
+ : Pair(MD, Success | ConstParamMatch << 1)<br>
+ {}<br>
+ SpecialMemberOverloadResult() {}<br>
+<br>
+ CXXMethodDecl *getMethod() const { return Pair.getPointer(); }<br>
+ void setMethod(CXXMethodDecl *MD) { Pair.setPointer(MD); }<br>
+<br>
+ bool hasSuccess() const { return Pair.getInt() & 0x1; }<br>
+ void setSuccess(bool B) { Pair.setInt(B | hasConstParamMatch() << 1); }<br>
+<br>
+ bool hasConstParamMatch() const { return Pair.getInt() & 0x2; }<br>
+ void setConstParamMatch(bool B) { Pair.setInt(B << 1 | hasSuccess()); }<br>
+ };<br>
+<br>
+ /// \brief A cache of special member function overload resolution results<br>
+ /// for C++ records.<br>
+ ///<br>
+ /// In C++, special member functions of records (such as the copy constructor)<br>
+ /// are used a lot. As a result, we cache the lookups here so as to make the<br>
+ /// lookups far easier to perform.<br></blockquote><div><br></div><div>I'm not sure what this comment is adding past the brief comment...</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
+ llvm::DenseMap<SpecialMemberID, SpecialMemberOverloadResult><br>
+ SpecialMemberCache;<br>
+<br>
/// \brief Whether the code handled by Sema should be considered a<br>
/// complete translation unit or not.<br>
///<br>
@@ -1594,6 +1681,14 @@<br>
private:<br>
bool CppLookupName(LookupResult &R, Scope *S);<br>
<br>
+ SpecialMemberOverloadResult LookupSpecialMember(CXXRecordDecl *D,<br>
+ CXXSpecialMember SM,<br>
+ bool ConstArg,<br>
+ bool VolatileArg,<br>
+ bool RValueThis,<br>
+ bool ConstThis,<br>
+ bool VolatileThis);<br>
+<br>
public:<br>
/// \brief Look up a name, looking for a single declaration. Return<br>
/// null if the results were absent, ambiguous, or overloaded.<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaLookup.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=132572&r1=132571&r2=132572&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=132572&r1=132571&r2=132572&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaLookup.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaLookup.cpp Fri Jun 3 13:36:49 2011<br>
@@ -14,6 +14,7 @@<br>
#include "clang/Sema/Sema.h"<br>
#include "clang/Sema/SemaInternal.h"<br>
#include "clang/Sema/Lookup.h"<br>
+#include "clang/Sema/Overload.h"<br>
#include "clang/Sema/DeclSpec.h"<br>
#include "clang/Sema/Scope.h"<br>
#include "clang/Sema/ScopeInfo.h"<br>
@@ -2136,6 +2137,51 @@<br>
}<br>
}<br>
<br>
+Sema::SpecialMemberOverloadResult Sema::LookupSpecialMember(CXXRecordDecl *D,<br>
+ CXXSpecialMember SM,<br>
+ bool ConstArg,<br>
+ bool VolatileArg,<br>
+ bool RValueThis,<br>
+ bool ConstThis,<br>
+ bool VolatileThis) {<br>
+ D = D->getDefinition();<br>
+ assert((D && !D->isBeingDefined()) &&<br>
+ "doing special member lookup into record that isn't fully complete");<br>
+ if (RValueThis || ConstThis || VolatileThis)<br>
+ assert((SM == CXXCopyAssignment || SM == CXXMoveAssignment) &&<br>
+ "constructors and destructors always have unqualified lvalue this");<br>
+ if (ConstArg || VolatileArg)<br>
+ assert((SM != CXXDefaultConstructor && SM != CXXDestructor) &&<br>
+ "parameter-less special members can't have qualified arguments");<br>
+<br>
+ // Check the cache for this member<br>
+ SpecialMemberID ID = {D, SM, ConstArg, VolatileArg, RValueThis, ConstThis,<br>
+ VolatileThis};<br>
+ SpecialMemberOverloadResult Blank;<br>
+ llvm::DenseMap<SpecialMemberID, SpecialMemberOverloadResult>::iterator It;<br>
+ bool New;<br>
+<br>
+ llvm::tie(It, New) = SpecialMemberCache.insert(std::make_pair(ID, Blank));<br>
+ SpecialMemberOverloadResult &Result = It->second;<br></blockquote><div><br></div><div>This seems like the exact pattern that FoldingSet was designed to address...</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
+<br>
+ // This was already cached<br>
+ if (!New)<br>
+ return Result;<br>
+<br>
+ if (SM == CXXDestructor) {<br>
+ if (!D->hasDeclaredDestructor())<br>
+ DeclareImplicitDestructor(D);<br>
+ CXXDestructorDecl *DD = D->getDestructor();<br>
+ assert(DD && "record without a destructor");<br>
+ Result.setMethod(DD);<br>
+ Result.setSuccess(DD->isDeleted());<br>
+ Result.setConstParamMatch(false);<br>
+ return Result;<br>
+ }<br>
+<br>
+ llvm_unreachable("haven't implemented this for non-destructors yet");<br>
+}<br>
+<br>
/// \brief Look up the constructors for the given class.<br>
DeclContext::lookup_result Sema::LookupConstructors(CXXRecordDecl *Class) {<br>
// If the copy constructor has not yet been declared, do so now.<br>
@@ -2153,17 +2199,13 @@<br>
<br>
/// \brief Look for the destructor of the given class.<br>
///<br>
-/// During semantic analysis, this routine should be used in lieu of<br>
-/// CXXRecordDecl::getDestructor().<br>
+/// The destructor will be declared if necessary.<br>
///<br>
/// \returns The destructor for this class.<br>
CXXDestructorDecl *Sema::LookupDestructor(CXXRecordDecl *Class) {<br>
- // If the destructor has not yet been declared, do so now.<br>
- if (CanDeclareSpecialMemberFunction(Context, Class) &&<br>
- !Class->hasDeclaredDestructor())<br>
- DeclareImplicitDestructor(Class);<br>
-<br>
- return Class->getDestructor();<br>
+ return cast<CXXDestructorDecl>(LookupSpecialMember(Class, CXXDestructor,<br>
+ false, false, false,<br>
+ false, false).getMethod());<br>
}<br>
<br>
void ADLResult::insert(NamedDecl *New) {<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br>