On Mon, Jun 18, 2012 at 5:29 PM, João Matos <[email protected]> wrote: > Thanks for the review. > >> Per the coding conventions, this should be named BackReferences (and >> likewise for other variable and field declarations in the patch). > > Done.
Still a few left behind with lowercase names (type, canonical, ...). >> This will do different things depending on the order in which the >> arguments to operator= are evaluated. > > I changed it to get the size in a different statement. > >> It looks like this will include cv-qualifiers on function parameter >> types in the mangled name. That can't be right, since they're not part >> of the function type. I suspect you should drop the uses of >> getTypeSourceInfo() here. (This bug seems to predate your change). > > Well I tried dropping it, but it is needed for arrays, tests were > failing without it. OK, I've been told that MSVC does not actually follow the C++ standard here, and includes top-level cv qualifiers on function arguments in parameter types. That's completely terrible, but it seems like we need to do the same for compatibility. :-( >> We use 'cxx11' not 'cpp11' in the tests/ area to refer to C++11. > > Renamed. > > I also added a fix in the new patch for bug #12603 > (http://llvm.org/bugs/show_bug.cgi?id=12603). LGTM other than the capitalization. _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
