Skip to content

Conversation

per42
Copy link

@per42 per42 commented Sep 16, 2025

No description provided.

Copy link
Member

@dcbaker dcbaker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like you've tried to implement a single compiler class for handling both C and C++, which is not the way Meson's abstractions are structured. Generally when you have this kind of code sharing what we do is add a new file in mesonbuild/compilers/mixins/windriver.py, with a DiabCompilerMixin class (there's lots of examples), and then implement a DiabCCompiler(DiabCompilerMixin) and DiableCPPCompiler(DiabCompilerMixin) with just the C and C++ unique pieces implemented in the leaf classes.

There's more to look at here, but some of the comments I would give should resolve thiemselves if you do that split.

Comment on lines +3 to +5
## Added Wind River Diab compiler suite for bare-metal PowerPC

Tested with C++, C and assembler using [Diab](https://www.windriver.com/products/diab-compiler) 5.2.1.0 from 2004.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The release notes are compiled at release time by a script from individual snippets in docs/markdown/snippets/*.md, so this should be moved there.

from .. import options
from .. import mlog
from ..mesonlib import MesonException, version_compare
from ..mesonlib import MesonException, version_compare, File, FileOrString
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FileOrString is a just for typing, so it should go inside a if T.TYPE_CHECKING: block.


## Added Wind River Diab compiler suite for bare-metal PowerPC

Tested with C++, C and assembler using [Diab](https://www.windriver.com/products/diab-compiler) 5.2.1.0 from 2004.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've mentioned C here, but there's no C compiler implementation, just a C++. That means if a user requests project('name', 'c') then this compiler won't be selected.

Comment on lines 1369 to 1370
def get_compiler_check_args(self, mode: CompileCheckMode) -> T.List[str]:
return super(CPPCompiler, self).get_compiler_check_args(mode)# + ['-fpermissive']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it just does a super() call? an it be dropped.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CPPCompiler adds the -fpermissive flag which Diab doesn't accept. My intention here is to skip that by directly returning CPPCompiler's super()'s output.

The commented # + ['-fpermissive'] should be removed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay.

Side note to self, we should not just blindly add -fpermissive (or equivalent)

@per42
Copy link
Author

per42 commented Sep 16, 2025

It looks like you've tried to implement a single compiler class for handling both C and C++, which is not the way Meson's abstractions are structured. Generally when you have this kind of code sharing what we do is add a new file in mesonbuild/compilers/mixins/windriver.py, with a DiabCompilerMixin class (there's lots of examples), and then implement a DiabCCompiler(DiabCompilerMixin) and DiableCPPCompiler(DiabCompilerMixin) with just the C and C++ unique pieces implemented in the leaf classes.

There's more to look at here, but some of the comments I would give should resolve thiemselves if you do that split.

I wasn't sure how much having separate compilers depended on how much of a suite entry point for multiple languages was provided. In the project I work on dplus handles C++, C and assembler by invoking separate binaries depending on the source file extension. It also does the linking. It is also possible to directly call the C compiler dcc and the linker dld - in fact the option subprog_cmd logs those commands as called by dplus.

I agree that even if I added C as language it would not be ideal to use dplus for a non-C++ project, both because it is not as clean, and also because the linker command will include C++ libraries (libd).

What I couldn't figure out was how to create multiple compiler classes without duplicating the generated options. It has options to set the target cpu, object format and add basic os functionality (similar to .spec files in arm-none-eabi). It also has some suite options for license checking, and symbol map generation. All of these are cpp_ prefixed options, but I suppose a C compiler would add c_ duplicates?

I didn't see how other compilers added cpp_args and cpp_link_args, but they were required to exist so I created them.

I tried to find a way to point out a linker script in the project source, possibly using options, but resorted to manually add link_args and link_depends on the executables in meson.build.

Finally I wonder if there needs to be test cases for this, given it is such an exotic compiler.

@dcbaker
Copy link
Member

dcbaker commented Sep 18, 2025

I didn't see how other compilers added cpp_args and cpp_link_args, but they were required to exist so I created them.

That's all provided by the base class, so the CCompiler or CPPCompiler

@dcbaker
Copy link
Member

dcbaker commented Sep 18, 2025

Finally I wonder if there needs to be test cases for this, given it is such an exotic compiler.

If the compiler is freely available running tests would be good.

The most recent compiler addition was the tasking compiler, I'd have a look at that series if you want some ideas on getting this going.

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