Community
Participate
Working Groups
AJDT 1.4.0.20060512075430 for Eclipse 3.2RC3 AspectJ version: 1.5.2.20060511130959 - Create the Tracing Example and activate the "tracelib.ajproperties" config - Make sure the XLint for "No guard for lazy tjp" is set to "Warning" - Open AbstractTrace.aj - there should be lots of "no guard" warnings - Now type a space somewhere in the file, and then delete the space, so that the contents of the file is *exactly* the same as before - Save the file to trigger an incremental build - The "no guard" warnings disappear (both markers and problems view entries) The warnings reappear after a full build, and after an incremental build where a change has been made to the file (even just a whitespace change)
Created attachment 42245 [details] failing testcase Apply this patch to the tests project.
This seems to be only a problem with the "no guard for lazy tjp" xlint warning. Following the same steps with the "swallowed exception in catch block" xlint option set to warning does not result in this disappearing after incremental builds.
"Swallowed Exception in catch block" is dealt with slightly differently than the other xlint options as its passed to the compiler, however, the invalidAbsoluteTypeName xlint warning also behaves as expected and does not disappear after an incremental build. This is because the warning has been marked against the compilationResult and we ask the compilationResult for its problems and then call TaskListManager.addSourcelineTask(IMessage) for each one. The bug is also a regression since 1.5.0....possibly due to the changes which were put in after 1.5.0 to do with AjState (see bug 129163). I believe (I can't verify this as I can't get hold of a dev AJDT environment with an AspectJ 1.5.0 to step through the debugger) this is because what we used to do was weave everything if there was a change to an aspect (due to the implementation, or lack thereof, of various equals methods). Consequently the advice was rewoven and the lazyTjp warning was added again. Post these changes we now only reweave the bits we really need to. Consequently, the advice isn't rewoven and the lazyTjp warning isn't added. The reason that other xlint warnings appear to stay are that either they're marked against another resource (and so aren't affected by the incremental compile), or making a change forces a reweave of that piece of advice, and/or it’s a warning against the compilationResult and this is reported separately. Note also that the noGuardForLazyTjp warning isn't marked against the CompilationResult, instead it is handled by the WeaverMessageHandler. Other xlint warnings are marked against the CompilationResult. The reason the noGuardForLazyTjp warning comes back after full builds or if a change (even whitespace change) has been made is the same as the reason I believe is behind the regression, namely that in the case of a whitespace change we decide that things have changed enough so force a full build. This causes the world to be rewoven and the warning is recorded. In the case of a whitespace change the comparing of the sets of shadowmungers in CrosscuttingMembers.replaceWith() returns that they are not equal whereas in the case of creating a change and then reversing it before saving this comparison returns that they are equal. This, however, is the topic for incremental work and not a part of this bug. As it stands, we're behaving as you would expect......ultimately, by making a change and reversing it we haven't actually made a change. We correctly notice that there hasn't been a change and so do not decide to reweave the world. The bug here is that this particular xlint warning isn't behaving correctly (or we're not recording it correctly) to cope with this behaviour.
Created attachment 47208 [details] creating new IMessage subclass: LintMessage The first of several patches which together make up the fix and tests for this bug and bug 141564. This zip file contains 3 patches: * pr141556-ajde.txt - apply to the ajde project * pr141556-tests.txt - apply to the tests project * pr141556-weaver.txt - apply to the weaver project These patches create a new IMessage class called LintMessage which knows it's lint kind as well as whether or not its the noGuardForLazyTjp xlint message.
Created attachment 47219 [details] adding ignore(IMessage.Kind) to IMessageHandler This zip file contains the following patches: * pr141556-ajdt.txt: apply to the org.aspectj.ajdt.core project * pr141556-bridge.txt: apply to the bridge project * pr141556-client.txt: apply to the testing-client project * pr141556-loadtime.txt: apply to the loadtime project * pr141556-taskdefs.txt: apply to the taskdefs project * pr141556-testing.txt: apply to the testing project * pr141556-tests.txt: apply to the tests project * pr141556-weaver.txt: apply to the weaver project These patches add an ignore(IMessage.Kind) method to IMessageHandler and the consequent required implementations. This currently removes the need for casting in BuildArgParser.populateBuildConfig(..). However, also enables us to set correctly whether or not weaveinfo messages are required when we refactor CompilerAdapter.MessageHandlerAdpater (which is part of the fix for this bug).
I am about to commit the test code from #1 and #4 - and from #4 the change to introduce a LintMessage type (because that seems generally useful) At the moment I don't quite understand how the patches in comment #5 will take us towards fixing the bug reported here ? As they start talking about weaveinfo messages. Are these patches more to do with bug 141564 ?
is this fixed now with those Lintmessage/etc changes we put in recently? the test in MPITests passes fine.
I can no longer reproduce this.
This is fixed since we moved the noGuardForLazyTjp xlint message to be attached to the CompilationResult. There was one final bit of work that was going to be done as part of this bug. That was to move the MessageHandlerAdapter out of CompilerAdapter and into a class in its own right with a better name that said what it did (namely that this is the handler used by AJDT). The result of this was to then use the new IMessageHandler.dontIgnore method to work out whether or not we are ignoring weaveinfo messages rather than the unnecessary casting in BuildArgParser.populateBuildConfig(..). I believe this work should still be done as it makes the situation a little less confusing.
i knew there was something else - we should still do that.
Created attachment 48691 [details] adding method IMessagHandler.ignore(IMessage.Kind) The attached zip adds an IMessageHandler.ignore(IMessage.Kind) method and the consequent implementations. In particular, it removes the need for instance checking in BuildArgParser.populateBuildConfig(..) and removes the get/set showInfoMessages methods in CompilerAdapter. These were used within the ReweavableTestcase, BuildCancellingTestcase and ShowWeaveMessageTestcase to set this value to filter IMessage.INFO messages. By setting MessageHandlerAdapter to ignore info messages by default (which is the same behaviour as before since showInfoMessages was by default set to false) we are able to use the ignore/dontIgnore methods instead. The only testcase where we need to do this is with the ReweavableTestcase. BuildCancellingTestcase set this value to false anyway (which is redundant) and even though ShowWeaveMessagesTestcase set the value to true it filtered out all messages that weren’t WEAVEINFO messages in the comparison. This zip contains the following patches: *pr141556-testing-ignore.txt: apply to the testing project *pr141556-tests-ignore.txt: apply to the tests project *pr141556-weaver-ignore.txt: apply to the weaver project *pr141556-ajde-ignore.txt: apply to the ajde project *pr141556-ajdt-ignore.txt: apply to the org.aspectj.ajdt.core project *pr141556-bridge-ignore.txt: apply to the bridge project *pr141556-client-ignore.txt: apply to the testing-client project *pr141556-loadtime-ignore.txt: apply to the loadtime project *pr141556-taskdefs-ignore.txt: apply to the taskdefs project These patches replace the one in comment #5 as they work with the latest code in HEAD and do all the refactoring that adding the ignore method makes possible. After this patch has been applied there is one final piece of work that needs to be done and that is pulling out MessageHandlerAdapter into its own class.
that is a lot of change ... committed patches from #11, wonder what Matthew will make of it.
Created attachment 48700 [details] refactoring MessageHandlerAdpater The attached zip contains two patches: *pr141556-ajde-refactor.txt: apply to the ajde project *pr141556-tests-refactor.txt: apply to the tests project Together these patches fix the final part of this bug – refactoring of MessageHandlerAdapter into its own class AjdeMessageHandler which implements IMessageHandler directly rather than extending MessageHandler. This is because MessageHandler implements IMessageHolder and the implementation of the IMessageHolder methods all use the list of messages it stores. Since we’re no longer storing any messages (see bug 141564) we don’t need these extra methods.
comment #13 patches in.
Closing as aj dev build is available with this fix.
