Skip to content

Commit 43d412a

Browse files
facchinmmatthijskooijman
authored andcommitted
Fix duplicate-skipping in prototype generation
While generating prototypes, a scan is made for any function definitions which would result in identical prototypes, and all but the first are skipped. However, when doing so, this did not take into account function definitions which were already marked to be skipped, which could cause *all* definitions to be skipped. This behaviour caused a particular problem when a method definition existed with the same signature as a function definition. All method definitions are skipped through `tagIsUnhandled`, but could still make `skipDuplicates` skip the corresponding function definition. This commit fixes this. It is not entirely clear to me if this duplicate removal is actually needed at all. If code contains multiple function definitions with the same signature, you would expect the compiler to throw an error anyway. If duplicate removal would be removed, only one testcase (TestCTagsToPrototypesShouldDealFunctionWithDifferentSignatures) fails, but that testcase uses a predefined ctags output that contains a duplicate function definition. It is not quite clear what source files this output was generated from (it seems to stem from CharWithEscapedDoubleQuote.ino, which originated at arduino/Arduino#1245, but those only contain one version of the function definition, not two). Still, since the ctags parsing will hopefully be replaced in the near future, this just fixes the duplicate removal instead of removing it and risking a regression on some corner case. This fixes arduino#140. Signed-off-by: Martino Facchin <[email protected]> Signed-off-by: Matthijs Kooijman <[email protected]>
1 parent 6183f3b commit 43d412a

File tree

3 files changed

+61
-1
lines changed

3 files changed

+61
-1
lines changed

src/arduino.cc/builder/ctags/ctags_parser.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ func removeDuplicate(tags []*types.CTag) {
130130
definedPrototypes := make(map[string]bool)
131131

132132
for _, tag := range tags {
133-
if !definedPrototypes[tag.Prototype] {
133+
if !definedPrototypes[tag.Prototype] && tag.SkipMe == false {
134134
definedPrototypes[tag.Prototype] = true
135135
} else {
136136
tag.SkipMe = true

src/arduino.cc/builder/test/ctags_to_prototypes_test.go

+52
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,13 @@
3030
package test
3131

3232
import (
33+
"arduino.cc/builder"
34+
"arduino.cc/builder/constants"
3335
"arduino.cc/builder/ctags"
3436
"arduino.cc/builder/types"
3537
"github.com/stretchr/testify/require"
3638
"io/ioutil"
39+
"os"
3740
"path/filepath"
3841
"testing"
3942
)
@@ -459,3 +462,52 @@ func TestCTagsToPrototypesFunctionPointers(t *testing.T) {
459462

460463
require.Equal(t, 2, ctx.PrototypesLineWhereToInsert)
461464
}
465+
466+
func TestCTagsRunnerSketchWithClassFunction(t *testing.T) {
467+
DownloadCoresAndToolsAndLibraries(t)
468+
469+
sketchLocation := Abs(t, filepath.Join("sketch_class_function", "sketch_class_function.ino"))
470+
471+
ctx := &types.Context{
472+
HardwareFolders: []string{filepath.Join("..", "hardware"), "hardware", "downloaded_hardware"},
473+
ToolsFolders: []string{"downloaded_tools"},
474+
BuiltInLibrariesFolders: []string{"downloaded_libraries"},
475+
OtherLibrariesFolders: []string{"libraries"},
476+
SketchLocation: sketchLocation,
477+
FQBN: "arduino:avr:leonardo",
478+
ArduinoAPIVersion: "10600",
479+
Verbose: true,
480+
}
481+
482+
buildPath := SetupBuildPath(t, ctx)
483+
defer os.RemoveAll(buildPath)
484+
485+
commands := []types.Command{
486+
487+
&builder.ContainerSetupHardwareToolsLibsSketchAndProps{},
488+
489+
&builder.ContainerMergeCopySketchFiles{},
490+
491+
&builder.ContainerFindIncludes{},
492+
493+
&builder.PrintUsedLibrariesIfVerbose{},
494+
&builder.WarnAboutArchIncompatibleLibraries{},
495+
&builder.CTagsTargetFileSaver{Source: &ctx.Source, TargetFileName: constants.FILE_CTAGS_TARGET},
496+
&ctags.CTagsRunner{},
497+
&ctags.CTagsParser{},
498+
&CollectCtagsFromPreprocSource{},
499+
&ctags.CTagsToPrototypes{},
500+
}
501+
502+
for _, command := range commands {
503+
err := command.Run(ctx)
504+
NoError(t, err)
505+
}
506+
507+
prototypes := ctx.Prototypes
508+
509+
require.Equal(t, 3, len(prototypes))
510+
require.Equal(t, "void setup();", prototypes[0].Prototype)
511+
require.Equal(t, "void loop();", prototypes[1].Prototype)
512+
require.Equal(t, "void asdf();", prototypes[2].Prototype)
513+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
class test {
2+
void asdf() {}
3+
};
4+
void setup() {
5+
asdf();
6+
}
7+
void loop() {}
8+
void asdf() {}

0 commit comments

Comments
 (0)