alexfh added inline comments.
================
Comment at: clang-tidy/misc/DefinitionsInHeadersCheck.cpp:25
@@ +24,3 @@
+ SourceLocation ExpansionLoc = SM.getExpansionLoc(Node.getLocStart());
+ StringRef Filename = SM.getFilename(ExpansionLoc);
+ return Filename.endswith(".h") || Filename.endswith(".hh") ||
----------------
alexfh wrote:
> alexfh wrote:
> > hokein wrote:
> > > aaron.ballman wrote:
> > > > > We're looking at the problem from different angles. My view is that a
> > > > > reasonable file naming convention (which at least makes interface
> > > > > header files, textual headers and main files distinguishable) is a
> > > > > widespread enough practice, and the benefits it brings outweigh the
> > > > > costs of enforcing it. However, the opposite point of view also has
> > > > > its right to exist, so we need a solution that fits both ;)
> > > > >> Perhaps another solution to this is use isInMainFile() ||
> > > > >> usesHeaderFileExtension().
> > > > >You probably meant !isInMainFile() || usesHeaderFileExtension(). I
> > > > >guess, that will work for us. We could also make the list of header
> > > > >file extensions (or a regular expression pattern for header files)
> > > > >configurable, so that the usesHeaderFileExtension() part could be
> > > > >disabled, if needed.
> > > >
> > > > Oops, you are correct, I meant !isInMainFile(). :-) I definitely agree
> > > > that we should make the header file extensions configurable. Would it
> > > > make sense if this were a global option that any checker can use? We
> > > > have 3-4 other checkers that care about header files as well, and it
> > > > would be nice if they all behaved consistently without the user having
> > > > to write a lot of options for each checker.
> > > I'm :+1 on making header file extensions configurable. I think we can do
> > > that in a new patch.
> > Having to configure this in a single place would be convenient. OTOH, I can
> > imagine corner-cases, where a single option would be undesired. One thing
> > we could do is to allow global options that can be overridden for each
> > check. Something along the lines of (modulo tests):
> >
> > ```
> > std::string OptionsView::get(StringRef LocalName, std::string Default)
> > const {
> > - const auto &Iter = CheckOptions.find(NamePrefix + LocalName.str());
> > + auto Iter = CheckOptions.find(NamePrefix + LocalName.str());
> > if (Iter != CheckOptions.end())
> > return Iter->second;
> > + // Fallback to global setting, if present.
> > + Iter = CheckOptions.find(LocalName.str());
> > + if (Iter != CheckOptions.end())
> > + return Iter->second;
> > return Default;
> > }
> > ```
> >
> > Alternatively, we could add another method (e.g. `getLocalOrGlobal`) to
> > make the usage of the global setting explicit.
> >
> > What do you think?
> This should read "Having a way to configure this in a single place ..."
> I'm :+1 on making header file extensions configurable. I think we can do that
> in a new patch.
Yep, this is fine for a follow-up.
http://reviews.llvm.org/D15710
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
