Skip to content

Commit 5a3089d

Browse files
committed
Make sure we don't mangle roxygen comments when parsing; add a unit test
1 parent eecaf74 commit 5a3089d

File tree

3 files changed

+89
-13
lines changed

3 files changed

+89
-13
lines changed

inst/unitTests/cpp/attributes.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
#include <Rcpp.h>
2+
using namespace Rcpp;
3+
4+
//' @param foo // don't do anything to this
5+
//' // or this
6+
//' @param bar " // or this guy "
7+
// [[Rcpp::export]] // this comment marks the following function for export
8+
std::string comments_test( /// // "\""" some '"// more / // ///garbge"
9+
std::string msg = "Start a C++ line comment with the characters \"//\"" // "" \" ""
10+
) { // """
11+
return msg;
12+
}

inst/unitTests/runit.attributes.R

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
#!/usr/bin/r -t
2+
# -*- mode: R; tab-width: 4; -*-
3+
#
4+
# Copyright (C) 2014 Dirk Eddelbuettel, Romain Francois, and Kevin Ushey
5+
#
6+
# This file is part of Rcpp.
7+
#
8+
# Rcpp is free software: you can redistribute it and/or modify it
9+
# under the terms of the GNU General Public License as published by
10+
# the Free Software Foundation, either version 2 of the License, or
11+
# (at your option) any later version.
12+
#
13+
# Rcpp is distributed in the hope that it will be useful, but
14+
# WITHOUT ANY WARRANTY; without even the implied warranty of
15+
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
16+
# GNU General Public License for more details.
17+
#
18+
# You should have received a copy of the GNU General Public License
19+
# along with Rcpp. If not, see <http://www.gnu.org/licenses/>.
20+
21+
.runThisTest <- Sys.getenv("RunAllRcppTests") == "yes"
22+
23+
if (.runThisTest) {
24+
25+
.setUp <- Rcpp:::unitTestSetup("attributes.cpp")
26+
test.attributes <- function() {
27+
checkEquals(
28+
comments_test(),
29+
"Start a C++ line comment with the characters \"//\""
30+
)
31+
}
32+
33+
}

src/attributes.cpp

Lines changed: 44 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ namespace attributes {
7474

7575
// Trim a string
7676
void trimWhitespace(std::string* pStr);
77-
77+
7878
// Strip trailing line comments
7979
void stripTrailingLineComments(std::string* pStr);
8080

@@ -87,6 +87,9 @@ namespace attributes {
8787
// show a warning message
8888
void showWarning(const std::string& msg);
8989

90+
// is the line a C++ roxygen comment? (started with //')
91+
bool isRoxygenCpp(const std::string& str);
92+
9093
} // namespace attributes
9194
} // namespace Rcpp
9295

@@ -961,7 +964,7 @@ namespace attributes {
961964
// trim before we do this just in case someone updates the regex
962965
// to allow for whitespace around the call
963966
trimWhitespace(&paramsText);
964-
967+
965968
paramsText = paramsText.substr(1, paramsText.size()-2);
966969

967970
// parse the parameters
@@ -1012,7 +1015,7 @@ namespace attributes {
10121015
const std::string& input) {
10131016

10141017
const std::string delimiters(" ,");
1015-
1018+
10161019
std::vector<Param> params;
10171020
std::string::size_type current;
10181021
std::string::size_type next = -1;
@@ -2348,33 +2351,42 @@ namespace attributes {
23482351
bool isWhitespace(char ch) {
23492352
return std::strchr(kWhitespaceChars, ch) != NULL;
23502353
}
2351-
2354+
23522355
// Remove trailing line comments -- ie, find comments that don't begin
23532356
// a line, and remove them. We avoid stripping attributes.
23542357
void stripTrailingLineComments(std::string* pStr) {
2355-
2358+
23562359
if (pStr->empty()) return;
2357-
2360+
23582361
size_t len = pStr->length();
23592362
bool inString = false;
23602363
size_t idx = 0;
2361-
2364+
2365+
// if this is an roxygen comment, then bail
2366+
if (isRoxygenCpp(*pStr)) return;
2367+
23622368
// skip over initial whitespace
23632369
idx = pStr->find_first_not_of(kWhitespaceChars);
23642370
if (idx == std::string::npos) return;
2365-
2371+
23662372
// skip over a first comment
23672373
if (idx + 1 < len && pStr->at(idx) == '/' && pStr->at(idx + 1) == '/') {
23682374
idx = idx + 2;
23692375
}
2370-
2376+
23712377
// since we are searching for "//", we iterate up to 2nd last character
23722378
while (idx < len - 1) {
2373-
2374-
if (pStr->at(idx) == '"') {
2375-
inString = !inString;
2379+
2380+
if (inString) {
2381+
if (pStr->at(idx) == '"' && pStr->at(idx - 1) != '\\') {
2382+
inString = false;
2383+
}
2384+
} else {
2385+
if (pStr->at(idx) == '"') {
2386+
inString = true;
2387+
}
23762388
}
2377-
2389+
23782390
if (!inString &&
23792391
pStr->at(idx) == '/' &&
23802392
pStr->at(idx + 1) == '/') {
@@ -2425,6 +2437,25 @@ namespace attributes {
24252437
warning(msg, Rcpp::Named("call.") = false);
24262438
}
24272439

2440+
bool isRoxygenCpp(const std::string& str) {
2441+
size_t len = str.length();
2442+
if (len < 3) return false;
2443+
size_t idx = str.find_first_not_of(kWhitespaceChars);
2444+
if (idx == std::string::npos) return false;
2445+
2446+
// make sure there are characters to check
2447+
if (len - 2 < idx) return false;
2448+
2449+
if (str[idx] == '/' &&
2450+
str[idx + 1] == '/' &&
2451+
str[idx + 2] == '\'') {
2452+
return true;
2453+
}
2454+
2455+
return false;
2456+
2457+
}
2458+
24282459
} // namespace attributes
24292460
} // namespace Rcpp
24302461

0 commit comments

Comments
 (0)