On Jul 19, 2012, at 1:39 PM, Timur Iskhodzhanov wrote: >> Otherwise it seems very, very ad-hoc, > I do think this is more of a general rule as the same ABCD encoding > should be used to fix http://llvm.org/PR13182 too. > Maybe I should merge these two fixes into one patch and extract a > "mangleCVQualifiers" function? WDYT? > I was planning to have this as two separate patches.
If this is just about mangling CV qualifiers, then that sounds fine. How you choose to structures the patches doesn't matter much to me. >> which makes me worried that >> this it only works for the test cases we've run through it already. > That's TDD, right? :) TDD is not really appropriate here. We're not developing to vague requirements, we're implementing a spec. The Chromium build is not going to give us complete, or even near-complete, coverage of MSVC's mangling scheme. Fixing just enough to get Chromium to build is not going to leave us with a working mangler; it is going to leave us with a mangler that needs to be fixed again, and again, and again, every time we push a new project through it (or, for that matter, a new version of Chromium). You need to be running experiments and trying to look for unifying principles behind MSVC's manglings. Like, you're mangling qualifiers, but not when it's a pointer, reference (?!), or builtin type that's qualified? Something like that? That's a rather complicated list of special cases. Now, it's possible that there is no fundamental rule here, and that MSVC's mangler has logic just as convoluted in it; if so, fine, the spec's the spec. But please do the investigation, looking for corner cases that illustrate the general rule, and leave comments and test cases behind you that tell us what you've done. John. _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
