Closed Bug 391147 Opened 13 years ago Closed 13 years ago

hunspell doesn't build with msvc7.1 (no support for variadic macros, "unexpected in macro formal parameter list")

Categories

(Core :: Spelling checker, defect)

x86
Windows XP
defect
Not set
blocker

Tracking

()

RESOLVED FIXED

People

(Reporter: Gavin, Assigned: longsonr)

References

()

Details

Attachments

(1 file, 2 obsolete files)

http://lxr.mozilla.org/seamonkey/source/extensions/spellcheck/hunspell/src/atypes.hxx#65

msvc7.1 barfs on that line, because it doesn't support variadic macros.
msvc7.1 isn't officially supported on the trunk, but many people still use it. It'd be nice if we could at least workaround this issue, if not fix it (and ideally upstream the fix to avoid maintenance hassles).
Attached patch patch (obsolete) — Splinter Review
Attachment #275582 - Attachment is patch: true
Attachment #275582 - Flags: superreview?(mscott)
Attachment #275582 - Flags: review?(mscott)
Might I suggest asking Nemeth to review it too?
OOps, variadic macro is only C99 standard, not C++:
http://en.wikipedia.org/wiki/Variadic_macro.

I will use this nice patch in next Hunspell, but without variable names (unused variable names cause warnings in OOo build environment), if it is ok for MSVC:

#ifndef HUNSPELL_WARNING
#ifdef HUNSPELL_WARNING_ON
#define HUNSPELL_WARNING fprintf
#define WARNVAR warnvar
#else
static inline void HUNSPELL_WARNING(FILE *, const char *, ...) {}
#define WARNVAR
#endif
#endif

Thanks, Laci
Attached patch address review comments (obsolete) — Splinter Review
I've removed the variable names as suggested but you need #define WARNVAR warnvar rather than #define WARNVAR otherwise the code does not compile.
Attachment #275582 - Attachment is obsolete: true
Attachment #275754 - Flags: superreview?(mscott)
Attachment #275754 - Flags: review?(mscott)
Attachment #275582 - Flags: superreview?(mscott)
Attachment #275582 - Flags: review?(mscott)
Attachment #275754 - Attachment is obsolete: true
Attachment #275798 - Flags: superreview?(mscott)
Attachment #275798 - Flags: review?(mscott)
Attachment #275754 - Flags: superreview?(mscott)
Attachment #275754 - Flags: review?(mscott)
scott: can you please r+ this and get it in to the tree?
Severity: normal → blocker
Flags: blocking1.9+
Comment on attachment 275798 [details] [diff] [review]
#define should be inside outer #endif

thanks for the patch! I'll check this in right now.
Attachment #275798 - Flags: superreview?(mscott)
Attachment #275798 - Flags: superreview+
Attachment #275798 - Flags: review?(mscott)
Attachment #275798 - Flags: review+
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
I had to back this out, the tree is closed.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to comment #5)
>I've removed the variable names as suggested but you need #define WARNVAR
>warnvar rather than #define WARNVAR otherwise the code does not compile.
Or you could just remove the #define - WARNVAR is a perfectly legal name ;-)
Assignee: mscott → longsonr
Status: REOPENED → NEW
checked in.
Status: NEW → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.