-
Notifications
You must be signed in to change notification settings - Fork 161
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
Track location of InstallMethod
and InstallImmediateMethod
#2393
Track location of InstallMethod
and InstallImmediateMethod
#2393
Conversation
BTW, while I found outputting |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very useful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a nice feature.
I think it is not necessary to make the output of
RunImmediateMethods
optional.
... for accessing the methods in METHODS_OPERATION(oper,n). By using this constant, it becomes easier to add additional entries for each method. Note that the layout of METHODS_OPERATION is strictly speaking an internal implementation detail, and thus packages directly accessing it do so at their own risk. If a package accesses METHODS_OPERATION anyway (currently only Browse) it should be updated to use METHODS_OPERATION. (For backwards compatibility, if BASE_SIZE_METHODS_OPER_ENTRY is not defined, it can be set to 4).
... to make it a bit easier to figure out what's going on
This makes the information displayed by TraceMethods and by ApplicableMethod much more useful in many case, which resolves gap-system#2239. For example, consider this method for `AsList`, installed in coll.gi: InstallMethod( AsList, "for collections that are constant time access lists", [ IsCollection and IsConstantTimeAccessList ], Immutable ); With GAP 4.8, no location information is shown at all: gap> TraceMethods(AsList); AsList("abc");; #I AsList: for collections that are constant time access lists With GAP 4.9 / master, the location where the method (i.e., the underlying function) was defined is shown, which already does the job in many cases, but not here: gap> TraceMethods(AsList); AsList("abc");; #I AsList: for collections that are constant time access lists at src/objects.c:fail With this PR, we get this (note that instead of GAPROOT, the actual path is printed) gap> TraceMethods(AsList); AsList("abc");; #I AsList: for collections that are constant time access lists at GAPROOT/lib/coll.gi:302
Before: gap> TraceImmediateMethods( ); gap> g:= Group( (1,2,3), (1,2) );; #I immediate: Size #I immediate: IsCyclic #I immediate: IsCommutative #I immediate: IsTrivial gap> gap> M0:=Magma(FamilyObj([1]), []);; gap> AsSSortedList(M0); #I immediate: RepresentativeSmallest #I immediate: AsList #I immediate: EnumeratorSorted #I immediate: GeneratorsOfDomain #I immediate: Size #I immediate: RepresentativeSmallest #I immediate: IsFinite #I immediate: IsTrivial #I immediate: IsEmpty #I immediate: IsNonTrivial #I immediate: Size #I immediate: RepresentativeSmallest [ ] After (note how e.g. the entry for EnumeratorSorted points at coll.gi:379, and not at the location where the method function was defined): gap> TraceImmediateMethods( ); gap> g:= Group( (1,2,3), (1,2) );; #I RunImmediateMethods #I immediate: Size at GAPROOT/lib/coll.gi:179 #I immediate: IsCyclic at GAPROOT/lib/grp.gi:40 #I immediate: IsCommutative at GAPROOT/lib/magma.gi:196 #I immediate: IsTrivial at GAPROOT/lib/magma.gi:130 gap> gap> M0:=Magma(FamilyObj([1]), []);; #I RunImmediateMethods #I RunImmediateMethods gap> AsSSortedList(M0); #I RunImmediateMethods #I immediate: RepresentativeSmallest at GAPROOT/lib/coll.gi:226 #I immediate: AsList at GAPROOT/lib/domain.gi:206 #I immediate: EnumeratorSorted at GAPROOT/lib/coll.gi:379 #I immediate: GeneratorsOfDomain at GAPROOT/lib/domain.gi:181 #I immediate: Size at GAPROOT/lib/coll.gi:183 #I immediate: RepresentativeSmallest at GAPROOT/lib/coll.gi:215 #I immediate: IsFinite at GAPROOT/lib/coll.gi:148 #I immediate: IsTrivial at GAPROOT/lib/coll.gi:116 #I immediate: IsEmpty at GAPROOT/lib/coll.gi:97 #I immediate: IsNonTrivial at GAPROOT/lib/coll.gi:134 #I RunImmediateMethods #I immediate: Size at GAPROOT/lib/coll.gi:179 #I RunImmediateMethods #I immediate: RepresentativeSmallest at GAPROOT/lib/coll.gi:215 [ ]
a1b7b35
to
feb533d
Compare
Codecov Report
@@ Coverage Diff @@
## master #2393 +/- ##
=========================================
Coverage ? 73.54%
=========================================
Files ? 482
Lines ? 240853
Branches ? 0
=========================================
Hits ? 177142
Misses ? 63711
Partials ? 0
|
@ThomasBreuer this, however, breaks a function in |
This PR makes the information displayed by
TraceMethods
and byApplicableMethod
much more useful in many case, which resolves #2239. For example, consider this method forAsList
, installed incoll.gi
:With GAP 4.8, no location information is shown at all:
With GAP 4.9 / master, the location where the method (i.e., the
underlying function) was defined is shown, which already does the job in
many cases, but not here:
With this PR, we get this (note that instead of GAPROOT, the actual path
is printed)
For we immediate methods, this is what we got before this PR:
After (note how e.g. the entry for EnumeratorSorted points at
coll.gi:379, and not at the location where the method function
was defined):