[cfe-commits] r71467 - in /cfe/trunk: lib/CodeGen/CGObjCMac.cpp test/CodeGenObjC/metadata-symbols-64.m
Daniel Dunbar
daniel at zuster.org
Mon May 11 17:24:58 PDT 2009
Nice! A few comments:
On Mon, May 11, 2009 at 12:25 PM, Fariborz Jahanian <fjahanian at apple.com> wrote:
> + /// NoneLegacyDispatchMethods - List of methods for which we do *not* generate
> + /// legacy messaging dispatch.
> + llvm::StringMap<bool> NoneLegacyDispatchMethods;
None -> Non? Also, this would be better to be a DenseSet<Selector>, we
don't want to convert selectors to strings all the time.
> +
> + /// LegacyDispatchedSelector - Returns true if SEL is not in the list of
> + /// NoneLegacyDispatchMethods; flase otherwise.
Misspelled false...
> + // In 64bit ABI, type must be assumed VARARG. It 32bit abi,
it -> in
> } else if (ResultType->isFloatingType()) {
> // FIXME: Sadly, this is wrong. This actually depends on the
> // architecture. This happens to be right for x86-32 though.
Is this FIXME fixed? Seems dangerous if not...
> +/// LegacyDispatchedSelector - Returns true if SEL is not in the list of
> +/// NoneLegacyDispatchMethods; flase otherwise. What this means is that
flase -> false
> +/// except for the 19 selectors in the list, we generate 32bit-style
> +/// message dispatch call for all the rest.
> +///
> +bool CGObjCNonFragileABIMac::LegacyDispatchedSelector(Selector Sel) {
> + // FIXME! Lagcy API in Nonfragile ABI is for 10.6 on
Lagcy -> Legacy
> + if (NoneLegacyDispatchMethods.empty()) {
> + NoneLegacyDispatchMethods["allocWithZone:"] = true;
> + NoneLegacyDispatchMethods["alloc"] = true;
> + NoneLegacyDispatchMethods["class"] = true;
> + NoneLegacyDispatchMethods["self"] = true;
> + NoneLegacyDispatchMethods["isKindOfClass:"] = true;
> + NoneLegacyDispatchMethods["respondsToSelector:"] = true;
> + NoneLegacyDispatchMethods["isFlipped"] = true;
> + NoneLegacyDispatchMethods["length"] = true;
> + NoneLegacyDispatchMethods["objectForKey:"] = true;
> + NoneLegacyDispatchMethods["count"] = true;
> + NoneLegacyDispatchMethods["objectAtIndex:"] = true;
> + NoneLegacyDispatchMethods["isEqualToString:"] = true;
> + NoneLegacyDispatchMethods["isEqual:"] = true;
> + NoneLegacyDispatchMethods["retain"] = true;
> + NoneLegacyDispatchMethods["release"] = true;
> + NoneLegacyDispatchMethods["autorelease"] = true;
> + NoneLegacyDispatchMethods["hash"] = true;
> + NoneLegacyDispatchMethods["addObject:"] = true;
> + NoneLegacyDispatchMethods["countByEnumeratingWithState:objects:count:"]
> + = true;
> + }
> + const char *name = Sel.getAsString().c_str();
> + if (NoneLegacyDispatchMethods[name])
> + return false;
> + return true;
The use of name here isn't safe, Sel.getAsString() returns a temporary
string which will be destroyed. However, this problem goes away of the
table changes to be mapped by Selector instead of string.
Thanks,
- Daniel
More information about the cfe-commits
mailing list