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

Const generics: Constants using const generic structs don't quite work. #767

Open
emilio opened this issue Jun 9, 2022 · 3 comments
Open
Labels

Comments

@emilio
Copy link
Collaborator

emilio commented Jun 9, 2022

cc @jorendorff (not sure of a great fix for this, for the record, filing so that I don't lose track of it).

I wanted to do something like this today:

#[repr(C)]
pub struct FixedPoint<const FRACTION_BITS: u16> {
    value: u16,
}

pub const FONT_WEIGHT_FRACTION_BITS: u16 = 6;

#[repr(C)]
pub struct FontWeight(FixedPoint<FONT_WEIGHT_FRACTION_BITS>);

impl FontWeight {
    pub const NORMAL: FontWeight = FontWeight(FixedPoint { value: 400 << FONT_WEIGHT_FRACTION_BITS });
}

#[no_mangle]
pub extern "C" fn root(w: FontWeight) {}

That generates:

#include <cstdarg>
#include <cstdint>
#include <cstdlib>
#include <ostream>
#include <new>

constexpr static const uint16_t FONT_WEIGHT_FRACTION_BITS = 6;

template<uint16_t FRACTION_BITS>
struct FixedPoint {
  uint16_t value;
};

struct FontWeight {
  FixedPoint<FONT_WEIGHT_FRACTION_BITS> _0;
};
constexpr static const FontWeight FontWeight_NORMAL = FontWeight{ /* ._0 = */ FixedPoint{ /* .value = */ (400 << FONT_WEIGHT_FRACTION_BITS) } };

extern "C" {

void root(FontWeight w);

} // extern "C"

Which looks like it could compile, but doesn't:

out.cpp:17:79: error: use of class template 'FixedPoint' requires template arguments
constexpr static const FontWeight FontWeight_NORMAL = FontWeight{ /* ._0 = */ FixedPoint{ /* .value = */ (400 << FONT_WEIGHT_FRACTION_BITS) } };
                                                                              ^
out.cpp:10:8: note: template is declared here
struct FixedPoint {
       ^
1 error generated.

A workaround could be adding a type alias:

#[repr(C)]
pub struct FixedPoint<const FRACTION_BITS: u16> {
    value: u16,
}

pub const FONT_WEIGHT_FRACTION_BITS: u16 = 6;

pub type FontWeightFixedPoint = FixedPoint<FONT_WEIGHT_FRACTION_BITS>;

#[repr(C)]
pub struct FontWeight(FontWeightFixedPoint);

impl FontWeight {
    pub const NORMAL: FontWeight = FontWeight(FontWeightFixedPoint { value: 400 << FONT_WEIGHT_FRACTION_BITS });
}

#[no_mangle]
pub extern "C" fn root(w: FontWeight) {}

But that doesn't quite cut it either because it doesn't generate the inner expression:

constexpr static const FontWeight FontWeight_NORMAL = FontWeight{ /* ._0 = */ FontWeightFixedPoint{  } };

I think that should be fixable tho.

@emilio emilio added the bug label Jun 9, 2022
@emilio emilio changed the title Const generics: Constants using don't quite work. Const generics: Constants using const generic structs don't quite work. Jun 9, 2022
emilio added a commit that referenced this issue Jun 9, 2022
emilio added a commit that referenced this issue Jun 9, 2022
@emilio
Copy link
Collaborator Author

emilio commented Jun 9, 2022

#768 allows the workaround to well, work.

emilio added a commit that referenced this issue Jun 9, 2022
emilio added a commit that referenced this issue Jun 9, 2022
@landryb
Copy link

landryb commented Jun 11, 2022

Fwiw, not 100% sure but with cbindgen 0.24.3 i fail to build mozilla-beta (actually 102.0b6), which fails this way:

In file included from Unified_cpp_accessible_base0.cpp:119: 
In file included from /usr/obj/ports/firefox-102.0beta6/firefox-102.0/accessible/base/FocusManager.cpp:21:                                                                                                          
In file included from /usr/obj/ports/firefox-102.0beta6/build-amd64/dist/include/mozilla/dom/BrowserParent.h:23:
In file included from /usr/obj/ports/firefox-102.0beta6/build-amd64/dist/include/mozilla/layout/RemoteLayerTreeOwner.h:17:
In file included from /usr/obj/ports/firefox-102.0beta6/build-amd64/dist/include/nsDisplayList.h:48:                                                                                                                 
In file included from /usr/obj/ports/firefox-102.0beta6/build-amd64/dist/include/nsCSSRenderingBorders.h:20:                                                                                                         
In file included from /usr/obj/ports/firefox-102.0beta6/build-amd64/dist/include/gfxUtils.h:23:                                                                                                                      
In file included from /usr/obj/ports/firefox-102.0beta6/build-amd64/dist/include/mozilla/webrender/WebRenderTypes.h:11:                                                                                              
In file included from /usr/obj/ports/firefox-102.0beta6/build-amd64/dist/include/mozilla/webrender/webrender_ffi.h:104:                                                                                              
/usr/obj/ports/firefox-102.0beta6/build-amd64/dist/include/mozilla/webrender/webrender_ffi_generated.h:24:33: error: redefinition of 'ROOT_CLIP_CHAIN'                                                               
constexpr static const uint64_t ROOT_CLIP_CHAIN = ~0;^
/usr/obj/ports/firefox-102.0beta6/build-amd64/dist/include/mozilla/webrender/webrender_ffi.h:76:16: note: previous definition is here
const uint64_t ROOT_CLIP_CHAIN = ~0;
               ^
1 error generated.  

im not saying this is due/related to this issue but i had no issue building 102.0b5 with cbindgen 0.23.0 on OpenBSD. Will revert cbindgen to 0.24.2 to check if that's related to this issue or to other commits done since 0.23.0.

@emilio
Copy link
Collaborator Author

emilio commented Jun 12, 2022

That's known, and it's not a cbindgen bug. Removing the manual definition from webrender_ffi.h should fix this. See https://bugzilla.mozilla.org/show_bug.cgi?id=1773259#c5 and following.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants