Skip to content

Conversation

@fingolfin
Copy link
Member

If _Static_assert resp. static_assert are not available, we fall back to a
trick involving a typedef that's invalid if the assertion fails.

In order to avoid clashes with repeating (identical) typedefs, we tried to
insert the line number (via the __LINE__ macro), but that did not actually
work due to the way the C preprocessor works. To fix this, we need to use two
helper macros which ensure __LINE__ get evaluated before being concatenated
to another string.

I am not sure whether it's worth to mention this in the release notes; but then we did get a report involving this problem, so perhaps it is... In any case, I've not yet added any "release notes" labels.

If `_Static_assert` resp. `static_assert` are not available, we fall back to a
trick involving a typedef that's invalid if the assertion fails.

In order to avoid clashes with repeating (identical) typedefs, we tried to
insert the line number (via the `__LINE__` macro), but that did not actually
work due to the way the C preprocessor works. To fix this, we need to use two
helper macros which ensure `__LINE__` get evaluated before being concatenated
to another string.
@fingolfin fingolfin added kind: bug Issues describing general bugs, and PRs fixing them topic: kernel labels Aug 10, 2018
@codecov
Copy link

codecov bot commented Aug 10, 2018

Codecov Report

Merging #2691 into master will increase coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2691      +/-   ##
==========================================
+ Coverage   75.42%   75.42%   +<.01%     
==========================================
  Files         478      478              
  Lines      241598   241598              
==========================================
+ Hits       182227   182228       +1     
+ Misses      59371    59370       -1
Impacted Files Coverage Δ
hpcgap/lib/hpc/stdtasks.g 64.96% <0%> (+0.25%) ⬆️

@ChrisJefferson
Copy link
Contributor

Good fix. I should have spotted this originally, as I've had exactly this kind of stupid preprocessor issue before.

@fingolfin fingolfin merged commit 38bf634 into gap-system:master Aug 11, 2018
@fingolfin fingolfin deleted the mh/static_assert branch August 11, 2018 20:12
@olexandr-konovalov
Copy link
Member

@ChrisJefferson we are getting further reports about this problem and advise to apply the patch to GAP 4.9.2. Should this be backported into stable-4.9 branch then?

@olexandr-konovalov olexandr-konovalov added this to the GAP 4.9.3 milestone Aug 23, 2018
@fingolfin
Copy link
Member Author

If there is going to be a 4.9.3 release, then we should definitely backport this.

@olexandr-konovalov olexandr-konovalov added backport-to-4.9-DONE release notes: added PRs introducing changes that have since been mentioned in the release notes labels Aug 24, 2018
@olexandr-konovalov
Copy link
Member

Backported and added to release notes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind: bug Issues describing general bugs, and PRs fixing them release notes: added PRs introducing changes that have since been mentioned in the release notes topic: kernel

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants