Fixes #306 and adds a few small optimisations.
Co-authored-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Performance regression test for #306, failing before the enhancement and
passing afterward.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Use aspect file/line matching for weave messages in some tests
Instead of creating dedicated ITs for #218, just use the new matching
capabilities like this for good measure:
<message kind="weave" aspectFile="MyAspect.java" aspectLine="8" .../>
Tests affected:
- Ajc165Tests::testFunkyPointcut_pr272233_2
- Bugs1920Tests::test_GitHub_214
Relates to #218.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
BcelObjectType: Add experimental code to set source file name for
reference
Experimental code leading to undesired ripple effects elsewhere,
requiring more rework which now I do not feel inclined to invest to
perfect source file string representation for post-compile binary
classes and aspects.
Relates to #218.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Add more funky pointcuts concerning 'void[]' and pointcuts matching
arrays of generic types. Remove TODO after previously committed bugfix.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
- Improvement: Also add CompilationAndWeavingContext for constructor
with causing exception
- Remove home-brew stack trace printing, just call super constructors
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
- Bugfix: Flush stream.
- Adjust format to more closely resemble JVM format. E.g., do not print
the causing exception name twice.
- Add TODO, because this whole custom stack trace printing can just go
away. The JVM format should do just fine. This commit is merely meant
to document the decision to remove the cruft in the next commit.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Make Ajc165Tests.testFunkyPointcut_pr272233_2 pass
The test needs to expect the lately introduced Xlint:arrayCannotBeVoid
warning to be thrown, because one of the pointcuts in
tests/bugs165/pr272233/Iffy2.java contains a 'void[]' return type in a
method signature.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
New Xlint warning 'arrayCannotBeVoid' when resolving 'void[]'
Because void arrays are illegal (and nonsensical), now there is a new
Xlint warning whenever World.resolve resolves a new 'void[]'. Because
in the World class we do not have any source context, no path + line
number are logged. The user only sees something like:
[warning] arrays cannot have a void type, but found 'void[]' in
pointcut [Xlint:arrayCannotBeVoid]
Then later, if due to the returned MissingResolvedTypeWithKnownSignature
type a joinpoint does not match, there is an additional
my/path/MyAspect.aj:42 [warning] advice defined in MyAspect has not
been applied [Xlint:adviceDidNotMatch]
log line, but not necessarily anywhere near the former one.
On the one hand, this is better than nothing. OTOH, comparing the
situation with no logging message other than Xlint:adviceDidNotMatch in
case of something equally illegal like 'Foo<int>' (primitive generic
type parameter), this is actually more than we have in several other
situations and might even be regarded as superfluous. In case of
multiple 'void[]' cases within a big number of aspects, the same aspect
or even the same pointcut, the user would have no clue where exactly to
search for it. He would just see multiple log messages without source
context.
One option would be to set 'arrayCannotBeVoid=ignore' in
XlintDefault.properties, so the user would have to explicitly activate
it. But IMO, this message should be visible by default.
Another option would be to find out how to defer logging the messages
until later similarly to BcelWeaver.warnOnUnmatchedAdvice and then to
bulk-print them. But in order to achieve that, the information about the
existence of any 'void[]' occurrences would have to be stored in a flag
similar to BcelAdvice.hasMatchedAtLeastOnce, bloating BcelAdvice for
that rare case. Alternatively, each advice pointcut could be
heuristically scanned for the literal substring 'void[]', logging the
Xlint message if it is found anywhere.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
UnresolvedType.signatureToName: fix '*' case for generic type '?'
In generic type lists, after a '*' in any type parameter list, sometimes
the '*' (which should be converted to '?') itself and always the
subsequent parameters would be missing from the signature:
- '[Pjava/util/Collection<*>;' yielded
'java.util.Collection<>[]', but should be
'java.util.Collection<?>[]'
- '[Pjava/util/Map<*Pjava/util/List<[Ljava/lang/Integer;>;>;' yielded
'java.util.Map<?>[]', but should be
'java.util.Map<?,java.util.List<java.lang.Integer[]>>[]'
This is now fixed.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
WildTypePattern: match generic type params correctly for array types
For array reference types, match type parameters on component type, not
on array type itself.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
WildTypePattern.toString: do not parenthesesise generic type list
It is unnecessary to represent a pattern list 'A,B,C' as '(A,B,C)'. Not
only does it look ugly in a type signature like 'org.acme.Foo<(A,B,C)>',
but also is it not valid Java syntax. While the latter might not be
strictly necessary in a String representation, it certainly is
desirable, if such representations are ever used to generate code or
@AspectJ pointcut annotations.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Method visit(WildAnnotationTypePattern, Object) used to descend into
node.getTypePattern().accept(this, data), which since commit 6585b9ef46
is unnecessary, because WildAnnotationTypePattern::traverse already
traverses its type pattern.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
SignaturePattern: Add exception for meta annotations
Upon meta annotation usage in signature patterns, lint warnings like the
following were issued during type parameter traversal:
does not match because annotation @java.lang.annotation.Inherited
has @Target{ElementType.ANNOTATION_TYPE} [Xlint:unmatchedTargetKind]
To avoid this, we now heuristically check if we are in a meta annotation
situation and, if so, permit it.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Due to the latest improvements, an error which was previously not thrown
unexpectedly according to a source code comment in test aspect
ParameterizedTypesInAnnotationPatterns.aj is now thrown for this kind
of pointcut:
staticinitialization(@(Foo || List<String>) String)
Now the compiler correctly says:
no static initialization join points for parameterized types,
use raw type instead
Relates to #221.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Fixes #211. Previously, '?' was not converted to '*' in
UnresolvedType.nameToSignature, but kept as-is. That is why - falsely -
it was necessary to handle the '?' case in UnresolvedType.forSignature
at all, reading this kind of bogus signature and creating a type for it
in TypeFactory.createTypeFromSignature. This, ironically, led to correct
JVM generic type signatures containing '*' not being handled at all.
The conversion should now work correctly both ways.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
JoinPointImpl: Remove thread-locals after usage, where possible
Avoid potential ThreadLocalMap.Entry accumulation. Entry is a
WeakReference. When JoinPointImpl objects are collected by GC, Entry
instances are still be referenced by ThreadLocalMap, which leads to
memory pressure and potentially more full GCs. So, we proactively remove
ThreadLocal<Integer> arcIndex instances when arcIndex has been
decremented back to -1 per thread. This is not perfect, because not each
thread can be expected to proceed, but it should ameliorate the
situation to some degree.
Fixes #302.
Co-authored-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Signed-off-by: KimmingLau <294001791@qq.com>
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Revert WeavingAdaptor generated class map optimisation
This was introduced in commit 8a4aa03845 of PR #278 contribution as part
of the #279 fix. The contributor thought that the generated closure
class entries were never used, but in fact AJDT class OSGiWeavingAdaptor
relies on the presence of those entries.
To the best of my present knowledge, it looks as if this change was the
root cause of https://github.com/eclipse-aspectj/ajdt/issues/57.
Therefore, I reverted it, simultaneously refactoring Iterator::remove
usage to delete entries from the map to Collection::removeIf.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
- Add some code comments and javadoc for SimpleCache::getAndInitialize
- Add TODO to migrate from using ClassLoader::defineClass to the class
definition strategy used in ClassLoaderWeavingAdaptor
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Relates to #117. In commit f986c3d183, the asm-commons dependency became
necessary to pull off the new trick to define classes in arbitrary class
loaders during LTW. The dependency was added to aspectjweaver, but not
to aspectjtools due to an oversight. As aspectjtools is meant to be a
super set of aspectjweaver, add the dependency to the assembly
descriptor.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Raise ClassLoaderWeavingAdaptor::defineClass visibility to protected
Relates to https://github.com/eclipse-aspectj/ajdt/issues/57 and it a
precondition for refactoring phase 2 of child class
OSGiWeavingAdaptor::defineClass, which can now directly call the super
methods instead of using reflection.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
The tests from Java 21 were copied to 22. Inactive ones were activated
after their features under test were fixed/implemented. Preview ones
were promotes to final ones for unnamed variables and patterns.
TODO: Add tests for new Java 22 features and maybe adjust or
amend existing feature tests, if preview or final characteristics have
changed since Java 21.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Initially, these tests made sure that an old AJC bug causing
incompatibility to ECJ when using `-proceedOnError` was fixed and there
were no regressions. See also:
https://bugs.eclipse.org/bugs/show_bug.cgi?id=102733
Now with the Java 22 changes for JEP 463 "Implicitly Declared Classes
and Instance Main Methods (Second Preview)" in JDT Core, source code is
parsed into a significantly different AST structure than before, even
when using compiler targets < 22. See also https://openjdk.org/jeps/463.
One test has been temporarily adjusted to the byte code created by
ECJ/AJC now.
TODO: Revert/adjust after this upstream bug has been
fixed:
https://github.com/eclipse-jdt/eclipse.jdt.core/issues/2205
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Add hint: "As of AspectJ 1.9.21.1, '--add-opens' is no longer necessary.
Please upgrade, if it bothers you too much."
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
The default is 2 in Asciidoctor. A higher TOC level enables developers
and users to post more precise deep-links into docs. In PDF versions, it
is also easier now to navigate from the TOC to a specific subchapter.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
If any of the old Ant builds, e.g. tests/profiling/build.xml, which
have never been mavenised, need JDT Core, they should be converted to
Maven builds and refer to it as a regular dependency. As is, the Ant
builds would not run anyway, because other dependency locations have
changed as well.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Alexander Kriegisch
Ric Harris
KimmingLau