[PATCH] D34449: [clang-tidy] Enable constexpr definitions in headers.

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 21 08:32:26 PDT 2017


hokein added inline comments.


================
Comment at: test/clang-tidy/misc-definitions-in-headers.hpp:1
-// RUN: %check_clang_tidy %s misc-definitions-in-headers %t
+// RUN: %check_clang_tidy %s misc-definitions-in-headers %t -- -- -std=c++1z
 
----------------
The original code should work as `-std=c++11` will be added defaultly by `check_clang_tidy` script.


================
Comment at: test/clang-tidy/misc-definitions-in-headers.hpp:180
+class CE {
+  constexpr static int i = 5; // OK: constexpr definition.
+};
----------------
aaron.ballman wrote:
> This is not as safe as you might think. As-is, this is fine, however, if the class is given an inline function where that variable is odr-used, you will get an ODR violation.
> 
> I think it's mildly better to err on the side of safety here and diagnose.
I think the current code (Line `97` in `DefinitionsInHeadersCheck.cpp`) has already guaranteed this case. Can you try to run it without your change in the `DefinitionsInHeadersCheck.cpp`?

I think it still makes sense to add `constexpr` test cases.

  


Repository:
  rL LLVM

https://reviews.llvm.org/D34449





More information about the cfe-commits mailing list