ilya-biryukov added a comment.
The new uploaded diff has lots of unrelated changes to clang-tidy, clang-move,
etc...
Looking at commits, it seems `arc diff` was called with the wrong base
commit...
Could you please reupload the change?
================
Comment at: clangd/Quality.h:45
+
+// Attributes of a symbol that affect how much we like it.
+struct SymbolQualitySignals {
----------------
ilya-biryukov wrote:
> Maybe use doxygen-style comments to be consistent with the rest of LLVM?
Some changes are missing?
File still uses 2-slash instead of 3-slash comments.
================
Comment at: clangd/Quality.h:66
+ float NameMatch = 1;
+ bool Unavailable = false;
+
----------------
sammccall wrote:
> ilya-biryukov wrote:
> > Maybe rename to `Inaccessible`? It seems to be closer to what this bool
> > means in C++, if I'm reading the code correctly.
> > Or add a comment explaining what "unavailable" means?
> So it's `Unavailable || Inaccessible`, where neither is all that well-defined
> :-)
> I renamed to `Forbidden` to avoid conflation with either, and added examples
> as a comment.
Thanks! `Forbidden` with a comment LG.
================
Comment at: unittests/clangd/TestTU.h:28
+ TestTU() = default;
+ TestTU(llvm::StringRef Code, llvm::StringRef HeaderCode = "")
+ : Code(Code), HeaderCode(HeaderCode) {}
----------------
sammccall wrote:
> ilya-biryukov wrote:
> > I really like this helper, now that we can reuse the code between different
> > tests!
> > It took me some time to get the semantics of this constructor, though.
> > I suggest to have a few constructor functions with more descriptive name
> > (my names are not great, but should give the idea):
> > ```
> > static TestTU FromSourceFile(StringRef Code);
> > static TestTU FromHeaderFile(StringRef Code);
> > static TestTU WithImplicitInclude(StringRef Source, StringRef
> > IncludedHeader);
> > ```
> >
> >
> >
> Done, Just added `withCode` and `withHeaderCode` for now, and anyone who
> wants something more complicated can set the fields directly.
> (The names mirror the struct fields)
LG, thanks.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D46524
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
