After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 727667 - Order problems with constants and static inline arrays
Order problems with constants and static inline arrays
Status: RESOLVED OBSOLETE
Product: vala
Classification: Core
Component: Code Generator
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Vala maintainers
Vala maintainers
Depends on:
Blocks:
 
 
Reported: 2014-04-05 16:01 UTC by Simon Werbeck
Modified: 2018-05-22 15:07 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Change ccode write order for constant declarations (2.62 KB, patch)
2014-04-05 16:03 UTC, Simon Werbeck
rejected Details | Review
Fix C code order problems (2.21 KB, patch)
2014-04-06 11:03 UTC, Simon Werbeck
none Details | Review
Fix C code order problems (2.09 KB, patch)
2014-08-05 07:51 UTC, Simon Werbeck
none Details | Review

Description Simon Werbeck 2014-04-05 16:01:21 UTC
Now that inline arrays can take arbitrary constant expressions as length, the following program should actually compile:

public class Test : Object {
	const size_t CACHE_SIZE = 512;
	static int cache[CACHE_SIZE];
}

void main (string[] args) {
}


However, the generated C code looks like this:

static gint test_cache[TEST_CACHE_SIZE];
static gint test_cache[TEST_CACHE_SIZE] = {0};
// ...
#define TEST_CACHE_SIZE ((gsize) 512)
Comment 1 Simon Werbeck 2014-04-05 16:03:27 UTC
Created attachment 273632 [details] [review]
Change ccode write order for constant declarations

Fixes bug 727667
Comment 2 Luca Bruno 2014-04-05 16:07:51 UTC
Review of attachment 273632 [details] [review]:

Thanks, however it does not pass tests (make check).
Comment 3 Simon Werbeck 2014-04-06 11:03:28 UTC
Created attachment 273655 [details] [review]
Fix C code order problems

I tested it so it 'should' work now. The problem was that declarations were always written first and since macro expansions are not considered as such they come always last.
Comment 4 Luca Bruno 2014-05-06 23:13:49 UTC
Thanks for your patch. However:
1. I'd keep write_declaration/write as is instead of write_combined, and rather change the ccodemacroreplacement to use write_declaration instead of write.
2. Using write_combined does not save you from the missing dependency. Nowhere when visiting the field declaration, the length is emitted. That means it's still possible for the length to come after the declaration, because there's no dependency.

Fixing the two above is not enough in case the constant and the field are public. In fact, header files are broken. Example: valac -H foo.h foo.vala. If you are unlucky and the write of the field comes before the constant, that's broken, and it's not even easy to fix.
The problem is that, in the header files, until now there was no need for tracking dependencies for symbol access, but only between types.

The broken code reference: valaccodememberaccessmodule.vala line 130:
generate_constant_declaration (c, cfile, ...

As you can see, cfile is hardcoded, but we want the declaration to be written in the header file, in case the constant is public!

This is what happens:
1. Field gets visited
2. Field declaration for the header is generated, because it's a public field (valaccodebasemodule line 1188), note in the *header_file*
3. Let's say now we add the dependency with array_type.length.emit(this), in which case we'll emit the memberaccess referring the constant
4. The memberaccess module generates the constant declaration in *cfile* instead of the header file.

Now that's a nice challenge to find a good code design for making memberaccess choose the right decl space.
Comment 5 Simon Werbeck 2014-08-05 07:51:59 UTC
Created attachment 282500 [details] [review]
Fix C code order problems

Changes made:
 * use 'write_declaration' instead of 'write' for macro replacements.
 * when emitting fixed-size array lengths, explicitly set cfile to the correct decl_space

The second change is more of an ugly hack but should be save since Array.length can only contain constant expressions anyway.
Comment 6 GNOME Infrastructure Team 2018-05-22 15:07:41 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/vala/issues/440.