[PATCH] D116326: [CodeCompletion] Signature help for aggregate initialization.

Kadir Cetinkaya via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 3 04:41:51 PST 2022


kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1184
-      auto Name = Ident->getName();
-      if (!Name.empty())
-        ParamNames.insert(Name.str());
----------------
we were never recording unnamed params before, now they'll be surfaced during comment code completions. can you either keep dropping them here or on the code completion generation side (~line 1997)


================
Comment at: clang/include/clang/Sema/CodeCompleteConsumer.h:1057
+        : Kind(CK_Aggregate), AggregateType(Aggregate) {
+      assert(Aggregate != nullptr);
+    }
----------------
we don't have assertions in other cases, why here? (i think we should have it in other cases too, just wondering if there are any valid states where we construct an overloadcandidate from nullptr)


================
Comment at: clang/lib/Sema/CodeCompleteConsumer.cpp:522
+    if (const auto *CRD = dyn_cast<CXXRecordDecl>(AggregateType))
+      Count += CRD->getNumBases();
+    return Count;
----------------
i think this is only valid for c++17 onwards `If the number of initializer clauses exceeds the number of members and bases (since C++17) to initialize, the program is ill-formed.` (same for param type/decl)

it gets even more complicated, since one doesn't have to nest initialization of base class fields, e.g this is valid:
```
struct X
{
    int a;
    int b;
};

struct Y : X
{
    int c;
};

Y y1{ {1,2}, 3};
Y y2{ 1, 2, 3 }; // both of these set a=1, b=2, c=3
```


================
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:6031
+                                         ArrayRef<Expr *> Args) {
+  constexpr unsigned Invalid = std::numeric_limits<unsigned>::max();
+
----------------
also `static`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116326/new/

https://reviews.llvm.org/D116326



More information about the cfe-commits mailing list