Merge pull request #2523 from secondlife/lua-feature-flag

Put viewer's Lua functionality behind a feature flag, default off.

12 files changed, 352 insertions, 107 deletions

diff --git a/indra/llcommon/lldefs.h b/indra/llcommon/lldefs.h
index 2fbb26dc1a..d4b063f88c 100644
--- a/indra/llcommon/lldefs.h
+++ b/indra/llcommon/lldefs.h
@@ -28,6 +28,7 @@
 #define LL_LLDEFS_H
 
 #include "stdtypes.h"
+#include <cassert>
 #include <type_traits>
 
 // Often used array indices
@@ -169,6 +170,38 @@ constexpr U32   MAXADDRSTR      = 17;       // 123.567.901.345 = 15 chars + \0 +
 //   llclampb(a)     // clamps a to [0 .. 255]
 //
 
+// llless(d0, d1) safely compares d0 < d1 even if one is signed and the other
+// is unsigned. A simple (d0 < d1) expression converts the signed operand to
+// unsigned before comparing. If the signed operand is negative, that flips
+// the negative value to a huge positive value, producing the wrong answer!
+// llless() specifically addresses that case.
+template <typename T0, typename T1>
+inline bool llless(T0 d0, T1 d1)
+{
+    if constexpr (std::is_signed_v<T0> && ! std::is_signed_v<T1>)
+    {
+        // T0 signed, T1 unsigned: negative d0 is less than any unsigned d1
+        if (d0 < 0)
+            return true;
+        // both are non-negative: explicitly cast to avoid C4018
+        return std::make_unsigned_t<T0>(d0) < d1;
+    }
+    else if constexpr (! std::is_signed_v<T0> && std::is_signed_v<T1>)
+    {
+        // T0 unsigned, T1 signed: any unsigned d0 is greater than negative d1
+        if (d1 < 0)
+            return false;
+        // both are non-negative: explicitly cast to avoid C4018
+        return d0 < std::make_unsigned_t<T1>(d1);
+    }
+    else
+    {
+        // both T0 and T1 are signed, or both are unsigned:
+        // straightforward comparison works
+        return d0 < d1;
+    }
+}
+
 // recursion tail
 template <typename T>
 inline auto llmax(T data)
@@ -180,7 +213,7 @@ template <typename T0, typename T1, typename... Ts>
 inline auto llmax(T0 d0, T1 d1, Ts... rest)
 {
     auto maxrest = llmax(d1, rest...);
-    return (d0 > maxrest)? d0 : maxrest;
+    return llless(maxrest, d0)? d0 : maxrest;
 }
 
 // recursion tail
@@ -194,12 +227,28 @@ template <typename T0, typename T1, typename... Ts>
 inline auto llmin(T0 d0, T1 d1, Ts... rest)
 {
     auto minrest = llmin(d1, rest...);
-    return (d0 < minrest) ? d0 : minrest;
+    return llless(d0, minrest) ? d0 : minrest;
 }
 
 template <typename A, typename MIN, typename MAX>
 inline A llclamp(A a, MIN minval, MAX maxval)
 {
+    // The only troublesome case is if A is unsigned and either minval or
+    // maxval is both signed and negative. Casting a negative number to
+    // unsigned flips it to a huge positive number, making this llclamp() call
+    // ineffective.
+    if constexpr (! std::is_signed_v<A>)
+    {
+        if constexpr (std::is_signed_v<MIN>)
+        {
+            assert(minval >= 0);
+        }
+        if constexpr (std::is_signed_v<MAX>)
+        {
+            assert(maxval >= 0);
+        }
+    }
+
     A aminval{ static_cast<A>(minval) }, amaxval{ static_cast<A>(maxval) };
     if ( a < aminval )
     {
diff --git a/indra/llcommon/lua_function.cpp b/indra/llcommon/lua_function.cpp
index f7876e4aaf..eefb1e62cf 100644
--- a/indra/llcommon/lua_function.cpp
+++ b/indra/llcommon/lua_function.cpp
@@ -24,6 +24,7 @@
 #include <unordered_map>
 // external library headers
 // other Linden headers
+#include "commoncontrol.h"
 #include "fsyspath.h"
 #include "hexdump.h"
 #include "llcoros.h"
@@ -529,9 +530,35 @@ int lua_metaipair(lua_State* L);
 } // anonymous namespace
 
 LuaState::LuaState(script_finished_fn cb):
-    mCallback(cb),
-    mState(luaL_newstate())
+    mCallback(cb)
 {
+    /*---------------------------- feature flag ----------------------------*/
+    try
+    {
+        mFeature = LL::CommonControl::get("Global", "LuaFeature").asBoolean();
+    }
+    catch (const LL::CommonControl::NoListener&)
+    {
+        // If this program doesn't have an LLViewerControlListener,
+        // it's probably a test program; go ahead.
+        mFeature = true;
+    }
+    catch (const LL::CommonControl::ParamError&)
+    {
+        // We found LLViewerControlListener, but its settings do not include
+        // "LuaFeature". Hmm, fishy: that feature flag was introduced at the
+        // same time as this code.
+        mFeature = false;
+    }
+    // None of the rest of this is necessary if we're not going to run anything.
+    if (! mFeature)
+    {
+        mError = "Lua feature disabled";
+        return;
+    }
+    /*---------------------------- feature flag ----------------------------*/
+
+    mState = luaL_newstate();
     // Ensure that we can always find this LuaState instance, given the
     // lua_State we just created or any of its coroutines.
     sLuaStateMap.emplace(mState, this);
@@ -682,75 +709,81 @@ int lua_metaipair(lua_State* L)
 
 LuaState::~LuaState()
 {
-    // We're just about to destroy this lua_State mState. Did this Lua chunk
-    // register any atexit() functions?
-    lluau_checkstack(mState, 3);
-    // look up Registry.atexit
-    lua_getfield(mState, LUA_REGISTRYINDEX, "atexit");
-    // stack contains Registry.atexit
-    if (lua_istable(mState, -1))
+    /*---------------------------- feature flag ----------------------------*/
+    if (mFeature)
+    /*---------------------------- feature flag ----------------------------*/
     {
-        // We happen to know that Registry.atexit is built by appending array
-        // entries using table.insert(). That's important because it means
-        // there are no holes, and therefore lua_objlen() should be correct.
-        // That's important because we walk the atexit table backwards, to
-        // destroy last the things we created (passed to LL.atexit()) first.
-        int len(lua_objlen(mState, -1));
-        LL_DEBUGS("Lua") << LLCoros::getName() << ": Registry.atexit is a table with "
-                         << len << " entries" << LL_ENDL;
-
-        // Push debug.traceback() onto the stack as lua_pcall()'s error
-        // handler function. On error, lua_pcall() calls the specified error
-        // handler function with the original error message; the message
-        // returned by the error handler is then returned by lua_pcall().
-        // Luau's debug.traceback() is called with a message to prepend to the
-        // returned traceback string. Almost as if they'd been designed to
-        // work together...
-        lua_getglobal(mState, "debug");
-        lua_getfield(mState, -1, "traceback");
-        // ditch "debug"
-        lua_remove(mState, -2);
-        // stack now contains atexit, debug.traceback()
-
-        for (int i(len); i >= 1; --i)
+        // We're just about to destroy this lua_State mState. Did this Lua chunk
+        // register any atexit() functions?
+        lluau_checkstack(mState, 3);
+        // look up Registry.atexit
+        lua_getfield(mState, LUA_REGISTRYINDEX, "atexit");
+        // stack contains Registry.atexit
+        if (lua_istable(mState, -1))
         {
-            lua_pushinteger(mState, i);
-            // stack contains Registry.atexit, debug.traceback(), i
-            lua_gettable(mState, -3);
-            // stack contains Registry.atexit, debug.traceback(), atexit[i]
-            // Call atexit[i](), no args, no return values.
-            // Use lua_pcall() because errors in any one atexit() function
-            // shouldn't cancel the rest of them. Pass debug.traceback() as
-            // the error handler function.
-            LL_DEBUGS("Lua") << LLCoros::getName()
-                             << ": calling atexit(" << i << ")" << LL_ENDL;
-            if (lua_pcall(mState, 0, 0, -2) != LUA_OK)
+            // We happen to know that Registry.atexit is built by appending array
+            // entries using table.insert(). That's important because it means
+            // there are no holes, and therefore lua_objlen() should be correct.
+            // That's important because we walk the atexit table backwards, to
+            // destroy last the things we created (passed to LL.atexit()) first.
+            int len(lua_objlen(mState, -1));
+            LL_DEBUGS("Lua") << LLCoros::getName() << ": Registry.atexit is a table with "
+                             << len << " entries" << LL_ENDL;
+
+            // Push debug.traceback() onto the stack as lua_pcall()'s error
+            // handler function. On error, lua_pcall() calls the specified error
+            // handler function with the original error message; the message
+            // returned by the error handler is then returned by lua_pcall().
+            // Luau's debug.traceback() is called with a message to prepend to the
+            // returned traceback string. Almost as if they'd been designed to
+            // work together...
+            lua_getglobal(mState, "debug");
+            lua_getfield(mState, -1, "traceback");
+            // ditch "debug"
+            lua_remove(mState, -2);
+            // stack now contains atexit, debug.traceback()
+
+            for (int i(len); i >= 1; --i)
             {
-                auto error{ lua_tostdstring(mState, -1) };
-                LL_WARNS("Lua") << LLCoros::getName()
-                                << ": atexit(" << i << ") error: " << error << LL_ENDL;
-                // pop error message
-                lua_pop(mState, 1);
+                lua_pushinteger(mState, i);
+                // stack contains Registry.atexit, debug.traceback(), i
+                lua_gettable(mState, -3);
+                // stack contains Registry.atexit, debug.traceback(), atexit[i]
+                // Call atexit[i](), no args, no return values.
+                // Use lua_pcall() because errors in any one atexit() function
+                // shouldn't cancel the rest of them. Pass debug.traceback() as
+                // the error handler function.
+                LL_DEBUGS("Lua") << LLCoros::getName()
+                                 << ": calling atexit(" << i << ")" << LL_ENDL;
+                if (lua_pcall(mState, 0, 0, -2) != LUA_OK)
+                {
+                    auto error{ lua_tostdstring(mState, -1) };
+                    LL_WARNS("Lua") << LLCoros::getName()
+                                    << ": atexit(" << i << ") error: " << error << LL_ENDL;
+                    // pop error message
+                    lua_pop(mState, 1);
+                }
+                LL_DEBUGS("Lua") << LLCoros::getName() << ": atexit(" << i << ") done" << LL_ENDL;
+                // lua_pcall() has already popped atexit[i]:
+                // stack contains atexit, debug.traceback()
             }
-            LL_DEBUGS("Lua") << LLCoros::getName() << ": atexit(" << i << ") done" << LL_ENDL;
-            // lua_pcall() has already popped atexit[i]:
-            // stack contains atexit, debug.traceback()
+            // pop debug.traceback()
+            lua_pop(mState, 1);
         }
-        // pop debug.traceback()
+        // pop Registry.atexit (either table or nil)
         lua_pop(mState, 1);
-    }
-    // pop Registry.atexit (either table or nil)
-    lua_pop(mState, 1);
 
-    lua_close(mState);
+        // with the demise of this LuaState, remove sLuaStateMap entry
+        sLuaStateMap.erase(mState);
+
+        lua_close(mState);
+    }
 
     if (mCallback)
     {
         // mError potentially set by previous checkLua() call(s)
         mCallback(mError);
     }
-    // with the demise of this LuaState, remove sLuaStateMap entry
-    sLuaStateMap.erase(mState);
 }
 
 bool LuaState::checkLua(const std::string& desc, int r)
@@ -768,6 +801,14 @@ bool LuaState::checkLua(const std::string& desc, int r)
 
 std::pair<int, LLSD> LuaState::expr(const std::string& desc, const std::string& text)
 {
+    /*---------------------------- feature flag ----------------------------*/
+    if (! mFeature)
+    {
+        // fake an error
+        return { -1, stringize("Not running ", desc) };
+    }
+    /*---------------------------- feature flag ----------------------------*/
+
     set_interrupts_counter(0);
 
     lua_callbacks(mState)->interrupt = [](lua_State *L, int gc)
@@ -843,6 +884,9 @@ std::pair<int, LLSD> LuaState::expr(const std::string& desc, const std::string&
     return result;
 }
 
+// We think we don't need mFeature tests in the rest of these LuaState methods
+// because, if expr() isn't running code, nobody should be calling any of them.
+
 LuaListener& LuaState::obtainListener(lua_State* L)
 {
     lluau_checkstack(L, 2);
diff --git a/indra/llcommon/lua_function.h b/indra/llcommon/lua_function.h
index e28656c03b..967d8eaba1 100644
--- a/indra/llcommon/lua_function.h
+++ b/indra/llcommon/lua_function.h
@@ -115,8 +115,11 @@ public:
     void check_interrupts_counter();
 
 private:
+    /*---------------------------- feature flag ----------------------------*/
+    bool mFeature{ false };
+    /*---------------------------- feature flag ----------------------------*/
     script_finished_fn mCallback;
-    lua_State* mState;
+    lua_State* mState{ nullptr };
     std::string mError;
     S32 mInterrupts{ 0 };
 };
diff --git a/indra/llcommon/tests/llcond_test.cpp b/indra/llcommon/tests/llcond_test.cpp
index f2a302ed13..7d1ac6edb9 100644
--- a/indra/llcommon/tests/llcond_test.cpp
+++ b/indra/llcommon/tests/llcond_test.cpp
@@ -19,6 +19,7 @@
 // other Linden headers
 #include "../test/lltut.h"
 #include "llcoros.h"
+#include "lldefs.h"                 // llless()
 
 /*****************************************************************************
 *   TUT
@@ -64,4 +65,78 @@ namespace tut
         cond.set_all(2);
         cond.wait_equal(3);
     }
+
+    template <typename T0, typename T1>
+    struct compare
+    {
+        const char* desc;
+        T0 lhs;
+        T1 rhs;
+        bool expect;
+
+        void test() const
+        {
+            // fails
+//          ensure_equals(desc, (lhs <  rhs), expect);
+            ensure_equals(desc, llless(lhs, rhs), expect);
+        }
+    };
+
+    template<> template<>
+    void object::test<3>()
+    {
+        set_test_name("comparison");
+        // Try to construct signed and unsigned variables such that the
+        // compiler can't optimize away the code to compare at runtime.
+        std::istringstream input("-1 10 20 10 20");
+        int minus1, s10, s20;
+        input >> minus1 >> s10 >> s20;
+        unsigned u10, u20;
+        input >> u10 >> u20;
+        ensure_equals("minus1 wrong", minus1, -1);
+        ensure_equals("s10 wrong", s10, 10);
+        ensure_equals("s20 wrong", s20, 20);
+        ensure_equals("u10 wrong", u10, 10);
+        ensure_equals("u20 wrong", u20, 20);
+        // signed < signed should always work!
+        compare<int, int> ss[] =
+            { {"minus1 < s10", minus1, s10, true},
+              {"s10 < s10", s10, s10, false},
+              {"s20 < s10", s20, s20, false}
+            };
+        for (const auto& cmp : ss)
+        {
+            cmp.test();
+        }
+        // unsigned < unsigned should always work!
+        compare<unsigned, unsigned> uu[] =
+            { {"u10 < u20", u10, u20, true},
+              {"u20 < u20", u20, u20, false},
+              {"u20 < u10", u20, u10, false}
+            };
+        for (const auto& cmp : uu)
+        {
+            cmp.test();
+        }
+        // signed < unsigned ??
+        compare<int, unsigned> su[] =
+            { {"minus1 < u10", minus1, u10, true},
+              {"s10 < u10", s10, u10, false},
+              {"s20 < u10", s20, u10, false}
+            };
+        for (const auto& cmp : su)
+        {
+            cmp.test();
+        }
+        // unsigned < signed ??
+        compare<unsigned, int> us[] =
+            { {"u10 < minus1", u10, minus1, false},
+              {"u10 < s10", u10, s10, false},
+              {"u10 < s20", u10, s20, true}
+            };
+        for (const auto& cmp : us)
+        {
+            cmp.test();
+        }
+    }
 } // namespace tut
diff --git a/indra/llmeshoptimizer/llmeshoptimizer.cpp b/indra/llmeshoptimizer/llmeshoptimizer.cpp
index 7339454367..9d62a72188 100644
--- a/indra/llmeshoptimizer/llmeshoptimizer.cpp
+++ b/indra/llmeshoptimizer/llmeshoptimizer.cpp
@@ -57,7 +57,7 @@ void LLMeshOptimizer::generateShadowIndexBufferU32(U32 *destination,
     S32 index = 0;
     if (vertex_positions)
     {
-        streams[index].data = (const float*)vertex_positions;
+        streams[index].data = vertex_positions->getF32ptr();
         // Despite being LLVector4a, only x, y and z are in use
         streams[index].size = sizeof(F32) * 3;
         streams[index].stride = sizeof(F32) * 4;
@@ -65,14 +65,14 @@ void LLMeshOptimizer::generateShadowIndexBufferU32(U32 *destination,
     }
     if (normals)
     {
-        streams[index].data = (const float*)normals;
+        streams[index].data = normals->getF32ptr();
         streams[index].size = sizeof(F32) * 3;
         streams[index].stride = sizeof(F32) * 4;
         index++;
     }
     if (text_coords)
     {
-        streams[index].data = (const float*)text_coords;
+        streams[index].data = text_coords->mV;
         streams[index].size = sizeof(F32) * 2;
         streams[index].stride = sizeof(F32) * 2;
         index++;
@@ -108,21 +108,21 @@ void LLMeshOptimizer::generateShadowIndexBufferU16(U16 *destination,
     S32 index = 0;
     if (vertex_positions)
     {
-        streams[index].data = (const float*)vertex_positions;
+        streams[index].data = vertex_positions->getF32ptr();
         streams[index].size = sizeof(F32) * 3;
         streams[index].stride = sizeof(F32) * 4;
         index++;
     }
     if (normals)
     {
-        streams[index].data = (const float*)normals;
+        streams[index].data = normals->getF32ptr();
         streams[index].size = sizeof(F32) * 3;
         streams[index].stride = sizeof(F32) * 4;
         index++;
     }
     if (text_coords)
     {
-        streams[index].data = (const float*)text_coords;
+        streams[index].data = text_coords->mV;
         streams[index].size = sizeof(F32) * 2;
         streams[index].stride = sizeof(F32) * 2;
         index++;
@@ -162,9 +162,9 @@ size_t LLMeshOptimizer::generateRemapMultiU32(
     U64 vertex_count)
 {
     meshopt_Stream streams[] = {
-       {(const float*)vertex_positions, sizeof(F32) * 3, sizeof(F32) * 4},
-       {(const float*)normals, sizeof(F32) * 3, sizeof(F32) * 4},
-       {(const float*)text_coords, sizeof(F32) * 2, sizeof(F32) * 2},
+       {vertex_positions->getF32ptr(), sizeof(F32) * 3, sizeof(F32) * 4},
+       {normals->getF32ptr(), sizeof(F32) * 3, sizeof(F32) * 4},
+       {text_coords->mV, sizeof(F32) * 2, sizeof(F32) * 2},
     };
 
     // Remap can function without indices,
@@ -236,7 +236,7 @@ void LLMeshOptimizer::remapPositionsBuffer(LLVector4a * destination_vertices,
     U64 vertex_count,
     const unsigned int* remap)
 {
-    meshopt_remapVertexBuffer((float*)destination_vertices, (const float*)vertex_positions, vertex_count, sizeof(LLVector4a), remap);
+    meshopt_remapVertexBuffer(destination_vertices->getF32ptr(), vertex_positions->getF32ptr(), vertex_count, sizeof(LLVector4a), remap);
 }
 
 void LLMeshOptimizer::remapNormalsBuffer(LLVector4a * destination_normalss,
@@ -244,7 +244,7 @@ void LLMeshOptimizer::remapNormalsBuffer(LLVector4a * destination_normalss,
     U64 mormals_count,
     const unsigned int* remap)
 {
-    meshopt_remapVertexBuffer((float*)destination_normalss, (const float*)normals, mormals_count, sizeof(LLVector4a), remap);
+    meshopt_remapVertexBuffer(destination_normalss->getF32ptr(), normals->getF32ptr(), mormals_count, sizeof(LLVector4a), remap);
 }
 
 void LLMeshOptimizer::remapUVBuffer(LLVector2 * destination_uvs,
@@ -252,7 +252,7 @@ void LLMeshOptimizer::remapUVBuffer(LLVector2 * destination_uvs,
     U64 uv_count,
     const unsigned int* remap)
 {
-    meshopt_remapVertexBuffer((float*)destination_uvs, (const float*)uv_positions, uv_count, sizeof(LLVector2), remap);
+    meshopt_remapVertexBuffer(destination_uvs->mV, uv_positions->mV, uv_count, sizeof(LLVector2), remap);
 }
 
 //static
@@ -273,7 +273,7 @@ U64 LLMeshOptimizer::simplifyU32(U32 *destination,
         return meshopt_simplifySloppy<unsigned int>(destination,
             indices,
             index_count,
-            (const float*)vertex_positions,
+            vertex_positions->getF32ptr(),
             vertex_count,
             vertex_positions_stride,
             target_index_count,
@@ -286,7 +286,7 @@ U64 LLMeshOptimizer::simplifyU32(U32 *destination,
         return meshopt_simplify<unsigned int>(destination,
             indices,
             index_count,
-            (const float*)vertex_positions,
+            vertex_positions->getF32ptr(),
             vertex_count,
             vertex_positions_stride,
             target_index_count,
@@ -315,7 +315,7 @@ U64 LLMeshOptimizer::simplify(U16 *destination,
         return meshopt_simplifySloppy<unsigned short>(destination,
             indices,
             index_count,
-            (const float*)vertex_positions,
+            vertex_positions->getF32ptr(),
             vertex_count,
             vertex_positions_stride,
             target_index_count,
@@ -328,7 +328,7 @@ U64 LLMeshOptimizer::simplify(U16 *destination,
         return meshopt_simplify<unsigned short>(destination,
             indices,
             index_count,
-            (const float*)vertex_positions,
+            vertex_positions->getF32ptr(),
             vertex_count,
             vertex_positions_stride,
             target_index_count,
diff --git a/indra/llui/llmenugl.cpp b/indra/llui/llmenugl.cpp
index 69ffa9a94f..cc770ca90a 100644
--- a/indra/llui/llmenugl.cpp
+++ b/indra/llui/llmenugl.cpp
@@ -2625,7 +2625,9 @@ void LLMenuGL::insert( S32 position, LLView * ctrl, bool arrange /*= true*/ )
 {
     LLMenuItemGL * item = dynamic_cast<LLMenuItemGL *>(ctrl);
 
-    if (NULL == item || position < 0 || position >= mItems.size())
+    // If position == size(), std::advance() will return end() -- which is
+    // okay, because insert(end()) is the same as append().
+    if (NULL == item || position < 0 || position > mItems.size())
     {
         return;
     }
diff --git a/indra/newview/app_settings/settings.xml b/indra/newview/app_settings/settings.xml
index 248992fd07..c6946d1ec1 100644
--- a/indra/newview/app_settings/settings.xml
+++ b/indra/newview/app_settings/settings.xml
@@ -3999,6 +3999,17 @@
         <string>scripts/lua</string>
       </array>
     </map>
+    <key>LuaFeature</key>
+    <map>
+      <key>Comment</key>
+      <string>Enable viewer's Lua script engine.</string>
+      <key>Persist</key>
+      <integer>1</integer>
+      <key>Type</key>
+      <string>Boolean</string>
+      <key>Value</key>
+      <integer>0</integer>
+    </map>
     <key>LuaRequirePath</key>
     <map>
       <key>Comment</key>
diff --git a/indra/newview/lluilistener.cpp b/indra/newview/lluilistener.cpp
index c389d04443..44eb8461f1 100644
--- a/indra/newview/lluilistener.cpp
+++ b/indra/newview/lluilistener.cpp
@@ -90,12 +90,14 @@ LLUIListener::LLUIListener():
     add("addMenuItem",
         "Add new menu item [\"name\"] with displayed [\"label\"]\n"
         "and call-on-click UI function [\"func\"] with optional [\"param\"]\n"
-        "to the [\"parent_menu\"] within the Top menu.",
+        "to the [\"parent_menu\"] within the Top menu.\n"
+        "If [\"pos\"] is present, insert at specified 0-relative position.",
         &LLUIListener::addMenuItem,
         required_args.with("func", LLSD()));
 
     add("addMenuSeparator",
-        "Add menu separator to the [\"parent_menu\"] within the Top menu.",
+        "Add menu separator to the [\"parent_menu\"] within the Top menu.\n"
+        "If [\"pos\"] is present, insert at specified 0-relative position.",
         &LLUIListener::addMenuSeparator,
         llsd::map("parent_menu", LLSD(), "reply", LLSD()));
 
@@ -264,6 +266,13 @@ LLMenuGL* get_parent_menu(LLEventAPI::Response& response, const LLSD&event)
     return parent_menu;
 }
 
+// Return event["pos"].asInteger() if passed, but clamp (0 <= pos <= size).
+// Reserve -1 return to mean event has no "pos" key.
+S32 get_pos(const LLSD& event, U32 size)
+{
+    return event["pos"].isInteger()? llclamp(event["pos"].asInteger(), 0, size) : -1;
+}
+
 void LLUIListener::addMenu(const LLSD&event) const
 {
     Response response(LLSD(), event);
@@ -302,9 +311,19 @@ void LLUIListener::addMenuItem(const LLSD&event) const
     item_params.on_click = item_func;
     if(LLMenuGL* parent_menu = get_parent_menu(response, event))
     {
-        if(!parent_menu->append(LLUICtrlFactory::create<LLMenuItemCallGL>(item_params)))
+        auto item = LLUICtrlFactory::create<LLMenuItemCallGL>(item_params);
+        // Clamp pos to getItemCount(), meaning append. If pos exceeds that,
+        // insert() will silently ignore the request.
+        auto pos = get_pos(event, parent_menu->getItemCount());
+        if (pos >= 0)
         {
-            response.error(stringize("Menu item ", std::quoted(event["name"].asString()), " was not added"));
+            // insert() returns void: we just have to assume it worked.
+            parent_menu->insert(pos, item);
+        }
+        else if (! parent_menu->append(item))
+        {
+            response.error(stringize("Menu item ", std::quoted(event["name"].asString()),
+                                     " was not added"));
         }
     }
 }
@@ -314,7 +333,19 @@ void LLUIListener::addMenuSeparator(const LLSD&event) const
     Response response(LLSD(), event);
     if(LLMenuGL* parent_menu = get_parent_menu(response, event))
     {
-        if(!parent_menu->addSeparator())
+        // Clamp pos to getItemCount(), meaning append. If pos exceeds that,
+        // insert() will silently ignore the request.
+        auto pos = get_pos(event, parent_menu->getItemCount());
+        if (pos >= 0)
+        {
+            // Even though addSeparator() does not accept a position,
+            // LLMenuItemSeparatorGL isa LLMenuItemGL, so we can use insert().
+            LLMenuItemSeparatorGL::Params p;
+            LLMenuItemGL* separator = LLUICtrlFactory::create<LLMenuItemSeparatorGL>(p);
+            // insert() returns void: we just have to assume it worked.
+            parent_menu->insert(pos, separator);
+        }
+        else if (! parent_menu->addSeparator())
         {
             response.error("Separator was not added");
         }
diff --git a/indra/newview/scripts/lua/auto/menus.lua b/indra/newview/scripts/lua/auto/menus.lua
new file mode 100644
index 0000000000..b2f54d83df
--- /dev/null
+++ b/indra/newview/scripts/lua/auto/menus.lua
@@ -0,0 +1,51 @@
+-- Inject Lua-related menus into the top menu structure. Run this as a Lua
+-- script so that turning off the Lua feature also disables these menus.
+
+-- Under Develop -> Consoles, want to present the equivalent of:
+-- <menu_item_separator/>
+-- <menu_item_check
+--     label="LUA Debug Console"
+--     name="LUA Debug Console">
+--   <menu_item_check.on_check
+--       function="Floater.Visible"
+--       parameter="lua_debug" />
+--   <menu_item_check.on_click
+--       function="Floater.Toggle"
+--       parameter="lua_debug" />
+-- </menu_item_check>
+-- <menu_item_check
+--     label="LUA Scripts Info"
+--     name="LUA Scripts">
+--   <menu_item_check.on_check
+--       function="Floater.Visible"
+--       parameter="lua_scripts" />
+--   <menu_item_check.on_click
+--       function="Floater.Toggle"
+--       parameter="lua_scripts" />
+-- </menu_item_check>
+
+local startup = require 'startup'
+local UI = require 'UI'
+
+-- Don't mess with the viewer's menu structure until we've logged in.
+startup.wait('STATE_STARTED')
+
+-- Add LUA Debug Console to Develop->Consoles
+local pos = 9
+UI.addMenuSeparator{
+    parent_menu='Consoles', pos=pos,
+}
+pos += 1
+UI.addMenuItem{
+    parent_menu='Consoles', pos=pos,
+    name='lua_debug', label='LUA Debug Console',
+    func='Floater.ToggleOrBringToFront', param='lua_debug',
+}
+pos += 1
+
+-- Add LUA Scripts Info to Develop->Consoles
+UI.addMenuItem{
+    parent_menu='Consoles', pos=pos,
+    name='lua_scripts', label='LUA Scripts Info',
+    func='Floater.ToggleOrBringToFront', param='lua_scripts',
+}
diff --git a/indra/newview/scripts/lua/require/UI.lua b/indra/newview/scripts/lua/require/UI.lua
index 73a76fa6b8..cf2695917e 100644
--- a/indra/newview/scripts/lua/require/UI.lua
+++ b/indra/newview/scripts/lua/require/UI.lua
@@ -170,13 +170,13 @@ end
 
 -- see UI.callables() for valid values of 'func'
 function UI.addMenuItem(...)
-    local args = mapargs('name,label,parent_menu,func,param', ...)
+    local args = mapargs('name,label,parent_menu,func,param,pos', ...)
     args.op = 'addMenuItem'
     return leap.request('UI', args)
 end
 
 function UI.addMenuSeparator(...)
-    local args = mapargs('parent_menu', ...)
+    local args = mapargs('parent_menu,pos', ...)
     args.op = 'addMenuSeparator'
     return leap.request('UI', args)
 end
diff --git a/indra/newview/skins/default/xui/en/menu_viewer.xml b/indra/newview/skins/default/xui/en/menu_viewer.xml
index 8999797049..7fe9d0efe5 100644
--- a/indra/newview/skins/default/xui/en/menu_viewer.xml
+++ b/indra/newview/skins/default/xui/en/menu_viewer.xml
@@ -2565,27 +2565,6 @@ function="World.EnvPreset"
                parameter="scene monitor" />
             </menu_item_check>
             <menu_item_separator/>
-            <menu_item_check
-         label="LUA Debug Console"
-         name="LUA Debug Console">
-            <menu_item_check.on_check
-             function="Floater.Visible"
-             parameter="lua_debug" />
-            <menu_item_check.on_click
-             function="Floater.Toggle"
-             parameter="lua_debug" />
-        </menu_item_check>
-            <menu_item_check
-            label="LUA Scripts Info"
-            name="LUA Scripts">
-            <menu_item_check.on_check
-             function="Floater.Visible"
-             parameter="lua_scripts" />
-            <menu_item_check.on_click
-             function="Floater.Toggle"
-             parameter="lua_scripts" />
-            </menu_item_check>
-            <menu_item_separator/>
 
             <menu_item_call
              label="Region Info to Debug Console"
diff --git a/indra/newview/tests/cppfeatures_test.cpp b/indra/newview/tests/cppfeatures_test.cpp
index f5ea3a522b..ca94dcfc95 100644
--- a/indra/newview/tests/cppfeatures_test.cpp
+++ b/indra/newview/tests/cppfeatures_test.cpp
@@ -283,7 +283,7 @@ void cpp_features_test_object_t::test<8>()
     ensure("init member inline 1", ii.mFoo==10);
 
     InitInlineWithConstructor iici;
-    ensure("init member inline 2", iici.mFoo=10);
+    ensure("init member inline 2", iici.mFoo==10);
     ensure("init member inline 3", iici.mBar==25);
 }