This is an archive of the discontinued LLVM Phabricator instance.

[Clang] Fix some Clang-tidy modernize-use-using and Include What You Use warnings
Needs ReviewPublic

Authored by Eugene.Zelenko on Aug 16 2016, 5:55 PM.

Details

Summary

I checked this patch on my own build on RHEL 6. Regressions were OK.

Diff Detail

Repository
rL LLVM

Event Timeline

Eugene.Zelenko retitled this revision from to [Clang] Fix some Clang-tidy modernize-use-using and Include What You Use warnings.
Eugene.Zelenko updated this object.
Eugene.Zelenko set the repository for this revision to rL LLVM.
Eugene.Zelenko added a subscriber: cfe-commits.
compnerd added inline comments.Aug 16 2016, 10:29 PM
include/clang/Basic/IdentifierTable.h
34

Not sure the whitespace adds any benefit here.

103

Isn't the LLVM style to prefer the negation? Why did the infix operators not get space around them?

include/clang/Frontend/ASTUnit.h
52

Not sure the whitespace is beneficial here.

928

This is weirdly formatted. I think its fine to clang-format this line.

include/clang/Lex/PreprocessorOptions.h
22

Don't think that the whitespace is beneficial here.

Apply Clang-format.

Eugene.Zelenko marked an inline comment as done.Aug 17 2016, 2:23 PM
Eugene.Zelenko added inline comments.
include/clang/Basic/IdentifierTable.h
103

memcpy result is three state, so applying Boolean operator is not correct.

include/clang/Lex/PreprocessorOptions.h
22

I wanted to make spacing consistent. Same in other places.

Actually, Include What You Use recommended to include MemoryBuffer.h. MemoryBuffer is used in smart pointers, but I remember that I encountered problems in LLDB MSVC builds, where forward declaration was not enough for such usage.

mehdi_amini added inline comments.Aug 26 2016, 3:30 PM
include/clang/Basic/IdentifierTable.h
103

What do you mean "applying Boolean operator is not correct"?
http://en.cppreference.com/w/cpp/language/implicit_conversion # Boolean conversions

The value zero (for integral, floating-point, and unscoped enumeration) and the null pointer and the null pointer-to-member values become false. All other values become true.

Seems good to me?

include/clang/Lex/PreprocessorOptions.h
22

I think in a recent review you mentioned "empty lines make code more readable for large namespaces", which does not seem to be the case here?

Eugene.Zelenko added inline comments.Aug 26 2016, 4:15 PM
include/clang/Basic/IdentifierTable.h
103

I think explicit operation is more comprehensible them implicit.

include/clang/Lex/PreprocessorOptions.h
22

I prefer spacing to be consistent.