hokein added a comment.
Have thought this a bit more, I misunderstood your patch previously (sorry for
that).
I think what you intend to do is to ignore C++17 `inline variables` in headers,
am I correct?
================
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
----------------
hokein wrote:
> The original code should work as `-std=c++11` will be added defaultly by
> `check_clang_tidy` script.
`constexpr` variables have internal linkage, which should be detected for the
current check (but the test case is missing this kind of case).
If you want to test `inline` variables, I'd suggest adding a new test file like
`misc-definitions-in-headers-1z.hpp` which includes cases of inline variables.
================
Comment at: test/clang-tidy/misc-definitions-in-headers.hpp:180
+class CE {
+ constexpr static int i = 5; // OK: constexpr definition.
+};
----------------
hokein wrote:
> 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.
>
>
In C++17, `constexpr static int i` is an inline variable, which is fine to
define in C++ header -- because `inline` specifier provides a facility allowing
definitions (functions/variables) in header that is included in multiple TUs.
Additionally, one of the `inline variable` motivations is to support the
development of header-only libraries, you can find discussions in
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/n4424.pdf.
Therefore, I'm +1 ignore the inline variables (the same as inline functions).
Repository:
rL LLVM
https://reviews.llvm.org/D34449
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
