This feels wrong to me... It could just as well be a field or something else.

If anything, it feels more like a dynamic member access, but that is also a bit of a hack. Perhaps we need new classes of unknown expressions, i.e.

| 122 = @unknown_member_access_expr
| 123 = @unknown_binary_expr
...

and then have

@member_access_expr = ... | @unknown_member_access_expr;
@bin_op = ... | @unknown_binary_expr;

The choice is between wrong some of the time or missing all of the time. So I would argue that the change I made is a step in the right direction that is unlikely to make things worse. I think your changes seem reasonable but I don't think this PR is the place for them, particularly since dataflow libraries and queries would need to be reworked.

Perhaps we should add this to Semmle.Util and use instead:

using System.Collections;
using System.Collections.Concurrent;
using System.Collections.Generic;

namespace Semmle.Util
{
    public class ConcurrentHashSet<T> : IEnumerable<T>
    {
        private readonly ConcurrentDictionary<T, object> dict;
        public ConcurrentHashSet()
        {
            dict = new ConcurrentDictionary<T, object>();
        }

        public void Add(T t) => dict[t] = null;

        public bool Contains(T t) => dict.ContainsKey(t);

        public IEnumerator<T> GetEnumerator() => dict.Keys.GetEnumerator();

        IEnumerator IEnumerable.GetEnumerator() => dict.Keys.GetEnumerator();
    }
}

My reservation with this is that this would be a half-baked implementation, that should also implement

[System.Serializable]
public class ConcurrentHashSet<T> : System.Collections.Generic.ICollection<T>, System.Collections.Generic.IEnumerable<T>, System.Collections.Generic.IReadOnlyCollection<T>, System.Collections.Generic.ISet<T>, System.Runtime.Serialization.IDeserializationCallback, System.Runtime.Serialization.ISerializable

So we should really just reference the nuget package ConcurrentHashSet which I presume is totally correct and bug-free.....

Then it means updating resources/lib which is a bit of a pain.

I think a half-baked implementation suffices, as we only need to add the things that we actually need. If we ever need, say, to use it as an ICollection<T>, then we can implement it then.

There's literally one place where we use ConcurrentDictionary instead of ConcurrentHashSet so I'm really not convinced it's worth the effort. Particularly because we'd need a unit test suite for the new ConcurrentHashSet class.

OK 👍

Nice rewrite.