Re: RFR: 8287834: Add SymbolLookup::or method [v3]
On 17/05/2023 02:33, John Hendrikx wrote: SymbolLookup lookUp = name -> library1.find(name) .or(() -> library2.find(name)) .or(() -> loader.find(name)); What you say is true - e.g. the fact that a lookup returns an optional can be used to chain multiple lookups as you describe. There are, however some difference - note that your example only uses "captures" symbol lookup variable names - so the full code would be more something like: ```java SymbolLookup library1 = SymbolLookup.libraryLookup("lib1", arena); SymbolLookup library2 = SymbolLookup.libraryLookup("lib2", arena); SymbolLookup loader = SymbolLookup.loaderLookup(); SymbolLookup lookUp = name -> library1.find(name) .or(() -> library2.find(name)) .or(() -> loader.find(name)); ``` Without the initial setup, we could try to write something like this: ```java SymbolLookup lookUp = name -> SymbolLookup.libraryLookup("lib1", arena).find(name) .or(() -> SymbolLookup.libraryLookup("lib2", arena).find(name)) .or(() -> SymbolLookup.loaderLookup().find(name)); ``` But this more compact version has several issues. First, it creates a new library lookup on each call to "find" (possibly more). Second, the loader in which the lookup is retrieved depends on who calls the "find" method. So, the more compact version is not "like for like" for the more verbose option above. And this is due to the fact that, in most cases, retrieving a symbol lookup has some side-effects, such as loading a library - it is *not* a purely functional process. With the new method, it becomes like this: ```java SymbolLookup lookUp =SymbolLookup.libraryLookup("lib1", arena) .or(SymbolLookup.libraryLookup("lib2", arena)) .or(SymbolLookup.loaderLookup()); ``` So, not only the new method results in more succinct code (compared to the alternatives), but it also combines symbol lookups in the "right way", so that the chain of lookups is defined, once and for all, when the composite lookup is first created. Cheers Maurizio
Re: RFR: 8287834: Add SymbolLookup::or method [v3]
On Tue, 16 May 2023 22:36:51 GMT, Maurizio Cimadamore wrote: >> This patch adds a simpler method for composing symbol lookups. It is common >> for clients to chain multiple symbol lookups together, e.g. to find a symbol >> in multiple libraries. >> >> A new instance method, namely `SymbolLookup::or` is added, which first >> searches a symbol in the first lookup, and, if that fails, proceeds to >> search the symbol in the provided lookup. >> >> We have considered alternatives to express this, such as a static factory >> `SymbolLookup::ofComposite` but settled on this because of the similarity >> with `Optional::or`. > > Maurizio Cimadamore has updated the pull request incrementally with one > additional commit since the last revision: > > Tweak javadoc Isn't it already possible to easily create a composed `SymbolLookup`? SymbolLookup lookUp = name -> library1.find(name) .or(() -> library2.find(name)) .or(() -> loader.find(name)); The proposed method may be a bit nicer, but it is sort of duplicating what `Optional::or` was intended for. - PR Comment: https://git.openjdk.org/jdk/pull/13954#issuecomment-1550560151
Re: RFR: 8287834: Add SymbolLookup::or method [v3]
> This patch adds a simpler method for composing symbol lookups. It is common > for clients to chain multiple symbol lookups together, e.g. to find a symbol > in multiple libraries. > > A new instance method, namely `SymbolLookup::or` is added, which first > searches a symbol in the first lookup, and, if that fails, proceeds to search > the symbol in the provided lookup. > > We have considered alternatives to express this, such as a static factory > `SymbolLookup::ofComposite` but settled on this because of the similarity > with `Optional::or`. Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: Tweak javadoc - Changes: - all: https://git.openjdk.org/jdk/pull/13954/files - new: https://git.openjdk.org/jdk/pull/13954/files/27e82bb6..76578973 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=13954=02 - incr: https://webrevs.openjdk.org/?repo=jdk=13954=01-02 Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/13954.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/13954/head:pull/13954 PR: https://git.openjdk.org/jdk/pull/13954