Resolve tag calls at compile time instead of through the metaclass - #16134
Merged
codeconsole merged 83 commits intoAug 23, 2026
Merged
Conversation
The closure form is deprecated and carries no callable signature, so a tag defined that way cannot be resolved when a page is compiled. This was the last closure-based tag remaining in the repository.
Discovering which tags exist required loading every tag library and reflecting over it, which is only possible once the application is running. A GSP therefore had no way to know at compile time whether a tag call would resolve. The TagLib AST transformation now records each tag library's namespace and tag names as it is compiled, writing one descriptor per class under META-INF/grails/taglibs along with a manifest naming them. Descriptors are per class so that tag libraries packaged in separate jars merge on the classpath with no build step combining them, in the manner of META-INF/services entries. Deriving tag names from the AST has to agree exactly with the runtime rules in TagMethodInvoker, since a tag recorded in the index but rejected at runtime would resolve when a page is compiled and then fail when it renders. The framework method exclusions are shared rather than duplicated, and TagLibraryIndexAgreementSpec asserts the two views match for every framework tag library. Two cases the AST view has to account for: trait application generates super-accessor bridges that are synthetic at runtime but not marked so at canonicalization, and parameters with default values expand into overloads that reflection sees but the declaration does not show.
The type checking extension answered every unresolved tag call with makeDynamic, so compileStatic on a GSP verified model fields and left tag calls exactly as dynamic as they were without it. Tag calls are now checked against the tag library index. A call into a namespace backed by a compiled tag library must name a tag that library declares, and a misspelling is reported when the page is compiled rather than surfacing as a missing method when it renders. Namespaces the index does not know, as a tag library registered at runtime or supplied by a separately compiled plugin would be, keep resolving dynamically. Namespaces contributed by compiled tag libraries no longer have to be declared through the taglibs directive, because the index already states which tags they hold.
Dispatching a tag read Method.getParameters() on every invocation to work out which parameter takes the attribute map, which takes the body, and which are bound from named attributes. That allocates a fresh Parameter array and materialises reflection metadata each time, and it showed up directly in profiles of tag-heavy pages, yet the answer is fixed for a given method. The classification is now computed once, when the tag library class is first seen, and held alongside the method. Invocation walks the precomputed plan instead of re-reading reflection metadata, and the access check is suppressed once rather than paid per call. Also corrects two disagreements between the compile-time index and runtime dispatch that the framework tag libraries did not exercise: @tag and @NotATag override the conventional signature rule at runtime and now do so when scanning the AST, and an attributes parameter has to be assignable to Map, so an untyped parameter is not a dispatchable tag and is no longer recorded as one. IndexEdgeCaseTagLib covers both directions.
The index is written per tag library class specifically so that libraries packaged in separate jars combine on the classpath without a build step merging them. That is the central claim of the format and was previously only exercised indirectly, through tag libraries that all happened to live in one module. Builds classpaths out of temporary jars and asserts that two jars contributing to one namespace merge, that distinct namespaces stay distinct, that an empty classpath yields an empty index rather than failing, and that a malformed descriptor leaves its tags unknown so they fall back to dynamic resolution.
A design review found the compile-time index and runtime dispatch disagreeing in ways the framework tag libraries never exercise. Each would let a page compile and then fail as it renders. An attributes parameter is only recognised at runtime when it is named "attrs", unless the class was compiled without parameter names, in which case any name is accepted. The scanner checked only the type, so a tag written as foo(Map options) was recorded but is not dispatchable. Whether names are retained is read from the compiler configuration and the same rule applied, with the body parameter treated the same way. TagMethodInvoker scans declared methods, so a tag inherited from a base class is not dispatchable. The scanner walked inherited methods too and is now restricted to declarations on the tag library itself. Trait methods are woven as declarations and remain visible. A namespace is read at runtime through the class hierarchy and after its initialiser has run. The scanner looked only at the class itself and treated anything other than a constant as the default namespace, filing those tags under "g". It now walks the hierarchy, and when the namespace cannot be known without running the code the tag library is left out of the index rather than filed under a guess. An unrecognised tag is now a warning rather than a compilation error. The index describes the tag libraries compiled before a page, so a tag added without rebuilding its library, or a library registered at runtime, would otherwise fail a build whose pages are correct. Setting grails.views.gsp.strictTagChecking restores the error.
When more than one tag library declares the same namespace and tag, the one registered last wins, and registration order comes from artefact scanning rather than from the classpath. TagPrecedenceSpec pins that down: the winner flips purely with registration order and carries no inherent ranking, and returnObjectForTags follows the winner rather than accumulating. The index cannot reproduce that ordering, so it no longer tries. A tag declared by two tag libraries is recorded as ambiguous and is not resolved, which leaves the choice where it is actually made. Resolving it here would risk compiling against one implementation and dispatching to another. The same tag library reaching the classpath twice, as a duplicated dependency does, names one implementation and stays resolvable. Descriptors also carry the format version they were written with, and one written by a different version is ignored rather than read under rules that may since have changed.
Whether a method is a tag was decided in two places: by reflection when an application registers its tag libraries, and over the syntax tree when the tag library index is written. Keeping the two in step was left to a test, and they had already drifted apart three times. The rules now live in TagDiscoveryRules, over a TagMethodView that a compiled method and a method being compiled each adapt to. The two sources differ in only two respects, both confined to their adapters: parameter defaults have already become overloads by the time a class is reflected on, and whether parameter names survive into the class file is a property of the compilation rather than of the method. TagDiscoveryRulesSpec compiles one matrix of method shapes and classifies each of them twice, from the tree and from the resulting class, asserting the two agree as well as asserting the expected answer. It covers the shapes that caused the earlier drift: a Map parameter not named attrs, an untyped parameter, @tag and @NotATag, a framework trait name, and a defaulted trailing parameter.
The index was written as each tag library compiled, which left it unable to describe the source set as a whole. A renamed or deleted tag library kept its descriptor, and the manifest naming it, until the build directory was cleaned, so the index went on describing tags that no longer existed. TagLibraryIndexGenerator now writes it for a whole source directory at once, and clears what was there first, so what it describes is what exists. Sources are parsed only as far as the syntax tree, never loaded or executed, which is covered by a tag library whose static initialiser would throw if it ran. Regenerating unchanged sources produces a byte-identical index. The generateTagLibraryIndex Gradle task runs it, before page compilation and ahead of the artifact being packaged, so a project depending on this one can resolve its tags. The generator reads source rather than classes, so its classpath is the compile classpath alone: including this project's own output made it wait for the compilation it exists to precede, which showed up as a circular dependency through compileAstGroovy. Two tests hold that ordering in place. The AST transformation keeps writing descriptors, which covers tag libraries compiled outside this task.
Registering a tag library asked the class what tags it declares, which walks its metaclass properties, reflects over its declared methods and scans its fields. That happens for every tag library as an application starts, and the answer was already worked out when the tag library was compiled. Registration now prefers the tags recorded in the index, and discovers them from the class only when there is no record. That keeps working unchanged for a plugin built before the index existed, for a tag library registered while an application is being developed, and for one registered by a test. A tag declared by more than one tag library is deliberately absent from the index, so a tag library holding such a tag falls back to discovery rather than registering an incomplete set.
Resolving a tag installed it onto the caller's metaclass so that later calls bypassed methodMissing, and every namespace dispatcher was built with its own ExpandoMetaClass carrying a method for each tag in the namespace. Tag dispatch was therefore a read of an initialised ExpandoMetaClass, guarded by a read-write lock that profiles of concurrent rendering showed to be the largest single contended cost, and every caller mutated its own metaclass the first time it used a tag. Both now dispatch through the tag library lookup, which is a map read. Removing the installed methods is not simply removing a cache: they carried overloads that adapted a CharSequence body into a closure and routed the call through the output capture protocol. Dispatching straight at the tag library skipped that and broke a tag called with a string body. The dynamic path therefore goes through methodMissingForTagLib, which already does both, with the flag that installs the metaclass methods turned off. NoMetaClassMutationSpec holds the property that resolving a tag writes to no metaclass.
Now that the index is generated from source before anything resolving tag calls is compiled, it describes the tag libraries of this project as well as those of its dependencies, so a tag it cannot find in a namespace it knows is a misspelling rather than a gap in what it has seen. Those are reported as compilation errors. A namespace with no compiled tag library is still left to runtime resolution, as a tag library registered while developing or supplied by a plugin built before the index existed would be, and a tag declared by two tag libraries stays ambiguous and unresolved. Setting grails.views.gsp.strictTagChecking to false turns the error back into a warning. Generating the index no longer fails when one tag library cannot be resolved ahead of compilation. FormFieldsTagLib refers to services in its own project, which by design are not on the classpath the generator runs against, and that took the whole index down with it. Sources that fail are parsed individually and those that still fail are named and skipped, leaving them to be described by the compiler as they are built.
Calling a tag reaches the tag library through invokeMethod, which leaves a dynamic call site in the caller's bytecode even when that caller is statically compiled. Once a tag has been resolved against the index there is nothing left to decide beyond which bean holds it, so the call can be an ordinary method call. CompiledTagInvocation is that call. It takes the namespace and name as arguments and ends at TagOutput.captureTagOutput, which is where the dynamic path ends too, so attribute and body handling, output capture, encoding and return-object behaviour are the same either way. TagLibNamespaceMethodDispatcher, which is how a statically compiled page reaches a tag, now goes through it. This is the target a rewritten call site needs. Rewriting the call sites themselves is not part of this commit.
Every tag library had every tag in every namespace installed onto its metaclass as it was constructed, and again for the whole application at plugin bootstrap, so that a tag library calling another tag found a method rather than falling through to methodMissing. A namespace resolved through propertyMissing was installed as a property too. None of that is needed now that tags are resolved through the tag library lookup and invoked through CompiledTagInvocation, so it is gone. Registering a tag library with the lookup is all bootstrap does. TagLibraryMetaUtils is deprecated. What remains of it is the dynamic dispatch a tag library registered at runtime still relies on, reached with metaclass installation switched off. The compile-time warning for a closure-based tag now says what the consequence is, that calls to it stay dynamic because it cannot be resolved when a page is compiled, and shows the method form to use instead.
Writing g.link(controller: 'book') reaches the tag library through propertyMissing to find the namespace and invokeMethod to find the tag, which leaves a dynamic call site in the bytecode of a tag library even when it is statically compiled. Both names are fixed in the source and the index says whether that tag exists, so the call is replaced with a call to CompiledTagInvocation. Only calls whose shape is evident from the source are rewritten: a tag takes attributes, a body, both or neither, written as literals. A call whose attributes are assembled at runtime, a namespace no compiled tag library declares, a tag declared by more than one of them, and a namespace shadowed by a field of the same name are all left to resolve as they did before. CompiledTagCallRewriterSpec renders through each of those shapes, since a rewrite that changed behaviour is the failure that matters. Behaviour alone cannot show that anything was rewritten, because the dynamic route produces the same output, so CompiledTagCallBytecodeSpec compiles a tag library and looks for the invocation in the class file, and for its absence where nothing should have been rewritten.
A review of the stack found the strict check and the explicit invocation path each breaking cases the dynamic path handled. An unrecognised tag is a warning again rather than an error. Knowing that a namespace holds some compiled tag libraries is not knowing that it holds all of them: a plugin built before the index existed contributes tags to g without a descriptor, a tag library registered while an application runs contributes more, and the index generator skips a source it cannot resolve ahead of compilation. In each case the namespace is known but incomplete, so a tag missing from it is not necessarily a misspelling. Failing the build needs a namespace able to state that it is complete, which the descriptors cannot yet do. grails.views.gsp.strictTagChecking opts in to the error. A tag declared by more than one tag library was reported as no such tag. The index deliberately leaves it unresolved so that runtime precedence decides, which the checker read as absent. It now asks whether the tag is ambiguous before reporting it. A tag body given as text threw a GroovyCastException. The dynamic path accepted text through overloads that wrapped it in a closure, and the explicit API narrowed the body to Closure, which a namespaced dispatcher call with a string body could not satisfy. The API takes the body as it is given and wraps text, as before. Registering a tag library after startup, as reloading a changed class during development and registering one from a test both do, uses the descriptor supplied rather than the one recorded when the class was compiled, which no longer describes what is being registered.
A tag library rewrote its own tag calls as it compiled, but a controller can call tags as well. It gains that from the tag library invoker trait rather than from being a tag library, so nothing rewrote its calls and they stayed dynamic. A global transformation now rewrites tag calls in any class carrying that trait, which covers controllers without naming them and without a second copy of the rules. It runs after trait injection, since whether a class can call tags is only settled once its traits are applied, and it does nothing at all when no compiled tag library is on the classpath. ControllerTagCallRewriteSpec compiles a class with the trait and one without, and looks in the class files for the invocation, since a rewritten call and a dynamic one produce the same output.
Describes how tag libraries are described when compiled and how that resolves tag calls, what is compiled into a direct invocation and what stays dynamic, how an unrecognised tag is reported and how to turn that into an error, why a closure-based tag cannot be resolved, and where the description is written and packaged. Adds the corresponding what's new entry and an upgrade note covering the two things an existing application notices: the warning for an unrecognised tag, and the warning for a closure-based tag with the method form to replace it.
The pre-compilation task scanned only grails-app/taglib, so a project keeping tag libraries elsewhere had them described as they compiled rather than beforehand, which is later than anything resolving them in the same compilation needs. The task now takes a collection of directories, defaulting to the one it scanned before, and the generator can add to an index rather than always replacing it, so several directories contribute to one index instead of each erasing the last.
Three places were still installing methods onto metaclasses, so the earlier claim that dispatching a tag writes to none of them was wider than what had actually been done. A page had methodMissing installed onto its metaclass as it compiled, along with a method for every tag and a property for every namespace. GroovyPage declares methodMissing itself now and already resolved a namespace through getProperty, so a page reaches the same tags without any of those writes. The template namespace installed a method for each template name the first time it was used. Rendering goes through the render tag either way, so the name is resolved rather than installed. The unit testing support keeps installing tag methods, deliberately. Tests call tag methods directly, and the installed methods substitute an empty body for a missing one, so tagLib.someTag(attrs, null) works. Removing it broke twelve FormTagLibTests cases that rely on that calling convention. A running application does not depend on it. NoMetaClassMutationSpec now covers the template namespace and the page, alongside the namespace dispatcher it already covered.
The index said only that a tag existed, which is enough to tell a misspelling from a real tag but not enough to decide whether a call to it can be bound. A tag defined as a Closure field carries no signature, so a call to it cannot become a direct invocation, and nothing in the index said which tags those were. Each tag is now recorded with its kind, and a call is only compiled into a direct invocation when the tag is a method. A closure-based tag stays known, so it is never reported as a misspelling, and stays dynamically dispatched. This showed up immediately: g.link is a Closure field, so calls to it are correctly left alone. The descriptor format is version 2 as a result. A descriptor written by another version is ignored rather than read under the wrong rules, and a kind that cannot be recognised is treated as the dynamic one so that a newer descriptor can never cause a call to be bound wrongly.
A namespace is not declared anywhere: it is reached because nothing else answers to the name. The rewriter took any receiver that was not this or super as a namespace, checking only for a field of that name on the class itself, so a local variable, a parameter, an inherited field or a getter-only property called g had calls on it rewritten into tag invocations. The object the author wrote was then never called, and the code still compiled, which is the worst way for this to go wrong. A receiver that resolves to anything - a local, a parameter, a field, a property - is that thing, and the field and property checks now walk the hierarchy and consider getters. Rewriting is also confined to methods declared by the class being transformed. getMethods() reaches inherited methods, whose bodies belong to the class that declared them, so a subclass able to call tags could otherwise change a superclass that cannot. TagCallShadowingSpec covers a local, a parameter, a typed local, a field, an inherited method, and the unshadowed case that must still be rewritten.
The index a project generates from its own sources reached page compilation and the packaged artifact, but not the compilation of its own controllers and tag libraries. Those could resolve tags from dependencies while a call to a tag declared in the same project stayed dynamic, which is not what the documentation described. The generated directory now joins the compile classpath, and compileGroovy waits for it. It goes onto the classpath rather than into the source set output, which would make the index wait for the compilation it exists to precede. The documentation is also narrowed to what is actually rewritten. Expressions in a GSP page are checked against the descriptions but are not rewritten: a page selects the tag by name as it renders, through the namespace dispatcher, which no longer touches a metaclass but is still a runtime choice. An unqualified call such as message(code: 'x') is likewise left alone, since whether that name is a tag or a method of the calling class is decided where it is called. The examples now use a method-based tag, since the closure-based g.link they used is one of the calls that is deliberately not rewritten.
Groovy reads a property from getX() and, when the return type is boolean, from isX() as well. Only the first was checked, so a class declaring boolean isG() had this.g treated as a tag library namespace and calls on it rewritten, sending them to a tag library instead of the property the author wrote. Both forms now claim the name, with the isX form requiring a boolean return type as Groovy does. TagCallShadowingSpec covers each getter form and an inherited getter. Also proves the resolution the previous commit exists to enable. Generating an index from a tag library source, putting it on a compile classpath and compiling a controller that calls that namespace shows the call becoming an invocation, and shows it staying dynamic without the index. The build wiring is asserted separately; what was missing was evidence that the wiring is sufficient for the compiler to resolve the call.
Reading walks every jar on the classpath. A compiler that consulted the index for each source file walked it once per file, while the one caller that did cache it held the result in a static field, which carried one project's tag libraries into the next compilation in the same Gradle daemon and read them from the wrong class loader. It is read once per class loader instead, which is once per compilation and no longer. Asking what a single tag library declares is answered from its own descriptor. It used to be answered by scanning every namespace and then discarding the answer entirely if any namespace anywhere held a tag two libraries declared, so one overridden tag left every tag library undescribed. A tag two libraries declare is now reported as known even though it cannot say which of them will answer to it, so that it is never mistaken for a misspelling, and the settings a build states about its tag libraries are read alongside the descriptors.
Registration preferred the tags recorded when a tag library was compiled over the tags the class has, to save discovering them by reflection as the application starts. It saved nothing: the tag library class is constructed before it is registered, and constructing it already reads every tag by reflection and through the metaclass. What it did add was a way for a descriptor left behind by an earlier build to decide what a running application believes a tag library declares. Reflection is authoritative at runtime; the index describes what was true when the tag library was compiled and is used where that is the question being asked.
A Closure tag declared on a base class is registered at runtime, which walks the superclass chain for Closure-typed fields, but was missing from the index, which read declared fields only. The namespace still counted as completely described, so under strictTags a call to a working tag failed the build, and without it the call silently stayed dynamic. The rules already had one statement of whether a method is a tag, so the two sides could not disagree about that. They had two statements of which members to ask about and how far up the hierarchy, which is where they did disagree. Give enumeration the same treatment: a TagLibraryView over a syntax tree or a compiled class, one walk in TagDiscoveryRules, and a spec asserting the two views produce the same set. The walk also settles two smaller differences the same way the runtime does: a field typed as a subclass of Closure is a tag, and a name declared both as a closure and as a method is the closure.
Three things sbglasius found, all where the compiled path and the dynamic one disagreed about what a call means. The generator described any class named *TagLib that the compilation produced, which includes the collaborators the resolver adds to read a type. A helper under src/main/groovy would be filed as a tag library of the default namespace, making its methods g tags that either collide with real ones, silently disabling rewriting for that name, or resolve to a tag that does not exist when the call runs. Only the sources the generator was pointed at are described now. A tag takes attributes, a body, or both. Any other argument list was reduced to a call with neither, silently dropping what was written, so a name that is both a tag foo(Map) and a helper foo(String, String) ran the tag with nothing where it used to reach the helper. Dynamic dispatch now leaves such a call to the method lookup, and the rewriting declines to compile a shape the invocation cannot account for. Test source sets are matched as they are created rather than looked up on the groovy plugin being applied, since integrationTest is registered later and was being skipped without a word - the gap that wiring exists to close.
jdaugherty
reviewed
Aug 19, 2026
jdaugherty
left a comment
jdaugherty
left a comment
Contributor
There was a problem hiding this comment.
Re-review of the fix round
I checked out the updated branch and went through all twelve commits against each item from my last review. Every item is addressed: the reserved-name seeding, the per-site AST nodes, the declared transform order with its spec, the dispatch-semantics spec and upgrade note, the index API trims and format reset, the pinned constants on both sides of the module boundary, the configuration-cache and test-classpath coverage in the wiring spec, the encoding convention, the manifest locking (the 29-of-32 loss reproduction was worth having), and the docs. Verification was targeted rather than a full suite run: I re-read each fix in the code and reproduced locally where a claim needed it.
I also went through sbglasius's five comments and can confirm all five against the code. The two dispatch ones I verified against the merge-base as well — the old trait path really did reach a real overload through invokeMethod, and methodMissingForTagLib itself is unchanged by this PR, so the regression is purely which call sites now route into it. The source-set one holds as an ordering fragility: with the stock plugin order grails-app registers integrationTest before grails-gsp configures, so it bites when grails-gsp is applied first or without the app plugin — configureEach is still the right fix. One correction on the generator one is in a reply there.
Two new items below, both reproduced:
- Abstract classes are described by the generator but can never be registered at runtime — details on
isTagLibrary. - The
:grails-taglibbuild prints a javac deprecation note again, introduced by the manifest-sibling resolution — details onresolveSibling.
Contributor
A namespaced call names the tag library it means. A bare name is a tag only when nothing nearer answers to it, and what answers to it is not fully visible when compiling: a method Groovy gives every object, a delegate an enclosing closure is handed when it runs, an overload the tag library also declares. Each of those has been a bug in this branch, found one at a time and fixed by adding another exclusion, which is a sign the rule was wrong rather than that the exclusions were incomplete. So unqualified rewriting is now off unless a build sets grails.compileStatic.unqualifiedTagCalls. Namespaced calls, markup tags and statically compiled page expressions are unaffected, which is where the measured benefit came from. The exclusions stay: turning it on widens which calls are considered, not which names may be captured.
Every other assertion of this compiles a source in isolation, which shows the transform works but not that a project reaches it: the index has to be generated, packaged, put on the compile classpath and read, and the rewriting has to run after the trait that lets the class call tags. The spec that did cover the convention path drove it by writing a source into a temporary grails-app/controllers directory, and passed on macOS while failing on Linux and Windows - recognising a controller by its location depends on where the compilation happens, not on what is being compiled. Reading the class file a real build produced has no such dependence, so this answers the same question wherever CI runs it.
Kind was written into every descriptor as name:KIND, parsed back out and exposed as isBindable, and nothing ever asked. A closure tag and a method tag are dispatched the same way - by name, when the call runs - so no decision turned on it, and the javadoc claiming it decided whether a call could be resolved was simply wrong. Dropping it takes the encoding out of the format, the enum and the accessor out of the API, and the precedence rule out of the walk, which now just collects names. FORMAT_VERSION is what makes this reversible: the distinction can come back when something needs it.
Strict checking asked whether a tag was in the index, and the index holds what the tag libraries on the classpath described. For a namespace this project declares that is the whole answer. For any other it is not: a plugin built before descriptors existed contributes tags to g without one, as does one declaring its tag libraries by convention without the GSP Gradle plugin, and a tag library registered at runtime contributes more. Reporting a tag missing from such a namespace failed builds over correct code, which made strictTags unusable for g - the namespace it would matter most for. The generator already knows which namespaces it described, so the task records them beside the settings, which are not packaged, and reporting is limited to those. strictTags now catches a misspelling of your own tags in your own namespaces and never complains about a plugin's.
An abstract class kept beside the tag libraries that share it was described, with its methods filed under the default namespace. Artefact handling never registers one, and a subclass does not inherit its methods as tags, so those tags existed nowhere: a call to one compiled into an invocation that throws when it runs, and a misspelling matching such a name stopped being reported. Skip abstract class nodes in the generator and in the self-describing path, which covers traits and interfaces too. Also suppress the URL constructor deprecation deliberately rather than leave the note the last round removed: URI.resolve cannot resolve a relative name against an opaque jar: URI, so the constructor stays.
new URL(URL, String) is deprecated since JDK 20, so compiling this module printed a deprecation note. The constructor is nonetheless the tool that works: a manifest inside a jar is addressed by an opaque jar: URI, which URI.resolve cannot resolve a relative name against, and round-tripping the URL through URI breaks on characters ClassLoader.getResources does not encode. Suppress it deliberately and record the reason, so the module compiles quietly without the reason being lost with it.
…at/taglib-compile-time-index-8.0.x # Conflicts: # grails-doc/src/en/guide/introduction/whatsNew.adoc # grails-doc/src/en/guide/upgrading/upgrading80x.adoc
The rewrite decides whether a class can call tags by looking for the trait that gives it the ability. Ordering that after trait injection by priority holds only among transforms that declare one, and a trait can arrive from a local transform, which is applied after every global transform has run. A controller compiled from the conventional directory was read after it carried the trait; the same source compiled from elsewhere was read before. Which of the two happened for a given class also varied by platform, so the rewrite was applied on macOS and skipped on Linux for the same controller. Reading the class in a later phase makes the question decidable: every trait has been applied by then, whichever transform supplied it. The priority stays as a second guarantee for anything ordered within the phase. This also removes the limitation the rewrite carried, that a controller declared by annotation outside grails-app/controllers kept the dispatched call rather than the direct one. Its test now pins the rewrite instead of the limitation, and a new one pins the phase the correctness now rests on.
The case was asked for in review and dropped as platform-dependent, on the reading that recognising a controller by its location turns on where the compilation happens. That reading was wrong: the trait assertion holds on every platform, so the injector does recognise a controller compiled into a temporary directory. What varied was the rewrite, for the reason the previous commit fixed. This is the shape that carried the bug, so it is worth a test that runs in a second rather than only a functional one that needs a whole build.
The index describes what a tag library declares when it is compiled, which is not what a running application registers: a plugin can be excluded, a tag library can be named in nonEnhancedTagLibClasses, and a unit test can mock some tag libraries and not others. A call resolved against the index reached TagOutput directly and raised GrailsTagException there, where the same call dispatched dynamically raised MissingMethodException. Code catching that around a tag call, or probing with respondsTo, saw the difference. An unregistered tag now goes back through the namespace dispatcher, which is the dispatch the call would have had. That reports what it always reported, including the type it names, rather than a second copy of the rule here.
jdaugherty
approved these changes
Aug 21, 2026
jdaugherty
left a comment
jdaugherty
left a comment
Contributor
There was a problem hiding this comment.
I'm happy to see this change and I'm hoping we've gotten most of the edge cases with this implementation. This will be key to later optimizations and it's landing a full major release early. I'm going to approve with some minor comments left. I've also asked @davydotcom to take a look given his history of working in this area.
| A tag called *with* its namespace, and any tag called from a GSP, already captured, so only an | ||
| unqualified call from a controller to a method-declared tag changes. | ||
|
|
||
| ===== A Tag the Runtime Cannot Resolve Reports a Different Exception |
Contributor
There was a problem hiding this comment.
This section documents the behaviour the last commit removed. CompiledTagInvocation.invoke now hands a tag the running application has not registered back to the namespace dispatcher, which ends in MissingMethodException — exactly the cases this section lists (excluded plugin, nonEnhancedTagLibClasses, a unit test mocking only some tag libraries), and the method's own javadoc says resolving the call must not turn the exception into something else.
The advice here — switch catch (MissingMethodException) to catch (GrailsTagException) — is now wrong; following it would break the very fallback the change preserved. GrailsTagException still arises when a page has no tag library lookup at all, but that is a different case and not what this section describes.
Suggest deleting the section, or shrinking it to a sentence stating that a resolved call reports an unregistered tag exactly as the dynamic path did, so respondsTo probes and MissingMethodException handlers keep working.
Contributor
Author
There was a problem hiding this comment.
Deleted in c2c45fb. There is no upgrade note left to write: a resolved call reports an unregistered tag exactly as the dynamic path did, so nothing changes for a caller.
| Object body = null; | ||
| // Deliberately the same shapes, in the same order, as the dynamic dispatch in | ||
| // TagLibraryMetaUtils.methodMissingForTagLib, including its treatment of argument lists that | ||
| // match none of them: a call that produced an empty invocation there must produce one here. |
Contributor
There was a problem hiding this comment.
This claim is no longer true, and the behaviour it justifies now diverges from the path it cites. methodMissingForTagLib no longer reduces an unmatched argument list to an empty invocation: matchesTagShape declines it, and the call falls through to the overload lookup and then to MissingMethodException. Here an unmatched list still becomes attrs = [:], body = null, and runs the tag with nothing.
The rewriter's forwardableShape means no bytecode this branch emits can reach the default case any more, but invokeArguments/invokeArgumentsInContext are public entry points. Either make the unmatched shapes match the new dynamic behaviour — decline and dispatch dynamically, the way an unregistered tag already is — or rewrite the comment to say the empty invocation survives only for direct callers of the public API, and why that is acceptable.
Contributor
Author
There was a problem hiding this comment.
Confirmed and fixed in c2c45fb — I chose the first option. Two arguments whose first is not a Map, or three or more, now go back through the namespace dispatcher, which is where an unregistered tag already goes, so a shape a tag cannot take resolves as it would have unresolved. Two spec rows pin it.
You are right that no bytecode this branch emits reaches it, but leaving public entry points running a tag with nothing where dispatch would have found an overload was the wrong half to keep.
| * | ||
| * <p>It also covers ground a synthetic compilation cannot. An earlier spec drove the convention path | ||
| * by writing a source into a temporary {@code grails-app/controllers} directory; it passed on macOS | ||
| * and failed on Linux and Windows, because recognising a controller by its location depends on where |
Contributor
There was a problem hiding this comment.
This paragraph records the explanation that turned out to be wrong. The re-added convention case in ControllerTagCallRewriteSpec passes on macOS, Linux and Windows, and its trait assertion always held on both platforms — recognising a controller by its location does not depend on where the compilation happens. What varied was the rewrite, because at CANONICALIZATION the transform read the class before a locally-arriving trait had been applied.
The spec still earns its keep as the whole-build check — index generated, packaged, on the compile classpath, transform applied — but this second paragraph should tell the true story rather than the one the fix disproved.
Contributor
Author
There was a problem hiding this comment.
Rewritten in 79a69b3. It now says the rewrite read the class before a locally-arriving trait had been applied, and that this spec is what reported it, rather than the location explanation the fix disproved.
| * @param view the tag library, from a syntax tree or from a compiled class | ||
| * @return every tag name the library declares | ||
| */ | ||
| public static Set<String> findTags(TagLibraryView view) { |
Contributor
There was a problem hiding this comment.
findTags was inserted between isTagMethod's javadoc and isTagMethod itself, so the @param method the method to classify block now dangles above this method's own javadoc and isTagMethod is left undocumented.
The same slip happened twice more in this round: TagLibraryIndex.isStrict's javadoc (ending @return true when the build set grails.compileStatic.strictTags) now sits stranded above rewritesUnqualifiedCalls, and GrailsCompileStaticOptions.strictTags's doc (ending @since 8.0) sits above the new unqualifiedTagCalls property. Same fix in each file: place the new member after the one whose javadoc it split, or move the stranded block back onto its member.
Contributor
Author
There was a problem hiding this comment.
All three fixed in 79a69b3 — isTagMethod, TagLibraryIndex.isStrict and GrailsCompileStaticOptions.strictTags each have their block back. Same slip as the two earlier ones; worth me checking for it deliberately rather than one report at a time.
| when: | ||
| TagLibraryIndexGenerator.generate(sources.toFile(), output.toFile(), true, 'UTF-8') | ||
|
|
||
| then: 'the closure form is marked so that callers keep dispatching it dynamically' |
Contributor
There was a problem hiding this comment.
The label still says the closure form is "marked", but the marking is what this round removed — the assertion now checks a plain name list. While here: the new a class pulled in only to resolve a type is not described method is missing the blank line separating it from the helper above it.
Contributor
Author
There was a problem hiding this comment.
Both fixed. The label now says both forms are described by name, and the blank line is in.
| * @param args the arguments the call was made with | ||
| * @return true when the call can be treated as a tag invocation | ||
| */ | ||
| private static boolean matchesTagShape(Object[] args) { |
Contributor
There was a problem hiding this comment.
Non-blocking observation. The gate closes the shapes a tag cannot take, but the overlapping shapes still prefer the tag over a real overload the old call site reached. One CharSequence argument is a valid tag body, so format('x') beside def format(String) takes the tag branch where tagLibrary.invokeMethod used to find the overload; the same holds for no arguments beside a zero-argument helper, and for (Map, anything) beside a (Map, List) helper.
I don't think there is a better rule — those shapes are legitimate tag calls, and preferring any matching overload would dispatch the tag's own (Map) method without output capture — but the choice deserves to be pinned: a spec row for the one-argument overload case, and a sentence here saying overlapping shapes deliberately resolve to the tag, matching how a GSP has always dispatched them.
Contributor
Author
There was a problem hiding this comment.
Pinned as you suggested, in 79a69b3. matchesTagShape now states that overlapping shapes resolve to the tag deliberately, with your reasoning for why preferring the overload would be worse, and TagLibraryInvokerDispatchSpec has a row for the one-argument case — format(String) beside the tag, asserting the tag runs and its output is captured.
The dynamic path stopped reducing an unmatched argument list to an empty invocation when the shape gate went in: it declines those and resolves the name as an ordinary method instead, finding an overload or reporting a missing one. The resolved path still ran the tag with no attributes and no body, so two arguments whose first is not a Map, or three or more, reached the tag where dispatching them would not have. No bytecode this branch emits can produce such a call, but these are public entry points. They go back through the namespace dispatcher now, which is where an unregistered tag already went, so a shape a tag cannot take resolves as it would have unresolved. Also drops the upgrade section describing the exception change, which the previous commit removed: a resolved call reports an unregistered tag exactly as the dynamic path did, so there is nothing left to upgrade.
Each was split by a member added directly beneath it, leaving the block above the newcomer and the member it documented with none: isTagMethod, isStrict and strictTags. Also states that overlapping argument shapes resolve to the tag rather than to an overload, with a spec row for the one-argument case, and corrects two test labels that described behaviour since changed.
…at/taglib-compile-time-index-8.0.x # Conflicts: # grails-doc/src/en/guide/upgrading/upgrading80x.adoc
…dropped The merge already on the branch resolved the upgrade guide by keeping this branch's section and discarding the two upstream added, and took this branch's side of every other file upstream had touched -- 43 of them, including the CAS test configuration and the source checksum work in GroovyPageParser. Both sides are kept here, with this branch's upgrade section renumbered to 50 so that the two upstream added keep 48 and 49.
✅ All tests passed ✅Test SummaryCI - Groovy Joint Validation Build / Build Grails with Groovy snapshot (shard 1) > :grails-data-mongodb-core:test
🏷️ Commit: df895a7 Learn more about TestLens at testlens.app/docs. |
codeconsole
added a commit
to codeconsole/grails-core
that referenced
this pull request
Aug 23, 2026
Three files were touched by both. The type checking extension gains the operators this branch reports on beside the tag library index that branch reads, and the namespaces each seeds are added to the same set rather than one replacing the other. GrailsCompileStaticOptions keeps this branch's artefact properties without a convention, which is what lets an explicit value be held back from `all`, and adds that branch's three tag settings. GroovyPagePlugin keeps both sets of methods. With that branch merged, a tag library on the compile classpath no longer needs its namespace declared: the index states which namespaces exist. The guide says so, and says what the directive and the setting are still for -- a namespace filled in while the application runs, which nothing describes when the page is compiled.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Tag libraries are described as they are compiled, and that description resolves tag calls in code compiled afterwards. A call whose namespace and tag are known becomes a direct invocation instead of being dispatched through the metaclass, and nothing is installed onto a metaclass to make dispatch work.
Defining tags
Calling tags
In a tag library or a controller:
A call that names its namespace is compiled the same way inside a closure — a tag body, a
withFormatblock, anything taking a block — as outside one, as is one in a constructor or a field initialiser. A call written without a namespace inside a closure is not: a closure is handed a delegate when it runs, and a name the delegate answers to is the delegate's rather than a tag's.request.withFormat { form multipartForm { } }is the case that settles it —formthere is a format in a DSL, not theg:formtag.The tag is still selected by name when the call runs, through the same lookup dynamic dispatch uses. A tag library that overrides another, one registered while the application is running, and the order tag libraries are registered in all decide the outcome exactly as before. Nothing is bound to a particular tag library class, so a tag declared by more than one of them, and a tag declared as a
Closurefield, are compiled the same way.A call written without a namespace is not compiled unless the build asks for it with
grails { compileStatic { unqualifiedTagCalls = true } }. Whether a bare name is a tag depends on what else answers to it, and not all of that is visible when compiling — a method Groovy gives every object, a delegate an enclosing closure is handed, an overload the tag library also declares — so by default such a call is dispatched exactly as before.The attributes and body are passed straight through where the call says what they are; where it does not — a map held in a variable, a single value the tag reads under its own name — the arguments are forwarded as written and adapted by the same rules dynamic dispatch applies.
Tags in pages
A page resolves a name against the model it was rendered with before it reaches a tag library, and that model is not known when the page compiles. A page therefore keeps resolving its tags as it always has, unless it declares
compileStatic:Declaring it reserves the namespace names for tag libraries there.
grails.views.gsp.compileStaticapplies it to every page. A tag written as markup,<g:createLink controller="book"/>, already compiles into a direct call and is unchanged.Checking tags
By default nothing is reported: a tag no compiled tag library declares is left to resolve at runtime, because a namespace can legitimately hold tag libraries carrying no description. An application whose tag libraries are all described can ask for an error instead:
grails { compileStatic { strictTags = true dynamicTagNamespaces = ['legacy'] // namespaces filled in while the application runs } }Strict checking applies to the namespaces this project's own tag libraries declare, which are the only ones whose contents are knowable when compiling. Every other namespace is left alone,
gincluded: a plugin built before descriptors existed contributes tags to it without one, so a missing tag there is as likely to be a plugin's as a mistake.dynamicTagNamespacesturns compile-time resolution off for a namespace completely — calls into it are never rewritten, never reported, and dispatched exactly as before.Strict checking applies where the source says a call is a tag: one naming its namespace, and one written as markup. A call written without a namespace is never checked, and a namespaced expression in a page is checked only where the page declares
compileStatic.Deprecation
Defining a tag as a
Closurefield warns at compile time. It still works and is called the same way; the form is deprecated because a closure carries no signature, so nothing about the call can be checked:Why
Profiling a running application attributed roughly 25% of samples on a tag-heavy page to reflective and metaclass tag dispatch, and about 10% to
ExpandoMetaClassread-lock contention.Every caller used to mutate its own
ExpandoMetaClassthe first time it used a tag; every namespace dispatcher was built with a metaclass carrying a method per tag; and plugin bootstrap installed every tag onto every tag library. None of that remains.Measured on a page performing 400 tag invocations, 8 concurrent, 105k warmup requests, same publish flow both sides:
8.0.xCompiling the calls is worth this much again on top, measured with metaclass removal present on both sides and only the rewriting varying:
Where the description comes from
Under the Grails Gradle plugin the index is written twice, because the two things reading it need different guarantees.
generateTagLibraryIndexruns before compilation, so a call to a tag the project itself declares resolves as it compiles. Reading from source it cannot describe everything — a tag library referring to a type written in Java, or generated by the build, is left out — so what it missed is recorded, and nothing in an incompletely described namespace is ever reported. It is never packaged.packageTagLibraryIndexruns afterwards with the project's own classes on the classpath, where every tag library resolves. That index is the one pages compile against, the one packaged, and the one a project depending on this one reads. Each run replaces it, so a renamed or deleted tag library cannot survive.A build that does not write the index — a plain
groovyc, or a build without the Grails Gradle plugin — has each tag library annotated@TagLibdescribe itself as it compiles. That fallback does not reach a tag library declared by convention: an unannotated class undergrails-app/taglibis recognised as an artefact later in the compilation than the descriptor is written, so without the Gradle plugin it contributes none. A tag with no description is dispatched dynamically, so nothing breaks; it simply does not take the faster path.What is not rewritten
dynamicTagNamespacesgis that thingunqualifiedTagCallscompileStatic<g:set>with,each,printand the rest ofDefaultGroovyMethods, plus any extension module on the compiling classpath. Those are real methods on every receiver, so a tag of the same name must not capture the callLimitations
compileStaticpage. That is what declaring it means there. A page that has not declared it is unaffected.doWithDynamicMethods, loses to a tag of the same name when the call is written without a namespace. Declare the method on the class, name the namespace indynamicTagNamespaces, or call the tag with its namespace.tagLib.someTag(attrs, null)works. A running application does not depend on this.DefaultGrailsTagLibClassis built from. Three differences from the code it replaces:equals/hashCode/toStringand theGroovyObjectmembers are excluded by name rather than by full signature; a zero-argumentis*method is an accessor regardless of return type; and a name containing$is excluded. None is reachable by a tag that would otherwise have been discovered — the shape check rejects all three anyway — and each is pinned inTagDiscoveryRulesSpecthrough both the tree and the compiled class.