Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Operator __pragma #526

Merged
merged 5 commits into from Sep 20, 2020
Merged

Operator __pragma #526

merged 5 commits into from Sep 20, 2020

Conversation

Daniel-Cortez
Copy link
Contributor

@Daniel-Cortez Daniel-Cortez commented May 16, 2020

What this PR does / why we need it:

This PR implements operator __pragma (see #267).

Currently available options:

  • deprecated
  • unused
  • unread
  • unwritten
  • nodestruct
  • naked

Examples of use:

native Func() __pragma("deprecated - use OtherFunc() instead");

stock __pragma("naked") NakedFunc2() {}

NakedFunc()
{
    __pragma("naked", "deprecated - use NakedFunc2() instead");
}

new __pragma("nodestruct") Tag:b = Tag:0;
new a = 0 __pragma("unused");
new __pragma("unread") c = 0 __pragma("unwritten");
new d = 0 __pragma("unread", "unwritten");

Which issue(s) this PR fixes:

Fixes #267

What kind of pull this is:

  • A Bug Fix
  • A New Feature
  • Some repository meta (documentation, etc)
  • Other

Additional Documentation:

@Daniel-Cortez Daniel-Cortez requested a review from a team as a code owner May 16, 2020 14:39
@Y-Less
Copy link
Member

Y-Less commented May 16, 2020

__pragma("unread", "unwritten");

That's a good solution to multiple properties.

@Y-Less
Copy link
Member

Y-Less commented May 16, 2020

Why didn't you reuse the existing #pragma code? It seems it would have been simpler to just use that and add an extra parameter to disable some like codepage for __ compared to #.

@Daniel-Cortez
Copy link
Contributor Author

Because the code for __ is too different from #.

  • The option names are handled differently.
    Since operator __pragma can be specified before the symbol is known, e.g.
new __pragma("nodestruct") Tag:var;

... , I had to make the compiler memoize the specified attributes, so they could be applied to the symbol later - the most obvious solution was to store bit flags into a global variable.
This is why while #pragma distinguishes the options by their names, __pragma uses bit flags when applying the attributes.
Of course, I could just convert the bit flags back into attribute names, but there are still other problems listed below.

  • The option names and parameters are read differently.
    In #pragma the option name and parameters are read directly (by manipulating with lptr), but in __pragma they are read from the literal queue, because the operator reuses the standard string parsing mechanism.

  • The parameters are handled differently.
    In #pragma unused/unread/unwritten/nodestruct the symbol name is specified after the option name (e.g. #pragma nodestruct var), but in __pragma the symbol is separate from the operator.

Thus the only thing left to reuse is the code responsible for applying the attributes to symbols (which isn't really that much of code), which could be moved into separate reusable functions:

SC_FUNC void pragma_unused(symbol *sym, int unread, int unwritten)
{
  sym->usage |= unwritten ? 0 : uREAD;
  if (sym->ident == iVARIABLE || sym->ident == iREFERENCE
      || sym->ident == iARRAY || sym->ident == iREFARRAY)
    sym->usage |= unread ? 0 : uWRITTEN;
}

SC_FUNC void pragma_nodestruct(symbol *sym)
{
  if (sym->ident==iVARIABLE || sym->ident==iARRAY)
    sym->usage |= uNODESTRUCT;
}

@Y-Less
Copy link
Member

Y-Less commented May 19, 2020

That makes sense.

@Daniel-Cortez
Copy link
Contributor Author

Done, moved the code for deprecated, unused, unread. unwritten and nodestruct into reusable functions, as described in the post above.

While I was at it, I also fixed a potential memory leak in #pragma deprecated (the memory block for the deprecaton message is not freed if it's unused, which might happen either because of end of script or if there are two deprecation #pragmas used in a row).

@stale
Copy link

stale bot commented Aug 29, 2020

This issue has been automatically marked as stale because it has not had recent activity.

@stale stale bot added the state: stale label Aug 29, 2020
Y-Less added a commit to pawn-lang/YSI-Includes that referenced this pull request Sep 19, 2020
@Y-Less
Copy link
Member

Y-Less commented Sep 19, 2020

I've merged this, but it would be nice if it were possible to use "naked" inside functions as well as in their header:

Func()
{
    __pragma("naked");
}

@stale stale bot removed the state: stale label Sep 19, 2020
@Daniel-Cortez
Copy link
Contributor Author

it would be nice if it were possible to use "naked" inside functions as well as in their header

Done.
As a bonus, aside from "naked", the other attributes can be used inside functions as well:

NakedFunc()
{
	__pragma("naked", "deprecated - use NakedFunc2() instead");
}

@Y-Less
Copy link
Member

Y-Less commented Sep 20, 2020

Oh, I just discovered that naked removes PROC as well. That's good to know.

@Y-Less
Copy link
Member

Y-Less commented Sep 20, 2020

The tests don't really cover that, but they're only testing compiler warnings. Is that actually correct?

@Y-Less Y-Less merged commit 555027f into pawn-lang:dev Sep 20, 2020
@Daniel-Cortez
Copy link
Contributor Author

Daniel-Cortez commented Sep 20, 2020

The tests don't really cover that, but they're only testing compiler warnings. Is that actually correct?

If by "that" you mean all other attributes not being tested inside functions, then I think it might unnecessarily bloat the tests. As you can see it in ab66969, the only difference is in the code that calls functions dopragma() and pragma_apply(curfunc), making it possible for __pragma to be used inside functions; the code that reads and applies the attributes is still the same, which is why I added only one basic check to make sure the syntax is being handled correctly. But I can expand the tests if you think that check isn't enough.

@Y-Less
Copy link
Member

Y-Less commented Sep 20, 2020

Let me rephrase:

I just discovered that naked removes PROC as well, is that correct? The tests only cover compiler warnings, not assembly output, so they don't cover the lack of PROC.

I was just talking about PROC in both posts.

@Daniel-Cortez
Copy link
Contributor Author

Daniel-Cortez commented Sep 20, 2020

Ah, so that's what you meant.
Alright then, I'll add a P-code test.

EDIT: OK, done: #571.

@Daniel-Cortez Daniel-Cortez mentioned this pull request Sep 20, 2020
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants