MMgc ought to expose the same primitive types as the rest of the VM, but does not, and whether or not it does depends on the platform: * On Linux, at least uintptr_t is not known. * On Windows, at least uint8_t is not known. This causes needless build breakage.
in my local branch I have split out all the C99 types from avmplusTypes.h into "avmplus_stdint.h" -- basically, a wraper to declare the types properly (MSVC) or just include <inttypes.h> (everywhere else). sounds like mmgc needs something similar... want me to clean it up and submit it as (say) "mmgc_stdint.h" ?
That sounds like the right solution.
I'll do something like that later today (and probably have avmplus piggyback on it rather than redeclare).
At the risk of multiplying entities beyond necessity, should a header like this not be in a separate top-level directory (think of it as "substrate" though the name is terrible, what we're really talking about is the porting API) and included from both the VM and MMgc? Does MMgc really need to be self-contained, and if so, to what extent does it need to be in a single directory?
long-term, I completely agree with you; both MMGC and Avmplus implement stuff that should be in a lower-level substrate. short-term, though, this is an expedient fix.
Created attachment 346713 [details] [diff] [review] Patch Unify all C99 int type handling into mmgc_stdint.h (for MMgc) and avmplus_stdint.h (for avmplus). Note that the latter currently just includes the former. Long-term, we probably should pull this sort of thing into a more well-defined base layer; this is intended as a short-term fix to ensure that all C99 int types are predictably available in MMgc, which they weren't before.
Assignee: nobody → stejohns
Attachment #346713 - Flags: review?(lhansen)
Comment on attachment 346713 [details] [diff] [review] Patch It seems to me that there's a subtle change here. With the old system one could deselect MMGC_64BIT, and intptr/uintptr would be 32-bit values, even if uintptr_t were a 64-bit value (because the compiler was doing 64-bit compiles, say). With the new system, intptr/uintptr are always the same as intptr_t/uintptr_t, so this functionality is lost. I'm not objecting a whole lot since it's not obvious to me that the functionality was useful or correct, but. Have we actually lost something? (Otherwise no objections.)
Attachment #346713 - Flags: review?(lhansen) → review+
old system sounds incorrect to me -- if we were building 64-bit code that MMGC_64BIT must be defined for correct behavior. (ie what we are losing sounds like a defect in the first place.)
pushed to redux as changeset: 1061:1a44ddef4243 (plus two merge nodes)
Created attachment 347606 [details] [diff] [review] Patch Update to mmgc_stdint.h that should fix compile errors on Windows Mobile.
Created attachment 347608 [details] [diff] [review] Patch #2 Previous patch fixed portable-build but broke visual-studio build. this should be happy for both.
Comment on attachment 347608 [details] [diff] [review] Patch #2 Verified patch works for windows mobile, windows 32bit and mac
pushed to redux as changeset: 1093:4c3337488923
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Resolved fixed engineering / work item that has been pushed. Setting status to verified.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.