From fbe3349d60d9e1880b2b34bc6b32ad737d640f91 Mon Sep 17 00:00:00 2001 From: Rye Date: Sat, 10 Oct 2026 10:47:56 -0400 Subject: [PATCH] A local declared again in its own scope is offered and called as the one in effect Luau keeps every declaration a scope makes among its bindings, an unordered_map keyed by each local's node, and so ordered by the address the parser put it at. Its autocomplete, and a fragment's check bringing in the last check's type of each name the fragment uses, look a local up by its name there and take the first the map gives. Where a scope declares a name twice, which one that was changed from run to run: after `local function test()` and `local test = 1`, `test` was offered as the function in one run and as the number in the next, over the whole script and over a fragment alike, and signature help over a fragment could show the function a later declaration had shadowed. alluaufragment 2 failed in about a third of runs on scope3.lsl, whose SLua has both at its top level: the fragment and the whole script picked differently. While Luau is asked, ALLuauShadowed now takes out of each scope of the module the declarations of a name other than the one in effect at the place -- the last declared before it, but for one whose `local` the place is in -- and puts them back after. Where a fragment declares a name again beside the one it brought from the last check, both are in the fragment's own scope, which Luau makes and reads in the one call; there the whole script answers instead. Luau is slua's, so this is worked around here rather than patched. alluaufragment 6. alluaufragment passes 40 runs in a row, and 16 with the new solver. Co-Authored-By: Claude Opus 5.5 --- indra/alscript/CMakeLists.txt | 2 + indra/alscript/luau/alluaucompletion.cpp | 8 +- indra/alscript/luau/alluaufragment.cpp | 14 +- indra/alscript/luau/alluaushadowed.cpp | 194 +++++++++++++++++++ indra/alscript/luau/alluaushadowed.h | 72 +++++++ indra/alscript/tests/alluaufragment_test.cpp | 70 +++++++ 6 files changed, 357 insertions(+), 3 deletions(-) create mode 100644 indra/alscript/luau/alluaushadowed.cpp create mode 100644 indra/alscript/luau/alluaushadowed.h diff --git a/indra/alscript/CMakeLists.txt b/indra/alscript/CMakeLists.txt index e6e13350de..a7ae6d5182 100644 --- a/indra/alscript/CMakeLists.txt +++ b/indra/alscript/CMakeLists.txt @@ -68,6 +68,7 @@ target_sources( luau/alluaufrontend.cpp luau/alluaunavigation.cpp luau/alluauservice.cpp + luau/alluaushadowed.cpp luau/alluausharedstart.cpp luau/alluautaskpool.cpp luau/alluautypes.cpp @@ -153,6 +154,7 @@ target_sources( luau/alluaufrontend.h luau/alluaunavigation.h luau/alluauservice.h + luau/alluaushadowed.h luau/alluausharedstart.h luau/alluautaskpool.h luau/alluautypes.h diff --git a/indra/alscript/luau/alluaucompletion.cpp b/indra/alscript/luau/alluaucompletion.cpp index 3caac116ef..f899785942 100644 --- a/indra/alscript/luau/alluaucompletion.cpp +++ b/indra/alscript/luau/alluaucompletion.cpp @@ -28,6 +28,7 @@ #include "alluaufragment.h" #include "alluaufrontend.h" +#include "alluaushadowed.h" #include "alluautypes.h" #include "alscriptlexicon.h" @@ -340,7 +341,12 @@ std::vector ALLuauCompletion::complete(std::string_view sour { return {}; } - return answer(Luau::autocomplete(*mFront.frontend, mFront.moduleName, at, strings), *module, at); + Luau::AutocompleteResult found; + { + const ALLuauShadowed shadowed(*module, at); + found = Luau::autocomplete(*mFront.frontend, mFront.moduleName, at, strings); + } + return answer(found, *module, at); } std::vector ALLuauCompletion::answer(const Luau::AutocompleteResult& found, const Luau::Module& module, Luau::Position at) const diff --git a/indra/alscript/luau/alluaufragment.cpp b/indra/alscript/luau/alluaufragment.cpp index 529f824e0f..5c6c7b8407 100644 --- a/indra/alscript/luau/alluaufragment.cpp +++ b/indra/alscript/luau/alluaufragment.cpp @@ -27,6 +27,7 @@ #include "alluaufragment.h" #include "alluaufrontend.h" +#include "alluaushadowed.h" #include "alscriptlexicon.h" #include "Luau/Allocator.h" @@ -350,7 +351,8 @@ ALLuauFragment::Completion ALLuauFragment::complete(std::string_view source, Lua Luau::FragmentAutocompleteStatusResult made{ Luau::FragmentAutocompleteStatus::Success, std::nullopt }; bool timedOut = false; { - const Restored restored(*mFront.frontend, mFront.moduleName, *mBase); + const Restored restored(*mFront.frontend, mFront.moduleName, *mBase); + const ALLuauShadowed shadowed(*mBase, at); made = Luau::tryFragmentAutocomplete(*mFront.frontend, mFront.moduleName, at, context, std::move(callback)); timedOut = restored.timedOut(); } @@ -392,6 +394,13 @@ ALLuauFragment::Completion ALLuauFragment::complete(std::string_view source, Lua { return answer; } + // A name the fragment declares again beside the last check's of it, + // both in the fragment's own scope, where Luau took whichever its table + // gave first: the whole script, whose scopes are set right first. + if (made.result->freshScope && ALLuauShadowed::ambiguous(*made.result->freshScope, at, found)) + { + return answer; + } ++mFront.fragments; answer.outcome = Outcome::Answered; answer.found = std::move(made.result->acResults); @@ -429,7 +438,8 @@ ALLuauFragment::Typed ALLuauFragment::typecheck(std::string_view source, Luau::P std::pair made{ Luau::FragmentTypeCheckStatus::SkipAutocomplete, {} }; bool timedOut = false; { - const Restored restored(*mFront.frontend, mFront.moduleName, *mBase); + const Restored restored(*mFront.frontend, mFront.moduleName, *mBase); + const ALLuauShadowed shadowed(*mBase, at); try { // To the end of the call at the least, rather than Luau's own diff --git a/indra/alscript/luau/alluaushadowed.cpp b/indra/alscript/luau/alluaushadowed.cpp new file mode 100644 index 0000000000..40e54cc6ca --- /dev/null +++ b/indra/alscript/luau/alluaushadowed.cpp @@ -0,0 +1,194 @@ +/** + * @file alluaushadowed.cpp + * @brief Of a name a scope declares more than once, only the declaration in effect at a place, while Luau is asked about it. + * + * $LicenseInfo:firstyear=2026&license=viewerlgpl$ + * Alchemy Viewer Source Code + * Copyright (C) 2026, Rye + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; + * version 2.1 of the License only. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + * $/LicenseInfo$ + */ + +#include "linden_common.h" + +#include "alluaushadowed.h" + +#include "Luau/Ast.h" +#include "Luau/AstQuery.h" +#include "Luau/Module.h" + +#include +#include + +namespace +{ + // A local a scope declares. + struct Declared + { + std::string_view name; + Luau::Symbol symbol; + Luau::Location location; + }; + + // Each run of one name among locals in order of their names, `each` + // given its first and last. + template + void eachName(const std::vector& locals, Each each) + { + for (auto first = locals.begin(); first != locals.end();) + { + const auto last = std::find_if(first, locals.end(), [&first](const Declared& local) { return local.name != first->name; }); + each(first, last); + first = last; + } + } + + // The locals of a scope that another of its locals shares a name with, + // each name's together in the order they were declared. Most scopes + // declare no name twice, and have none. + void sharedNames(const Luau::Scope& scope, std::vector& out) + { + out.clear(); + if (scope.bindings.size() < 2) + { + return; + } + for (const auto& [symbol, binding] : scope.bindings) + { + if (symbol.local) + { + out.push_back({ symbol.local->name.value, symbol, binding.location }); + } + } + std::sort(out.begin(), out.end(), [](const Declared& a, const Declared& b) { + return a.name != b.name ? a.name < b.name : a.location.begin < b.location.begin; + }); + size_t shared = 0; + for (size_t first = 0; first < out.size();) + { + size_t last = first + 1; + while (last < out.size() && out[last].name == out[first].name) + { + ++last; + } + for (size_t i = first; last - first > 1 && i < last; ++i) + { + out[shared++] = out[i]; + } + first = last; + } + out.resize(shared); + } + + // Whether the place `ancestry` was found at is in the statement that + // declares `local`, as Luau's autocomplete asks (isBeingDefined): the + // `local` the name is in, its value being written. + bool declaring(const Luau::AstLocal* local, const std::vector& ancestry) + { + for (const Luau::AstNode* node : ancestry) + { + if (const Luau::AstStatLocal* statement = node->as()) + { + if (std::find(statement->vars.begin(), statement->vars.end(), local) != statement->vars.end()) + { + return true; + } + } + } + return false; + } +} + +ALLuauShadowed::ALLuauShadowed(const Luau::Module& module, Luau::Position at) +{ + std::vector shared; + std::vector ancestry; + bool found = false; + for (const auto& [where, scope] : module.scopes) + { + sharedNames(*scope, shared); + if (shared.empty()) + { + continue; + } + // What `at` is in, found as Luau's autocomplete finds it: only + // where a name is declared again at all. + if (!found && module.root) + { + found = true; + ancestry = Luau::findAncestryAtPositionForAutocomplete(module.root, at); + } + eachName(shared, [&](auto first, auto last) { + // The one in effect: declared before `at`, as Luau's own test + // has it (isBindingLegalAtCurrentPosition), the last of those + // but one whose statement `at` is in. + auto kept = last; + for (auto it = first; it != last; ++it) + { + if (it->location.end < at && !declaring(it->symbol.local, ancestry)) + { + kept = it; + } + } + for (auto it = first; it != last; ++it) + { + if (it == kept) + { + continue; + } + // Taken out by where it is: by its key is ambiguous to + // libc++, a Symbol being constructible, if deleted, from + // anything. + const Bindings::const_iterator bound = scope->bindings.find(it->symbol); + if (bound != scope->bindings.end()) + { + mAside.emplace_back(scope.get(), scope->bindings.extract(bound)); + } + } + }); + } +} + +ALLuauShadowed::~ALLuauShadowed() +{ + for (auto it = mAside.rbegin(); it != mAside.rend(); ++it) + { + it->first->bindings.insert(std::move(it->second)); + } +} + +// static +bool ALLuauShadowed::ambiguous(const Luau::Scope& scope, Luau::Position at, const Luau::AutocompleteResult& found) +{ + // Autocomplete reads a scope's bindings by name for an expression, + // passing over a local whose statement the place is in, and for a + // statement, not. + const bool expression = found.context == Luau::AutocompleteContext::Expression; + if (!expression && found.context != Luau::AutocompleteContext::Statement) + { + return false; + } + std::vector shared; + sharedNames(scope, shared); + bool told = false; + eachName(shared, [&](auto first, auto last) { + const auto offered = std::count_if(first, last, [&](const Declared& local) { + return local.location.end < at && !(expression && declaring(local.symbol.local, found.ancestry)); + }); + told = told || offered > 1; + }); + return told; +} diff --git a/indra/alscript/luau/alluaushadowed.h b/indra/alscript/luau/alluaushadowed.h new file mode 100644 index 0000000000..0de9fedde8 --- /dev/null +++ b/indra/alscript/luau/alluaushadowed.h @@ -0,0 +1,72 @@ +/** + * @file alluaushadowed.h + * @brief Of a name a scope declares more than once, only the declaration in effect at a place, while Luau is asked about it. + * + * $LicenseInfo:firstyear=2026&license=viewerlgpl$ + * Alchemy Viewer Source Code + * Copyright (C) 2026, Rye + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; + * version 2.1 of the License only. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + * $/LicenseInfo$ + */ + +#pragma once + +#include "Luau/AutocompleteTypes.h" +#include "Luau/Location.h" +#include "Luau/Scope.h" + +#include +#include + +namespace Luau +{ + struct Module; +} + +// A module's scopes as Lua has them at a place, while this lives: of the +// locals a scope declares more than once under one name, only the one in +// effect there -- the last declared before it, but for one whose statement +// the place is in -- and none where none is. The others are put back as +// this goes. +// +// Luau keeps each declaration a scope makes among its bindings, a table +// ordered by where the parser put each. Where it looks a local up by its +// name -- autocomplete offering what is in scope, a fragment's check +// bringing the last check's type of each name it uses -- it takes the first +// the table gives: for the same script at the same place, one run offers +// `local function test` and the next the `local test = 1` declared after +// it. Where it looks one up by its declaration, as a check does each use, +// there is no such choice. +// +// Over a module of one thread's front end, as the questions are. +class ALLuauShadowed +{ +public: + ALLuauShadowed(const Luau::Module& module, Luau::Position at); + ~ALLuauShadowed(); + ALLuauShadowed(const ALLuauShadowed&) = delete; + ALLuauShadowed& operator=(const ALLuauShadowed&) = delete; + + // Whether what autocomplete `found` at `at` took of `scope` by the + // table's order: two of its locals of one name it would offer there. + // For a scope Luau makes and reads in the one call, which nothing can + // set right before it is read: a fragment's own. + static bool ambiguous(const Luau::Scope& scope, Luau::Position at, const Luau::AutocompleteResult& found); + +private: + using Bindings = decltype(Luau::Scope::bindings); + std::vector> mAside; +}; diff --git a/indra/alscript/tests/alluaufragment_test.cpp b/indra/alscript/tests/alluaufragment_test.cpp index 71ea552669..e20d55976d 100644 --- a/indra/alscript/tests/alluaufragment_test.cpp +++ b/indra/alscript/tests/alluaufragment_test.cpp @@ -681,4 +681,74 @@ namespace tut ensure_equals(std::string(place.line) + ": over the whole script", stubbed(whole.complete(text, 1, place.column)), place.offered); } } + + template<> template<> + void alluaufragment_object::test<6>() + { + set_test_name("a local declared again in its own scope is the declaration in effect, offered and called, over a fragment as over the whole script, wherever the parser put each"); + ensure("definitions loaded: " + error, loaded); + // `test` a function, then a number in the same scope, and a line + // typed below since the last check: the name alone, and a third + // `test` initialized with it. Luau keeps both declarations among the + // scope's bindings, a table ordered by where the parser put each, + // and took whichever it gave first. And the number declared since + // the last check as well, which the fragment declares beside the + // function it brings from that check -- with the other locals the + // line uses, so that there are enough for that table's order to be + // by where they were put too. Each round puts more locals above, so + // that everything lands elsewhere. + struct Place + { + const char* checked; + const char* typed; + S32 column; + }; + const Place places[] = { + { "local test = 1\n", "test", 0 }, + { "local test = 1\n", "local test = test", 13 }, + { "", "local test = 1\ntest", 0 }, + { "", "local test = 1\nprint(a, b, c, d, e, f, g, h, test)", 30 }, + }; + fragment.setDocument("redeclared"); + whole.setDocument("redeclared"); + size_t asked = 0, answered = 0; + for (S32 round = 0; round < 24; ++round) + { + std::string above; + for (S32 i = 0; i < round; ++i) + { + above += llformat("local v%d = %d\n", i, i); + } + above += "local a, b, c, d, e, f, g, h = 1, 2, 3, 4, 5, 6, 7, 8\nlocal function test()\nend\n"; + const auto ask = [&](const std::string& base, const std::string& typed, auto question) { + const std::string text = base + typed + "\n"; + const S32 line = static_cast(std::count(text.begin(), text.end(), '\n')) - 1; + fragment.check(base); + fragment.warm(base); + const size_t before = fragment.fragmentsChecked(); + question(fragment, text, line, "a fragment"); + question(whole, text, line, "the whole script"); + ++asked; + answered += fragment.fragmentsChecked() != before; + }; + for (const Place& place : places) + { + ask(above + place.checked, place.typed, [&](ALLuauService& service, const std::string& text, S32 line, const char* over) { + const std::vector found = service.complete(text, line, place.column); + const auto test = std::find_if(found.begin(), found.end(), [](const ALScriptCompletion& c) { return c.text == "test"; }); + ensure(llformat("round %d, %s, over %s: the number", round, place.typed, over), + test != found.end() && test->kind == ALScriptSymbolKind::Variable); + }); + } + // A function of two strings declared over one of a number: a + // call typed below takes the two. + ask(above + "local function call(n: number)\nend\nlocal call = function(s: string, t: string)\nend\n", "call(\"x\", \"y\")", + [&](ALLuauService& service, const std::string& text, S32 line, const char* over) { + const ALScriptSignature found = service.signature(text, line, 5); + ensure(llformat("round %d, a call, over %s: the two strings, not %s", round, over, found.label.c_str()), + found.found && found.parameters.size() == 2); + }); + } + ensure("answered over a fragment", answered * 2 > asked); + } }