On Fri, Jun 7, 2013 at 11:45 AM, Adrian Prantl <[email protected]> wrote: > > On Jun 7, 2013, at 11:43 AM, Adrian Prantl <[email protected]> wrote: > >> >> On Jun 7, 2013, at 11:37 AM, Eric Christopher <[email protected]> wrote: >> >>> Two quick comments: >>> >>>> + assert(Setter->getDeclName().isObjCOneArgSelector()); >>>> + // Construct a setter name like SelectorTable::constructSetterName() >>>> + // does, but without entering it into the table. >>>> + SmallString<100> DefaultName("set"); >>> >>> 100 seems a bit big? >>> >> >> Well, as the documentation says, this is the exact same code as in >> SelectorTable::constructSetterName(), and I figured that whoever wrote that >> put some thought into that constant. >> >>>> + DefaultName += PD->getName(); >>>> + DefaultName[3] = toUppercase(DefaultName[3]); >>> >>> Magic numbers! doing magic things! That we don't mention what they are... >>> >>> How about some documentation here? :) >> >> It’s not exactly magic, we are capitalizing the first letter of >> PD->getName(), the first three characters in DefaultString being “set”. >
It would be much more obvious if you actually just wrote this in the code and why it's necessary. > That being said it might make sense to refactor this into a static method of > clang::SelectorTable, so we don’t have it implemented in two places. > Could. Thanks! -eric _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
