[PATCH] D119136: [clang] Implement Change scope of lambda trailing-return-type

[Noel Grandin via Phabricator via cfe-commits]

grandinj added a comment.

thanks for the quick response


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119136

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119136: [clang] Implement Change scope of lambda trailing-return-type

[Noel Grandin via Phabricator via cfe-commits]

grandinj added a comment.

I note that building on Ubuntu, I am now seeing failures like:

env/gbuildtojson/gbuildtojson.cxx:10:
In file included from 
/usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/algorithm:74:
In file included from 
/usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/pstl/glue_algorithm_defs.h:13:
In file included from 
/usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/functional:61:
In file included from 
/usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/unordered_map:52:
In file included from 
/usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/debug/unordered_map:49:
/usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/debug/safe_unordered_container.h:71:28:
 error: captured variable '__local_end' cannot appear here

  [__local_end](__decltype(__local_end) __it)
   ^

/usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/debug/safe_unordered_container.h:71:4:
 note: variable '__local_end' is explicitly captured here

  [__local_end](__decltype(__local_end) __it)
   ^

/usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/debug/safe_unordered_container.h:69:2:
 note: '__local_end' declared here

  auto __local_end = _M_cont()._M_base().cend(0);
  ^

/usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/debug/safe_unordered_container.h:79:44:
 error: captured variable '__end' cannot appear here

  this->_M_invalidate_if([__end](__decltype(__end) __it)
^

/usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/debug/safe_unordered_container.h:79:26:
 note: variable '__end' is explicitly captured here

  this->_M_invalidate_if([__end](__decltype(__end) __it)
  ^

/usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/debug/safe_unordered_container.h:78:2:
 note: '__end' declared here

  auto __end = _M_cont()._M_base().cend();
  ^


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119136

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86465: [analyzer][solver] Redesign constraint ranges data structure

[Noel Grandin via Phabricator via cfe-commits]

grandinj added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:88
+  // structure is preferred.
+  using ImplType = llvm::SmallVector;
+

Just curious - if they mostly contain 1 or 2 elements, why is N == 4  here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86465

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80202: [ASTMatchers] Performance optimization for memoization

[Noel Grandin via Phabricator via cfe-commits]

grandinj added inline comments.



Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:78
+  const BoundNodesTreeBuilder 
+  const TraversalKind Traversal;
+

note that making fields const tends to disable various move optimisations, so 
better to mark the whole struct const in code that uses it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80202



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76572: Replace `T(x)` with `reinterpret_cast(x)` everywhere it means reinterpret_cast. No functional change

[Noel Grandin via Phabricator via cfe-commits]

grandinj added a comment.

In D76572#1936861 , @Quuxplusone wrote:

> Nice. Does LibreOffice have anything (either in clang-tidy or in a paper 
> guideline) against `T(x)`-style casts? E.g.


No, we don't have very many of those in our codebase, so we have left them 
alone.
Our plugin is designed to convert c-style casts to modern C++ casts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76572



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76572: Replace `T(x)` with `reinterpret_cast(x)` everywhere it means reinterpret_cast. No functional change

[Noel Grandin via Phabricator via cfe-commits]

grandinj added a comment.

Libreoffice has a similar clang-tidy-style cast-checker here:

https://cgit.freedesktop.org/libreoffice/core/tree/compilerplugins/clang/cstylecast.cxx
https://cgit.freedesktop.org/libreoffice/core/tree/compilerplugins/clang/test/cstylecast.cxx


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76572



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52136: [clang-tidy] Add modernize-concat-nested-namespaces check

[Noel Grandin via Phabricator via cfe-commits]

grandinj added a comment.
Herald added a project: LLVM.

Hi

Thanks a lot for this checker - would it be possible to enhance it to also 
update stuff in associated header files?

Thanks


Repository:
  rL LLVM

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

https://reviews.llvm.org/D52136



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

[Noel Grandin via Phabricator via cfe-commits]

grandinj added a comment.

You might want to look at the equivalent LibreOffice checkers, which we have 
been using for some time

https://cgit.freedesktop.org/libreoffice/core/tree/compilerplugins/clang/external.cxx
https://cgit.freedesktop.org/libreoffice/core/tree/compilerplugins/clang/test/external.cxx

https://cgit.freedesktop.org/libreoffice/core/tree/compilerplugins/clang/externandnotdefined.cxx

https://cgit.freedesktop.org/libreoffice/core/tree/compilerplugins/clang/externvar.cxx
https://cgit.freedesktop.org/libreoffice/core/tree/compilerplugins/clang/test/externvar.hxx
https://cgit.freedesktop.org/libreoffice/core/tree/compilerplugins/clang/test/externvar.cxx

As you can see, we are a little less organised :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73413



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68074: [clang-tidy] Add readability-make-member-function-const

[Noel Grandin via Phabricator via cfe-commits]

grandinj added a comment.

Nice! I went with a more brute force approach, and I also struggled with 
logical vs physical const-correctness

https://cgit.freedesktop.org/libreoffice/core/tree/compilerplugins/clang/store/constmethod.cxx


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68074



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65250: [analyzer] exploded-graph-rewriter: Improve user-friendliness.

[Noel Grandin via Phabricator via cfe-commits]

grandinj added a comment.

There is no need to wrap SVG in HTML if you want to display it in a 
web-browser, you can just open the web-browser explicitly, they already support 
opening SVG, use something like

  webbrowser.open_new(url)

i.e. https://docs.python.org/2/library/webbrowser.html


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

https://reviews.llvm.org/D65250



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40409: [Tooling] Acknowledge that many CompilationDatabases don't support enumeration.

[Noel Grandin via Phabricator via cfe-commits]

grandinj added inline comments.



Comment at: include/clang/Tooling/CompilationDatabase.h:122
+  /// can enumerate their source files.
+  virtual std::vector getAllFiles() const { return {}; }
 

I know very little about LLVM's standards, so ignore me if I'm wrong, but 
shouldn't this be returning a pair of (begin,end) iterators rather than 
potentially a copy of a very large array of strings?

And shouldn't it be returning an iteration over StringRef rather then 
std::string, which will require copying the actual data?



Comment at: include/clang/Tooling/CompilationDatabase.h:133
+  /// getCompileCommands(). Subclasses may override this for efficiency.
+  virtual std::vector getAllCompileCommands() const;
 };

similarly here


https://reviews.llvm.org/D40409



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31029: [analyzer] Fix logical not for pointers with different bit width

[Noel Grandin via Phabricator via cfe-commits]

grandinj added inline comments.



Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:313
 
-  Loc makeNull() {
-return loc::ConcreteInt(BasicVals.getZeroWithPtrWidth());
+  // Pass type to accomodate for different pointer bit-witdths of different
+  // address spaces.

spelling and grammar and doxygen formatting:

/// \param type accommodate different pointer bit-widths of different address 
spaces.


Repository:
  rL LLVM

https://reviews.llvm.org/D31029



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28716: [libc++] Diagnose null inputs in std::string using _LIBCPP_DIAGNOSE_WARNING

[Noel Grandin via Phabricator via cfe-commits]

grandinj added a comment.

would it be better to internally treat __attribute__((non_null)) like 
diagnose_if ? then you get all the warnings for free?


https://reviews.llvm.org/D28716



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits