[Rd] Making headers self-contained for static analysis

[Marc Schwartz]

Hi,

There are a limited number of MIME file types that are accepted through the list server, with plain text being one. Even though a patch file should be plain text, it is possible that your mail client may not have set the correct MIME type for your patch file attachment. If so, that would explain why it was filtered from the list.

For future reference, if you change the file extension to ".txt" and then attach it, that should get picked up as plain text and get through the list server filters.

Regards,

Marc Schwartz
R-Devel Co-Admin


On March 16, 2023 at 2:32:39 PM, Lionel Henry via R-devel (r-devel using r-project.org (mailto:r-devel using r-project.org)) wrote:

> People have let me know that the attachment didn't make it through.
> Do patches get filtered out?
>
> Please find it there:
> https://github.com/lionel-/r-svn/commit/e3de56798b1321a3fa8688a42bbb73d763b78024.patch
>
> I'm also happy to post it on the bugzilla if that makes sense.
>
> Best,
> Lionel
>
> On 3/16/23, Lionel Henry wrote:
> > Hello,
> >
> > I started using clangd to get better static analysis and code
> > refactoring tooling with the R sources (using eglot-mode in Emacs, it
> > just works once you've generated a `compile_commands.json` file with
> > `bear make all`). I noticed that the static analyser can't understand
> > several header files because these are not self-contained. So I went
> > through all .h files and inserted the missing includes, cf the
> > attached patch.
> >
> > Making the headers self-contained has consequences for the .c or .cpp
> > files that include them. In the case of C files, the only downside I
> > see is that it might cause users to accidentally rely on indirect
> > inclusion of standard headers, instead of directly including the
> > header to make the dependency explicit as would be good practice.
> > This doesn't seem like a big deal compared to the benefits of enabling
> > static analysis.
> >
> > However in the case of C++ that's more problematic. We don't want to
> > include the C headers because that would pollute the global namespace
> > and users might prefer to import the missing symbols (`size_t` and
> > `FILE`) selectively. Also that wouldn't help static analysis within
> > the header files since the analysers use the C path. So I have guarded
> > inclusion of standard C headers behing a `__cplusplus` check.
> >
> > If that makes sense, would R core consider applying the attached patch
> > to the R sources?
> >
> > Best,
> > Lionel
> >
>
> ______________________________________________
> R-devel using r-project.org mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel