Opened 5 years ago

Closed 4 years ago

#5384 enhancement closed fixed (fixed)

Provide a library for bitvector-like valued named constants

Reported by: exarkun Owned by: exarkun
Priority: normal Milestone:
Component: core Keywords:
Cc: Branch: branches/flag-constants-5384
(github, coverage, patch, buildbot, log)
Author: exarkun


#4671 proposed adding a lot of new symbolic constant features to a helper somewhere in Twisted. This is for the kind of symbolic constant that has a value like a bit in a bitvector and which can be combined from other constants which are part of the same logical group, using all of the traditional bitwise operations.

This would be useful in conch, because SSH uses lots of bitvector-style constants.

Change History (5)

comment:1 Changed 4 years ago by exarkun

  • Author set to exarkun
  • Branch set to branches/flag-constants-5384

(In [33419]) Branching to 'flag-constants-5384'

comment:2 Changed 4 years ago by exarkun

  • Keywords review added
  • Owner exarkun deleted

I implemented many features. Possibly there is room for more, but maybe this is a good self-contained start? One thing that I'm somewhat uncertain about is the behavior of (SomeFlags.A | SomeFlags.B) is (SomeFlags.A | SomeFlags.B) or perhaps even more interestingly, SomeFlags.A is (SomeFlags.A | SomeFlags.B ^ SomeFlags.B) (in contrast to SomeFlags.A is SomeFlags.A).

Also, I feel like the documentation is terribly repetitive.

Build results

Finally, this is the last ticket for #4671, so when I close this ticket I expect to close that ticket as well.

comment:3 Changed 4 years ago by glyph

The buildbots are in a pretty nasty state, and the patch is a bit large, but I like what I see so far. I will think about it some more and hopefully look at a more comprehensive list of build results when the builders are back online.

comment:4 Changed 4 years ago by therve

  • Keywords review removed
  • Owner set to exarkun
  • +    Tests for L{twisted.python.constdants.Flags}, a base class for containers of

Typo: constants.

  • Flags and FlagConstant should be added to constants.__all__.
  • +        result._realize(self._container, set([]), 0)

I don't think you need to pass anything to build an empty set.

  • +        bits set which were not set in the original constant.  constants.

There is an extra "constants" here.

  • It may be more interesting to also test READ in FlagConstantNegationTests.test_value, as otherwise you don't check completely the value passed in the realize call in invert.
  • +            if (flag.value & self.value) == 0:

You don't need the parenthesis here.

Great branch, please merge if the buildot is happy.

comment:5 Changed 4 years ago by exarkun

  • Resolution set to fixed
  • Status changed from new to closed

(In [33677]) Merge flag-constants-5384

Author: exarkun Reviewer: therve Fixes: #5384, #4671

Add bitvector-like constant support to twisted.python.constants.

Note: See TracTickets for help on using tickets.