Concatenating context variables with identifiers in %define

[Rob Neff]

While working on NASMX I uncovered an issue in the %define preprocessing of identifiers involving Nasm context variables that drove me nuts for quite some time before I found it.  Basically, when concatenating context variables with additional identifiers to form a %define, valid identifiers appended AFTER a context variable are being ignored. This is true of all 2.09x release versions, except v2.09.02 which permits this.
All v2.09x versions do not permit prepending concatenation of prepended identifiers with context vars.
I would expect (think?) that the behavior of
    %define SOMEIDENTIFIER_%{$contextvar}
would exhibit the same behavior as
    %define SOMEIDENTIFIER_%{1}
but it currently does not.

The following code snippet shows 2 cases:
case 1 - a valid identifiers that appear AFTER the context variable
case 2 - a valid identifiers that appear BEFORE the context variable

Code: [Select]


%push

%define %$__proc WndProc
%define %$__calltype 3
%define %$__suffix 16
%define %$__nxtok_decorated_name _%{$__proc}@%{$__suffix}

;// case 1: works correctly if __nxsig@@ ( or any other valid identifiers )
;// appears AFTER context variable when using Nasm v2.09.02
;// Fails on Nasm v2.09 and v2.09.01
; %ifdef %{$__nxtok_decorated_name}__nxsig@@
;        %fatal error - ALREADY DEFINED??? %{$__nxtok_decorated_name}__nxsig@@
; %else
;        %fatal success - working correctly, not defined!!
; %endif

;// case 2: does not work correctly if __nxsig@@ ( or any other valid identifiers )
;// appears BEFORE context variable!!!
%ifdef __nxsig@@%{$__nxtok_decorated_name}
    %fatal error - ALREADY DEFINED??? __nxsig@@%{$__nxtok_decorated_name}
%endif

;// This is the global define required for function signatures
%xdefine __nxsig@@%{$__nxtok_decorated_name} %$__calltype,%$__proc,%$__suffix,%{$__nxtok_decorated_name}
%fatal function signature: __nxsig@@%{$__nxtok_decorated_name}

%pop


This is the main reason why helper macros are needed which are subsequently called with the necessary paramenters to correctly form a valid %define identifier using the macro parameters.  But this alone increases processing time tremendously when there are over a thousand defines required with each define having to call various macros just to work around this issue.  Fixing this will solve a slew of performance problems ( at least in regards to NASMX or any other project that uses Nasm preprocessing heavily is concerned anyways ).

I have already added this to the issue tracker:
[ nasm-Bugs-3073640 ] concatenating context variables with identifiers in %define

[Rob Neff]

Posting here for folks that don't subscribe to bug tracker:

As an addendum to this issue: if the construct %{} is replaced with %[] then we get the desired results.

When comparing the Nasm documented uses of %{} to %[]...

4.3.8 Concatenating Macro Parameters: "This concatenation can also be applied to other preprocessor in-line objects, such as macro-local labels (section 4.3.2) and context-local labels (section 4.7.2)."

4.1.3 Macro Indirection: %[...]: "%[...] concatenates to adjacent tokens in the same way that multi-line macro parameters do"

So why are there differences?  From my perspective they are actually both the same and probably should be using the exact same code.  It theoretically makes no difference to either %[] or %{} with regard to expansion of single-line macros, macro parameters, macro local variables, or context variables.  Why there are, in fact, two documented ways to perform the same task I'm not entirely sure except that currently %[] appears to work and %{} does not ( using Nasm v2.09x ).

I would like to see some consistency regarding this behavior such that:

%[] always does the immediate macro expansion and makes the %define available immediately to code following it's original point of definition (as is currently done).

%{} either does the exact same thing (which is kind of pointless as we already have %[] ) or is clearly marked for macro parameter expansion only ( %{1}, %{2}, etc ).  Although that would probably break other apps that relied on the %{%foo}bar examples in the docs, so maybe not a good idea.

I've been slugging through this issue (macro expansion/indirection) over several iterations of Nasm and hope that this can be put to rest soon.  Thank you all for your time and effort.

[Cyrill Gorcunov]

Hi Rob, thanks for report! Didn't manage to dive into it today, hope tomorrow

[Bryant Keller]

This is as expected, %[...] occurs earlier than %{} and is the reason why you'll notice some of my OOP code does things like %[%{$blah}_%{$bleh}] to force the entire operation to be processed earlier than it would as just %{$blah}_%{$bleh}. I remember working with NASM before %[] was available, it was a nightmare and lots of things that we can do now would have been impossible without it. So what is the suggestion? Process context stuff earlier? This is another one of those old discussions that tbh I think the solution was a bit of a hack-fix but it worked so nobody really complained. I kinda like the %[...] solution and it's not that bad, even though it does make code look really horrible sometime -- for example:

Code: [Select]

%if %[%{$name}]_total_static_methods > 0
    %assign %$ii 0
    %rep %[%{$name}]_total_static_methods
        . %+ %[ %{$name} %+ _static_method_ %+ %{$ii} ] RESD 1
        %assign %$ii %{$ii} + 1
    %endrep
%endif
Works great, works as it's supposed to and as I expect it to, but it is kinda ugly for those who aren't as used to NASM's syntax.

But I think you really answered your own question. The reason for the difference was simply the scope of the variables, %{...} allows you to perform a concatenation without needing to use the %+ directive whereas the %[...] actually change the scope of the translation by forcing it to be interpreted ASAP.

[Rob Neff]

But the %{} has a subtle bug with regard to its positioning when assembled with v2.09.02 compared to v2.09.01 and v2.09.
I agree regarding the scoping rules but personally I think these two constructs need further clarification in the docs.
I have no preference regarding the syntax, only that it work "as advertised"
Believe me when I tell you Bryant, after inheriting and hacking on NASMX, I truly can feel yours and Keith's pain! 
I've got thousands of lines of code built into the next beta version of NASMX supporting various calling conventions, 32/64-bit portability of data and function calls across Linux and Windows, and tbh this is usually the main issue I have to fight through when using Nasm.  But this feature is so useful that I can't imagine coding without it!

[Cyrill Gorcunov]

Hi Rob, guys,

I've restored old behavior of preprocessor (well, down to 2.05.01). We've changed the rules of tokens concatenation during 2.06 development series. Peter pointed that there might be some error previously that is why the changes were needed.

All-in-one, I've restored behavior but didn't push it into main repo since I'm waiting for Peter's approval for such
a massive change. Meanwhile, Rob, could you please fetch nasm repo from my repo and test it?

It's here http://repo.or.cz/w/nasm-cyr.git
or the link to ZIP'ed sources http://repo.or.cz/w/nasm-cyr.git/snapshot/15cd5fea6c53abf5f9949823f86efb25d8d979da.zip

[Rob Neff]

Most certainly I'll help out.  I'll download and play around with it this weekend.
However, 2.05.01 was published almost 2 years ago with quite a few modifications to the preproc.c file since then.  Is reverting back less painful than fixing forward?

[Cyrill Gorcunov]

Rob, it was not "revert" but rather bringing back old concatenation rules.

[Bryant Keller]

Cyrill,

Your revert so far hasn't broken anything I've got laying around (which I use both %[] and %{} heavily so that says a lot). Even when building against altnasmx (a standard macro build I keep at the [b0x] server) everything runs fine; that's a sorta big thing because altnasmx sometimes doesn't work because of a line which technically shouldn't work anyway... %[__%{1}_convention]_invoke where, since this is a standard macro it's being generated early anyway, and I'm forcing early interpretation using %[...], but then there is a late binding %{1} right in the middle which mucks up the whole thing. This line breaks from version to version when you are a standard macro (nothing wrong with this as a normal include file, btw) which is one of the many reasons altnasmx stays more of an in-house or beta-users only script. 

I'll keep throwing code at it for a while, see if anything breaks.. well, breaking NASM isn't an issue (can do that easily) just breaking it using the %[] and %{} is what I'll be aiming for.

Anyway, keep up the great work mate.

Best Regards,
Bryant Keller

[Rob Neff]

Downloaded and attempting to build.

Build stopped due to requiring directiv.h as there is no rule in your make file for it so I added

Code: [Select]

# Directives hash
directiv.h: directiv.dat directiv.pl perllib/phash.ph
$(PERL) $(srcdir)/directiv.pl h $(srcdir)/directiv.dat directiv.h
directiv.c: directiv.dat directiv.pl perllib/phash.ph
$(PERL) $(srcdir)/directiv.pl c $(srcdir)/directiv.dat directiv.c

to msvc.mak and cleared that hurdle.

Next build attempt: missing pptok.ph.  Again, this was not found in your version of msvc.mak.

Code: [Select]

pptok.ph: pptok.dat pptok.pl perllib/phash.ph
$(PERL) $(srcdir)/pptok.pl ph $(srcdir)/pptok.dat pptok.ph

Attempting another build I encountered this:

Code: [Select]

macros.c
macros.c(175) : error C2466: cannot allocate an array of constant size 0
macros.c(176) : error C2059: syntax error : '}'
NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual Studio 8\VC\BIN\cl.EXE"' : return code '0x2'
Stop.

Perhaps at this point a package with pre-built win32 binaries for testing would suffice

[Cyrill Gorcunov]

I suppose this post was written _before_ I gave you a PM link to pre-made header files, right?

[Rob Neff]

Yes, Cyrill, that was before you PM'd me the link with the code, which btw works like a charm so far

[Cyrill Gorcunov]

Excellent, HPA promised to review this patch next week, so I guess we merge it more/less soon. Thanks a huge for testing, Rob!