by refining and extending org.aspectj.systemtest.ajc171.NewFeatures,
also considering around advice types and repeated weaving, actually
re-using the cache.
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>
- 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.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>
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:
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:
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>
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>
In AtAjAnnotationGenTests.testRuntimePointcutsReferencingCompiledPointcuts,
the classpath issues mentioned in the comments do not seem to exist
anymore, with or without the removed lib/bcel/bcel.jar.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Remove Apache Commons from 'lib' module, update remaining dependencies
Of beanutils, collections, digester and logging actually only digester
and logging are directly used in AspectJ code. Therefore, remove the
unused ones and upgrade the remaining libraries' versions to ones which
also have source JARs on Maven Central. This makes downloading sources
from GitHub and packaging separate commons.jar and commons-src.zip
artifacts superfluous. Hence, we can get rid of them completely.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Throw exception for minimal AJC runtime version violation
When running AJC, throw an AbortException(MINIMAL_JRE_VERSION) instead
of just logging an error, if the minimal JRE version requirement is
violated. Otherwise, in-process compilation would not fail due to the
skipped System.exit(-1) that was used before. In-process compilation is,
for example, relevant for AspectJ Maven Plugin, but also for non-forked
executions of Plexus AspectJ via Maven Compiler.
I am not 100% sure that AbortException is the appropriate exception
type, because it was designed for an aborted compilation process and
here compilation has not even started yet, but it seems to work fine.
Relates to #269.
Fixes #292.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
according to https://rules.sonarsource.com/java/tag/leak/RSPEC-5164/.
Now, there no longer is a thread-local stack of AroundClosure instances,
but rather a list of them, which can only grow but never shrink.
Instead, we now have a thread-local (integer) list index, for every
thread being initialised with pointing to the last element. I.e., every
thread can unwind by decrementing the index while proceeding,
independently of other threads.
A positive side effect is that this approach also works for long-lived
threads from thread pools, used by executor services. Hence, test
Bugs199Tests.testAsyncProceedNestedAroundAdviceThreadPool_gh128, which
was previously commented out, has been activated and passes, see #141.
I am not sure if this brings @AspectJ style, non-inlined, nested around
advice execution functionally on par with native ones, but at least for
current scenarios it seems to work.
Fixes #288, #141.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Add regression tests for inheritable thread-local memory leak
Leak was introduced in commit 3c80a36527, fixing #128, but introducing
#288 instead, which was the lesser of two evils, but still bad for some
users unwilling to use native AspectJ syntax for their aspects, avoiding
the problem.
Relates to #288.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
KimmingLau [Wed, 28 Feb 2024 03:50:59 +0000 (11:50 +0800)]
Improved fix for #285
1. Write SAME_BYTES to cacheMap when woven bytes are null
2. Fix TODO in SimpleCache::getAndInitialize, using Optional to help
indicate cache hit for unwoven class
3. Improve test coverage (cache miss, cache hit for unwoven class)
Relates to #285.
Co-authored-by: Alexander Kriegisch <Alexander@Kriegisch.name> Signed-off-by: KimmingLau <294001791@qq.com> Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Fix: AllTestsAspectJ171 runs correct NewFeatures suite
While fixing #285, I noticed that *.ajc171.NewFeatures was never
executed as part of the AspectJ test suite, because since 2012 falsely
*.ajc1610.NewFeatures was imported and executed a second time instead.
In addition to an outdated AspetJ weaver library in 'lib', this was one
more factor why the regression bug was never spotted.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
- Add short description to AJC usage message via
messages_aspectj.properties
- Add longer description to ajc.adoc in the AspectJ Development
Environment Guide
While researching how and what to document for AJJ option
'-xmlConfigured', I needed to find out what happens in case of multiple
XML config files. Result: They are merged. the new test case for the old
1.6.4 release verifies and documents that. It can also serve as an
example for reference in case of user questions.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Extend documentation for Antrun execution to fix javadocs
Also update the internal review ID of the OpenJDK bug I created, because
previously I had created a feature request, which has remained
unanswered. But actually, I think it is rather a regression bug, having
found out that before JDK 16 the behaviour was exactly how the feature
request describes it.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
New abstract class JavaVersionSpecificXMLBasedAjcTestCase
Replaces now obsolete base classes
- XMLBasedAjcTestCaseForJava[n]OrLater,
- XMLBasedAjcTestCaseForJava[n]Only.
The new class is parametrised with minimum and maximum Java version and
hence can replace all the other classes. This does not only apply the
DRY principle, but also makes adding tests for new Java versions less
tedious.
By chance, I also noticed missing sanity tests for Java 12, which I
added as a little drive-by benefit.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
SOURCE_INPUTS, TARGET_INPUTS, COMPLIANCE_INPUTS are now populated in a
'for' loop in a static initialiser block. I.e., adding support for a new
Java version is now as simple as incrementing field JAVA_VERSION_MAX. In
case ECJ raises the minimum supporter compiler source/target version,
field JAVA_VERSION_MIN needs to be incremented. But that should happen
less frequently.
This was done to make the 'AspectJ_JDK_Update' tasks as easy and as
little error-prone as possible.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Remove most MINOR_*, MINOR_* BCEL class file versions
in favour of Constants.ClassFileVersion.of(int, int). The few ones
retained are used from production code, the vast majority of removed
ones only from test code.
This gets rid of yet another 'AspectJ_JDK_Update' tag. One less place to
check and update with each newly supported Java version. :-)
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
The hint is meant to help AspectJ developers identify the places where
there are to-dos for releases supporting new Java versions. This is work
in progress, new tags can be added wherever necessary in the future. But
for now, the most important places should be covered:
- AJC version string
- Test infrastructure (test suites, classes and XML files)
- BCEL class file version MAJOR_*, MINOR_* constants
- AjcTask constants for compiler source, target, release
- LangUtil::is*VMOrGreater methods
- ASM and JDT Core dependency versions
- CI workflow file
- Release notes
The to-do to check the tagged places is also mentioned in RELEASE.md.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
- Remove obsolete section "Deploying the AspectJ installer to
aspectj.dev", because we are publishing releases on GitHub, attaching
installers to them.
- Add section "Publish documentation on website".
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Use Antrun plugin to eliminate javadoc version number in HTML
This is the one issue that cannot be solved regarding reproducible
builds by setting javadoc options, because that header comment is
hard-coded into the JDK with no option to avoid it. I just created an
OpenJDK issue (internal review ID 9076583, not published yet) for it,
but that does not help us today.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Add decade 2020-2029 to copyright years in Checklics
FWIW, I just wanted to know if the tests in module 'build' would still
pass. They did not find copyrights for more recent years, so I added
them here.
But I am still unsure, if we need these tests at all. Obviously, they
were not running during CI builds, i.e. they seem to be non-essential.
But chances are, they were forgotten to be added to the suite.
BTW, running the tests in module 'build' with '-Drun.build.tests=true'
is failing, too, because some old JDK classes looked up there no longer
exist in more recent JDKs.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Avoid "Last updated [timestamp]" footer on each docs page
Set Asciidoctor option 'nofooter' to avoid the annoying timestamps. It
makes updating the website difficult, because always there are "changes"
in each file. Diffing real changes in generated files for the website
before committing becomes difficult. Furthermore, the user does not have
any immediate benefit for most documents, as they change infrequently or
never (e.g. old release notes).
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
projects / aspectj.git / log