aaron.ballman added inline comments.

================
Comment at: 
clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp:56
+                                            hasType(Container),
+                                            hasType(pointsTo(Container)),
+                                            hasType(references(Container))))))
----------------
Is this `pointsTo()` correct? IIRC, that would be trying to capture something 
like:
```
std::vector<int> *container;
&container[0];
```
but this wouldn't call the `operator[]` overload, it'd be using the builtin 
array subscripting, so I'm not certain this would match anything.


================
Comment at: 
clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp:63
+                                            hasType(Container),
+                                            hasType(pointsTo(Container)),
+                                            hasType(references(Container))))))
----------------
Same question here about `pointsTo()`.


================
Comment at: 
clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp:75
+                                          hasType(Container),
+                                          hasType(pointsTo(Container)),
+                                          hasType(references(Container))))))
----------------
Here too.


================
Comment at: 
clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp:88
+                                          hasType(Container),
+                                          hasType(pointsTo(Container)),
+                                          hasType(references(Container))))))
----------------
And here.


================
Comment at: 
clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp:102-105
+  std::string ReplacementText;
+  ReplacementText = std::string(Lexer::getSourceText(
+      CharSourceRange::getTokenRange(DRE->getSourceRange()),
+      *Result.SourceManager, getLangOpts()));
----------------



================
Comment at: 
clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h:23
+/// memory access when the container is empty.  Cases where the constant is not
+/// explictly zero can be addressed through teh clang static analyzer, and 
those
+/// which cannot be statically identified can be caught using UBSan.
----------------



================
Comment at: 
clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h:45
+
+#endif
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108893

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

Reply via email to