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

Add C++ style method definition and invocation. #839

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

marsupial
Copy link
Contributor

Description

Ads ability to invoke and define functions with an implicit this argument for both structs and vector types.

struct A {
  int val;

  int testA(int i) {
    this.val += i;
    return val; // implicit this
  }
};

void A::testB() {
  val += testA(2);  // implicit this for both val and function call
}

A a;
a.testA(2);
float color::average() {
  return (this[0]+this[1]+this[2]) / 3;
}

color(1,2,3).average();
float average(color c) {
  return (c[0]+c[1]+c[2]) / 3;
}

color Cf;
Cf.average();

I'm not particularly keen on the this[i] syntax of the second example and would prefer named component access (#836) and to make that a syntax error.

The third example is perhaps a bit to broad and could be limited to opt in or out via some meta-data on the function.

Tests

Variety of tests defining, invoking, and warning about variables shadowing fields:
testsuite/function-method
testsuite/oslc-err-method
testsuite/oslc-err-names

Checklist:

  • I have read the contribution guidelines.
  • I have previously submitted a Contributor License Agreement.
  • I have updated the documentation, if applicable.
  • I have ensured that the change is tested somewhere in the testsuite (adding new test cases if necessary).
  • My code follows the prevailing code style of this project.

@marsupial marsupial mentioned this pull request Dec 29, 2017
5 tasks
@dankripac
Copy link

Hi there, no comments about the details in the PR from me at this stage (I'm interested though!)

Can just suggest for these sort of major language additions/changes that the oslc compiler emits a cpp macro define that indicates the presence of the feature?

I haven't looked through the code in detail (sorry!) so it may be there already?

But for example in this case:
#define OSL_OBJECT_METHOD_SUPPORT 1

And perhaps for your other PR
#define OSL_OBJECT_INIT_LIST_SUPPORT 1

Absolutely no attachment to those names so feel free to change the naming schema/convention but just some sort of mechanism for osl shader writers to migrate their code gracefully between major feature leap versions of OSL.

Interesting stuff though! I'm not adverse to having a few C++-isms in OSL myself!
I enjoyed some of those types of features that crept into RSL 2.0+ in Renderman.
Especially resizable dynamic arrays - they were super handy!
Also, basic inheritance was sometimes handy when composing generic algorithms that operate on base or parent classes (not so fussed about this type of feature TBH)

I realise these sorts of things are hard to implement efficiently in a language but dynamic resizable arrays sure helped cut some corners when you are designing more niche/experimental algorithms (or bubble sorting algorithms etc). Sorry to go off topic a little but just throwing in 2 pence worth on this while I can :)

@marsupial
Copy link
Contributor Author

Using OSL_VERSION_MAJOR and OSL_VERSION_MINOR macros should work.

Besides the convenience of init-list syntax, I need OSL & C++ to be able to intermingle a bit better than they currently can. I can't speak for the maintainers, but inheritance and dynamic memory isn't on the horizon for me.

@marsupial
Copy link
Contributor Author

Interestingly, this also allows a clean path forward for the class of ambiguity:

float func (float x)  { return 1; } 
float func (vector x) { return 2; } 
float func (color x)  { return 3; } 
func(noise(1)); // warning

func(vector(1).noise()); // explicit and prettier than typecasting

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.

2 participants