tbourvon added a comment.
In https://reviews.llvm.org/D37014#849157, @lebedev.ri wrote:
> Please add the following test: (and make sure that it does the right thing :))
>
> bool f_with_preproc_condition() {
> auto test = 42;
> assert(test == 42);
> return test;
> }
>
>
> I.e. if `-DNDEBUG` is present, variable is not needed, but if `-DNDEBUG` is
> *NOT* present...
Shouldn't we ignore these cases whether or not `-DNDEBUG` is present? If we
apply the fix here and remove the variable while we have `-DNDEBUG`, and then
remove this define, then we'll get a compiler error for `test` not found.
In this case it can be fixed fairly easily, but this could in fact introduce
bugs with more complex conditional macros and preprocessor directives. For
example:
#ifdef WIN32
#define MY_MACRO(test) test = 0
#else
#define MY_MACRO(test) /* nothing */
bool f() {
auto test = 42;
MY_MACRO(test);
return (test == 42)
}
If we want to ignore these cases not matter what, which seems to me like the
most logical and reasonable thing to do, we need to be able to know if there
are preprocessor directives between the variable declaration and the `return`
statement.
In order to do that, we would need to be able to get the `PreprocessingRecord`
of the current compilation in order to call getPreprocessedEntitiesInRange()
<https://clang.llvm.org/doxygen/classclang_1_1PreprocessingRecord.html#a1f5d06d192dad434958ff00975aaaddd>,
but AFAICT this cannot be accessed from clang-tidy checks at the moment.
Should we introduce a way to reach that object from checks?
Repository:
rL LLVM
https://reviews.llvm.org/D37014
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
