[clang] Add flag to DeclContext to distinguish between qualified and unqualified name lookups

[evgeny777]

Summary

Hi, all!
Currently lldb evaluation of qualified global variables does not work properly.
I've created a revision addressing the issue, which is here:
http://reviews.llvm.org/D13350

To fix the issue in lldb one has to distinguish between qualified and unqualified identifier lookups. In case of qualified lookup the identifier search scope is restricted to DeclContext::DeclKind (Namespace, TranslationUnit). For unqualified lookups scopes are being searched from bottom to top, until you get the first name match

[evgeny777]

Hello Jim and Sean,

Greg suggested adding you for this review. Can you please take a look?

[aaron.ballman]

I'm uncertain whether this change is good or not (Richard is more likely to have thoughts on that), but the patch is missing tests.

clang/lib/Sema/SemaLookup.cpp

208

This should be a local class defined more closely to its usage.

[evgeny777]

What kind of tests do you propose for this? It doesn't introduce any new features to clang, only to lldb where the separate review exists

[aaron.ballman]

In D13383#260815, @evgeny777 wrote:

What kind of tests do you propose for this? It doesn't introduce any new features to clang, only to lldb where the separate review exists

It modifies the behavior of all qualified lookups in Clang, so I would assume there would be some behavior change associated with this?

[evgeny777]

In case of qualified lookup it raises flag in DeclContext, which is not used anywhere else in clang, only in lldb (clang expression parser is linked to lldb).

[aaron.ballman]

In D13383#260821, @evgeny777 wrote:

In case of qualified lookup it raises flag in DeclContext, which is not used anywhere else in clang, only in lldb (clang expression parser is linked to lldb).

Hah, good point, the test case is in lldb, not in clang, for that very reason. :-P Carry on (I must need more sleep).

[evgeny777]

Can someone look at it, please?

[aaron.ballman]

With moving the definition of the Deinitializer class closer to its use (it should be defined within the function itself given the limited use), LGTM.

clang/lib/Sema/SemaLookup.cpp

211

Formatting (the & goes with d).

[evgeny777]

Deinitializer class was renamed and moved under LookupQualifiedName() function scope

clang/include/clang/AST/DeclBase.h

   /// that are missing from the lookup table.
   mutable bool HasLazyExternalLexicalLookups : 1;
 
+  /// \brief If \c true, lookups should only return identifier from
+  /// DeclContext scope (for example TranslationUnit). Used in
+  /// LookupQualifiedName()
+  mutable bool UseQualifiedLookup : 1;
+
   /// \brief Pointer to the data structure used to lookup declarations
   /// within this context (or a DependentStoredDeclsMap if this is a
   /// dependent context). We maintain the invariant that, if the map

         ExternalVisibleStorage(false),
         NeedToReconcileExternalVisibleStorage(false),
         HasLazyLocalLexicalLookups(false), HasLazyExternalLexicalLookups(false),
+        UseQualifiedLookup(false),
         LookupPtr(nullptr), FirstDecl(nullptr), LastDecl(nullptr) {}
 
 public:

                  D == LastDecl);
   }
 
+  bool setUseQualifiedLookup(bool use = true) {
+    bool old_value = UseQualifiedLookup;
+    UseQualifiedLookup = use;
+    return old_value;
+  }
+
+  bool shouldUseQualifiedLookup() const {
+    return UseQualifiedLookup;
+  }
+
   static bool classof(const Decl *D);
   static bool classof(const DeclContext *D) { return true; }
 

clang/lib/Sema/SemaLookup.cpp

                                                UnqualUsingEntry::Comparator()));
     }
   };
+
+  class Deinitializer {
+    std::function<void()> Deinit;
+  public:
+    Deinitializer(const std::function<void()>& d): Deinit(d) {}
+    ~Deinitializer() { Deinit(); }
+  };
 }
 
 // Retrieve the set of identifier namespaces that correspond to a

          "Declaration context must already be complete!");
 
   // Perform qualified name lookup into the LookupCtx.
+  bool oldval = LookupCtx->setUseQualifiedLookup();
+  Deinitializer deinit([&]() { LookupCtx->setUseQualifiedLookup(oldval); });
+
   if (LookupDirect(*this, R, LookupCtx)) {
     R.resolveKind();
     if (isa<CXXRecordDecl>(LookupCtx))